Junio C Hamano <gitster@xxxxxxxxx> 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.
