Ghanshyam Thakkar [off-list ref] writes:
With the addition of this unit test, we impose a new restriction of
running the unit tests from either 't/' or 't/unit-tests/bin/'
directories. This is to construct the path to files which contain some
input urls under the 't/t-urlmatch-normalization' directory. This
restriction is similar to one we have for end-to-end tests, where they
can be ran from only 't/'.
Addition of 't/unit-tests/bin/' is to allow
for running individual tests which is not currently possible via any
'make' targets and also 'unit-tests-test-tool' target is also ran from
the 't/unit-tests/bin' directory.
Sorry, but I do not quite follow. The above makes it sound as if
the 'bin' subdirectory is something that never existed before this
patch and this patch introduces the use of that directory, but that
is hardly the case. What does that "Addition of" really refer to?
Do you mean "we cannot run the tests from arbitrary places, and we
allow them to be run from t/, just like the normal tests" followed
by "in addition, we also allow them to be run from t/unit-tests/bin
directory because ..."?
I wonder if we should get of t/t-urlmatch-normalization/ directory
and instead hold these test data in the form of string constants in
the program. After all, you have the expected normalization result
hardcoded in the binary (e.g. t_url_high_bit() asks the checker
function to read from "url-1" file and then compare the result of
normalization with a hardcoded string constant), so having the test
data in separate files only risks the input and the output easily
drift apart.
As a side effect, it would make it easily possible to run the tests
anywhere, because you no longer depend on these url-$n input files.
It of course depends on how burdensome the limitation that we can
run the tests only from a fixed place really is, but it generally is
not a good idea to have these random sequence of bytes in small
files that nobody looks at in a repository in the first place.
Thanks.
On 14/08/24 00:52, Junio C Hamano wrote:
Ghanshyam Thakkar [off-list ref] writes:
I wonder if we should get of t/t-urlmatch-normalization/ directory
and instead hold these test data in the form of string constants in
the program. After all, you have the expected normalization result
hardcoded in the binary (e.g. t_url_high_bit() asks the checker
function to read from "url-1" file and then compare the result of
normalization with a hardcoded string constant), so having the test
data in separate files only risks the input and the output easily
drift apart.
As a side effect, it would make it easily possible to run the tests
anywhere, because you no longer depend on these url-$n input files.
It of course depends on how burdensome the limitation that we can
run the tests only from a fixed place really is, but it generally is
not a good idea to have these random sequence of bytes in small
files that nobody looks at in a repository in the first place.
I think the reason these inputs are present in the files is solely
because they are random sequence of characters which contain unicode
and even some control characters. This makes it tricky to hold the
input string in the source itself.
I'm not sure there would be a straight-forward way to have these inputs
in the C source file. There may be some way to represent them in an
alternate form but I suppose that would sacrifice the readability of
these inputs which I believe is also a significant factor for test cases.
Feel free to enlighten us if we're possibly missing some straight
forward way of having these input URLs in the source files.
--
Sivaraam
Junio C Hamano [off-list ref] wrote:
Ghanshyam Thakkar [off-list ref] writes:
quoted
With the addition of this unit test, we impose a new restriction of
running the unit tests from either 't/' or 't/unit-tests/bin/'
directories. This is to construct the path to files which contain some
input urls under the 't/t-urlmatch-normalization' directory. This
restriction is similar to one we have for end-to-end tests, where they
can be ran from only 't/'.
Addition of 't/unit-tests/bin/' is to allow
for running individual tests which is not currently possible via any
'make' targets and also 'unit-tests-test-tool' target is also ran from
the 't/unit-tests/bin' directory.
Sorry, but I do not quite follow. The above makes it sound as if
the 'bin' subdirectory is something that never existed before this
patch and this patch introduces the use of that directory, but that
is hardly the case. What does that "Addition of" really refer to?
Do you mean "we cannot run the tests from arbitrary places, and we
allow them to be run from t/, just like the normal tests" followed
by "in addition, we also allow them to be run from t/unit-tests/bin
directory because ..."?
Yes, I meant that. I sent a v3 which embeds those URLs from files into
the code itself, which should not require such restriction.
Link: https://lore.kernel.org/git/20240814142057.94671-1-shyamthakkar001@gmail.com/
Thanks.