Re: [PATCH v3 00/35] 20210215154427.32693-1-avarab@gmail.com

From: Johannes Sixt <j6t@kdbg.org>
Date: Mar 1, 2021, 07:10 UTC
Message-ID: <b9a0406d-a913-767c-fbec-be57246bd4e4@kdbg.org>
In-Reply-To: <875z2ckwfl.fsf@evledraar.gmail.com>

Am 28.02.21 um 17:07 schrieb Ævar Arnfjörð Bjarmason:

Show 23 quoted lines

> On Sun, Feb 28 2021, Johannes Sixt wrote:
>> 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.

I missed that the descriptions are gone now that many test cases are shoved into a single file when the simple framework is used. That is indeed a disadvantage. But please keep in mind that code is more often read than written. *If* we have to grow a new framework, then it must not suffer from unreadability.

> 
>  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.

As I said, the config in perl.sh is only used for a multi-line pattern test. That is dispensable as there is already a similar test with custom patterns. And the tests for custom patterns can be moved to the main test file entirely.

> 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.

That sounds like an improvement.

-- Hannes