Re: [PATCH v3 3/7] unit-tests: add for_test

[Junio C Hamano <gitster@pobox.com>]

Kyle Lippincott <spectral@google.com> writes:

> But the main simplification in that patch (not using
> `setup_populated`) can be achieved without `for_test`:
>
> + static void t_addch_appends(void)
> + {
> +     struct strbuf sb = STRBUF_INIT;
> +     t_addstr(&sb, "initial value");
> +     t_addch(&sb, 'a');
> +     t_release(&sb);
> + }
>
> -       TEST(setup_populated(t_addch, "initial value", "a"),
> -            "strbuf_addch appends to initial value");
> +       TEST(t_addch_appends(), "strbuf_addch appends to initial value");

Yup.  I like that better when we are using TEST(), whether the other
thing exists or not.

> It seems to me that all `for_test` is getting us there is an inlining
> of the test logic, which seems like it's optimizing for vertical space
> in the file.

I would consider that it optimizes for human readers who have to
scan the file.  Having to jump around from caller to callee to
understand the whole thing (rather, the description being away from
the callee, which is where the most of the logic is anyway) is one
gripe I have with the approach taken by the TEST() thing.

> Maybe it's because I'm coming from a C++ environment at
> $JOB that's using Google's gunit and gmock frameworks, where every
> test is in its own function and we usually don't even write the main
> function ourselves, but I have a preference for the separate
> functions.

If we do not have to write the main at all, then it would make the
separate function that implements a single test a lot more palatable,
and we do not even have to implement and call TEST() macro ;-).

Thanks.