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

From: René Scharfe <l.s.r@web.de>
Date: Jul 7, 2024, 07:20 UTC
Message-ID: <62d221cc-532a-4a6d-8e96-b5a246ddeb1b@web.de>
In-Reply-To: <6c83357a-825f-49d9-8cc2-e81415e8010d@web.de>

Am 05.07.24 um 20:01 schrieb René Scharfe:

Show 36 quoted lines

> Am 05.07.24 um 11:42 schrieb phillip.wood123@gmail.com:
>> On 02/07/2024 21:55, René Scharfe wrote:
>>> Am 02.07.24 um 17:13 schrieb phillip.wood123@gmail.com:
>>>
>>>> I'm not very enthusiastic about requiring the test author to wrap
>>>> TEST_RUN() in an if() statement rather than just doing that for them.
>>>> It makes it explicit but from the test author's point of view it just
>>>> feels like pointless boilerplate.
>>>
>>> Hmm.  We can add more magic, but I suspect that it might confuse
>>> developers and editors.
>>
>> To me its confusing to have to wrap TEST_RUN() in an if() statement
>> until one realizes that the test might be skipped. If we document
>> that the test body should be enclosed in braces I don't think it
>> should confuse developers or editors and will keep the tests a bit
>> cleaner.
>
> You don't need braces in either case.  I.e. something like
>
> 	if (TEST_RUN("foo"))
> 		foo();
>
> works fine.  And
>
> 	#define IF_TEST_RUN(...) if (TEST_RUN(__VA_ARGS__))
> 	IF_TEST_RUN("foo")
> 		foo();
>
> works fine as well.
>
> The confusion that I'm afraid of is that we'd effectively invent a new
> control flow keyword here, which is unusual.  There are precedents,
> though: foreach macros like for_each_string_list_item.  We tell
> clang-format about them using its option ForEachMacros.  I see it also
> has an option IfMacros since version 13 (unused by us so far).  Hmm.

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?

	#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. Requires learning that this macro is not function-like, but it's probably not too much to ask.

René