Thread (2 messages) 2 messages, 2 authors, 2024-07-25

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

From: Junio C Hamano <hidden>
Date: 2024-07-25 16:02:27

Kyle Lippincott [off-list ref] 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help