On Sun, Feb 28 2021, Johannes Sixt wrote:
> Am 27.02.21 um 08:47 schrieb Johannes Sixt:
>> Am 24.02.21 um 20:50 schrieb Ævar Arnfjörð Bjarmason:
>>> Addresses feedback on v2. Since Junio & Johannes expressed a desire to
>>> keep the existing test scheme in t4018/* it's still there, but it's
>>> also possible to add *.sh tests in that directory to use the more
>>> familiar test framework used elsewhere in the test suite.
>>>
>>> The tests added here make use of it to e.g. supply custom -U<n>
>>> arguments, set config before the tests etc.
>>>
>>> I also improved that existing test support so you can have N tests in
>>> one file with (mostly) the existing test syntax. See the "userdiff
>>> tests: add a test with multiple tests in a LANG file" patch.
>>
>> I've read through all patches and had a comment here and there. I like a
>> lot that we can now put more than one test into a single file.
>>
>> However, I do not like the shell script version of tests, because the
>> syntax is so hard to read. Also, it looks to me that they are only
>> needed for a few tests that could just as well be coded as one-off tests
>> outside the framework.
>>
>> I've now pulled avar/t4018-diff-hunk-header-regex-tests-3 from your
>> github repo and will check again if I missed some cruicial points.
>
>
> I've now looked through the patch series again.
>
> I appreciate that you dug through the history and discovered and fixed a
> few deficiencies and loose ends. The way to throw all test cases for a
> language driver into a single file I like a lot, too. The way to specify
> expected results is manageable (modulo the dependency on the test
> number, t4018, that Junio mentioned).
Yes, maybe just "HEADER:" or something would be better. I was trying to
come up with something guaranteed not to conflict with the language in
question. Maybe:
=> HEADER =
=> description =
> But I do not see the need for the framework provided by the new
> test_diff_funcname. At the end of the series, it is only used by Perl
> and custom driver tests. (I discount the new ADA and Ruby tests as they
> can easily migrated to the simple test scheme.) But then the Perl tests
> do not do anything special, either. The multi-line pattern test is just
> a nice add-on but not strictly needed. In the end, the Perl test is just
> as straight-forward as all others.
The benefit now is:
1. Unlike the new plain-text "all test cases for a language driver" in
the same file mechanism you can have test descriptions. The
"description" in the golang one is just for show, you won't get
anything informative from test-lib.sh when your test fails.
2. I think having #1 beats not having test descriptions at all, or
having to shove a description like the Ruby:
"picks first "class/module/def" before changed context"
into something that would make all the FS's we have to support
happy.
3. A test in the new perl.sh one sets config. I think in both that case
and custom.sh it's more readable to carry such config setting with
the test, rather than at a distance in the main setup of t4018-*.sh.
That being said I'd like to improve the syntax a bit, in particular
instead of having a wrapper for test_expect_success I think it makes
sense just to have the test call test_expect_success, and then provide a
couple of helper functions.
