Re: [PATCH 5/6] t-strvec: use TEST_RUN

["René Scharfe" <l.s.r@web.de>]

Am 02.07.24 um 17:14 schrieb phillip.wood123@gmail.com:
> Hi René
>
> On 29/06/2024 16:46, René Scharfe wrote:
>> The macro TEST takes a single expression.  If a test requires
>> multiple statements then they need to be placed in a function
>> that's called in the TEST expression.
>>
>> Remove the overhead of defining and calling single-use functions
>> by using TEST_RUN instead.
>
> I'm not sure what the overhead is that you keep referring to - the
> compiler will inline functions that are only called once so I presume
> you mean overhead for test authors or contributors reading the code?

Yes, I mean the additional effort for developers to come up with
function names and write single-call functions, and for readers to
mentally connect test description and function.  Of course you still can
do that with TEST_RUN, but unlike TEST it doesn't force you.

I don't care much about changed duration of compilation or execution
here -- it shouldn't be much either way.

> This is not related to you changes but while looking at this patch I
> noticed that the existing code in check_strvec_loc() contains
>
>
>         if (!check_uint(vec->nr, >, nr) ||
>             !check_uint(vec->alloc, >, nr) ||
>             !check_str(vec->v[nr], str)) {
>             struct strbuf msg = STRBUF_INIT;
>             strbuf_addf(&msg, "strvec index %"PRIuMAX, (uintmax_t) nr);
>             test_assert(loc, msg.buf, 0);
>             strbuf_release(&msg);
>             va_end(ap);
>             return;
>
> Which looks like it should be using test_msg() instead of writing a
> message to an strbuf and calling test_assert().

Erm, yes, good find.  You know something isn't right if you see an
assert(0) or similar..

> The conversion itself looks correct. It is a shame that each test has
> to have boilerplate to initalize and free the strvec but that comes
> from the existing implementation.

I see that as a benefit: You can see what each test does at a glance;
no tricks, no dependencies.  Just exercise steps and checks.

René