Re: [PATCH 1/2] t/unit-tests: match functions signature with trailing code

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

Junio C Hamano <gitster@pobox.com> writes:

> A quick peek at [PATCH 2/2] tells me that this is not even something
> that would make it easier to port the existing tests by allowing
> more straight line-by-line copies or something.  The patch splits
> many in-line test pieces in the "main" into separate functions, and
> it does so in a rather unusual format, e.g.,
>
>   void test_hash__multi_character(void) TEST_HASH_STR("abc",
>           "a9993e364706816aba3e25717850c26c9cd0d89d",
>           "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad")
>
> where TEST_HASH_STR() expands to the function body that starts with
> a "{" and ends with a "}".  It can well be written more like
>
>     void test_hash__multi_character(void)
>     {
> 	TEST_HASH_STR("abc",
>         	"a9993e364706816aba3e25717850c26c9cd0d89d",
> 		"ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad");
>     }
>
> and we do not need this step at all if we did so.  Such a construct
> would be a lot friendlier to the editors that auto-indent, too.
>
> So, I do not quite see much value in this particular change.

Having said that, if this were more like that you write a series of

    DEF_HASH_TEST(multi_character, "abc", "a9993e...", "ba7816bf...")

and they expand to

    void test_hash__multi_character(void)
    {
	const char *expected[] = {"a9993e...", "ba7816bf..."};
	check_hash_data("abc", strlen("abc"), expected);
    }

then a preparatory step like this patch _might_ be justifiable.  You
may want to avoid having to write too many boilerplate, and a
special rule to find "DEF_HASH_TEST(name, ...)" and it might make
sense to add support to extract the name of the test function being
defined by the macro automatically.

Not that I think such a sequence of DEF_HASH_TEST(), one per line,
is an improvement at all (it also is unfriendly to editors that
auto-indent the same way as your original version).  I just wanted
to say that a change to the pattern to pick up the function name may
be justifiable if it were so.

Thanks.