Subject: Re: [PATCH 2/6] unit-tests: add TEST_RUN

From: Junio C Hamano <[email-masked]>
Date: Mon, 08 Jul 2024 08:39:16 -0700

[email-masked] writes:

> Hi René
>
> On 07/07/2024 08:20, René Scharfe wrote:
>> Hmm, again.  I can see the appeal.  How to call it?  Given that the
>> foreach macros have lower-case names, perhaps to indicate that they act
>> as control flow statements, not function-like macro calls, would we want
>> lower case here as well?
>
> I'd automatically assumed we'd want an uppercase name to signal that
> it was a pre-processor macro but I've not really spent any time
> thinking about it.
>
>> 	#define test(...) if (TEST_RUN(__VA_ARGS__))
>> 	        test ("passing test")
>> 	                check(1);
>> 		test ("split single item") {
>> 			struct strvec vec = STRVEC_INIT;
>> 			strvec_split(&vec, "foo");
>> 			check_strvec(&vec, "foo", NULL);
>> 			strvec_clear(&vec);
>> 		}
>> Can't get much cleaner than that.
>
> Yes, I like it

Yeah, if you squint your eyes hard, it starts to look a bit like
test_expect_success we use in the test suite ;-)

Isn't this introducing a new control structure to the language?

A macro that is a conditional switch (aka "if"-like statement),
having "if" in the name somewhere, and a macro that wrapts a loop
around a block (aka "for/while" like statement), having "for" in the
name somewhere, might be less confusing for the uninitiated.

> For someone new to the project it should hopefully be pretty clear how
> to use it from the existing tests once we have a few more test files
> that use it. Maybe an uppercase name would signal that it is special?

As to the cases, I personally prefer lowercase names whose semantics
is very clear, just like e.g., kh_foreach() makes it clear with
"foreach" that it iterates over the set.  But the above "test" may
not qualify---it is not making it sufficiently clear what control
structure it is creating with its name.

Thanks.