Am 02.07.24 um 17:14 schrieb phillip.wood123@xxxxxxxxx:
> 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é
