Re: [PATCH v2 4/4] t-ctype: avoid duplicating class names
Hi Phillip,
Am 09.03.24 um 12:28 schrieb Phillip Wood:
Show 24 quoted lines
> On reflection I don't think I'm objecting to allowing statements,
> only the use of the private functions to do so. If we tweak
> test__run_begin() and test__run_end() so that the description is
> passed to test__run_begin() and we invert the return value of that
> function to match what test__run_end() is expecting then we can have
>
> #define TEST_BEGIN(...) \
> do { \
> int run__ = test__run_begin(__VA_ARGS__); \
> if (run__)
>
> #define TEST_END \
> test_run_end(run__); \
> } while (0)
>
> Which allow test authors to write
>
> TEST_BEGIN("my test") {
> /* test body here */
> } TEST_END;
>
> The macros insulate the test code from having to worry about
> test_skip_all() and the "do { ... } while (0)" means that the
> compiler will complain if the author forgets TEST_END.the location information is missing, but I get the idea. That would certainly work for t-ctype.
Show 6 quoted lines
> I'm slightly on the fence about including the braces in the macros
> instead as that would make them harder to misuse but it would be less
> obvious that the test body is run in its own block. The compiler will
> allow the test author to accidentally nest two calls to TEST_BEGIN()
> but there is an assertion in test__run_begin() which will catch that
> at run time.
Thought about that as well, and I'd also be wary of including any of the control statements. Custom syntax requires learning, can have weird side-effects and may be misunderstood by editors.
Below is a patch that adds the function test_start() and its companion macro TEST_START, which allow defining tests with minimal ceremony, similarly as in the shell-based test suite:
#include "test-lib.h"
int cmd_main(int argc, const char **argv)
{
if (TEST_START("something works"))
check(something());
if (TEST_START("something else works"))
check(something_else());
return test_done();
}It requires storing string copies and sits between the states of the original test machinery, so it's a bit complicated.
The biggest downside so far, though, is that I couldn't find an example in the other unit tests that it would simplify significantly. At least it would allow getting rid of the void pointers in t-strbuf, however.
Show 10 quoted lines
> The slight downside compared to TEST() is that it is harder to return
> early if a check fails - we'd need something like
>
> TEST_BEGIN("my test") {
> if (!check(0))
> goto fail
> /* more checks */
> fail:
> ;
> } TEST_END;TEST is worse in this regard, as it doesn't allow "if" directly at all. You can use short-circuiting, of course:
TEST(check(!!ptr) && check(*ptr == value), "ptr points to value");
But you can do that with TEST_BEGIN as well. In a function you can return early, but you can use functions with both, too.
In your example you can use "continue" instead of "goto fail". And with "break" you can skip the test_run_end() call. I consider both to be downsides, though -- the abstraction is leaky.
> Also unlike TEST(), TEST_END does not indicate to the caller whether
> the test failed or not but I'm not sure that matters in practice.
Most TEST invocations out of t-basic ignore its return value so far.
ctx.result is left unchanged by test__run_end(), so we could still access it if really needed. Perhaps through a sanctioned function, last_test_result() or similar.
René
Show changes to 3 files +49 −4
t/unit-tests/t-ctype.c, t/unit-tests/test-lib.c, t/unit-tests/test-lib.h
diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
index d6ac1fe678..e7c846814e 100644
--- a/t/unit-tests/t-ctype.c
+++ b/t/unit-tests/t-ctype.c
@@ -4,15 +4,13 @@
size_t len = ARRAY_SIZE(string) - 1 + \
BUILD_ASSERT_OR_ZERO(ARRAY_SIZE(string) > 0) + \
BUILD_ASSERT_OR_ZERO(sizeof(string[0]) == sizeof(char)); \
- int skip = test__run_begin(); \
- if (!skip) { \
+ if (TEST_START(#class " works")) { \
for (int i = 0; i < 256; i++) { \
if (!check_int(class(i), ==, !!memchr(string, i, len)))\
test_msg(" i: 0x%02x", i); \
} \
check(!class(EOF)); \
} \
- test__run_end(!skip, TEST_LOCATION(), #class " works"); \
} while (0)
#define DIGIT "0123456789"
diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c
index 66d6980ffb..bc484c92da 100644
--- a/t/unit-tests/test-lib.c
+++ b/t/unit-tests/test-lib.c
@@ -16,6 +16,8 @@ static struct {
unsigned running :1;
unsigned skip_all :1;
unsigned todo :1;
+ char *desc;
+ char *location;
} ctx = {
.lazy_plan = 1,
.result = RESULT_NONE,
@@ -123,9 +125,45 @@ void test_plan(int count)
ctx.lazy_plan = 0;
}
+static void test_run_maybe_end(void)
+{
+ if (ctx.running) {
+ assert(ctx.location);
+ assert(ctx.desc);
+ test__run_end(0, ctx.location, "%s", ctx.desc);
+ FREE_AND_NULL(ctx.location);
+ FREE_AND_NULL(ctx.desc);
+ }
+ assert(!ctx.running);
+ assert(!ctx.location);
+ assert(!ctx.desc);
+}
+
+int test_start(const char *location, const char *format, ...)
+{
+ va_list ap;
+ char *desc;
+
+ test_run_maybe_end();
+
+ va_start(ap, format);
+ desc = xstrvfmt(format, ap);
+ va_end(ap);
+
+ if (test__run_begin()) {
+ test__run_end(1, location, "%s", desc);
+ free(desc);
+ return 0;
+ } else {
+ ctx.location = xstrdup(location);
+ ctx.desc = desc;
+ return 1;
+ }
+}
+
int test_done(void)
{
- assert(!ctx.running);
+ test_run_maybe_end();
if (ctx.lazy_plan)
test_plan(ctx.count);
diff --git a/t/unit-tests/test-lib.h b/t/unit-tests/test-lib.h
index a8f07ae0b7..2be95b3ab8 100644
--- a/t/unit-tests/test-lib.h
+++ b/t/unit-tests/test-lib.h
@@ -21,6 +21,15 @@
*/
void test_plan(int count);
+/*
+ * Start a test. It ends when the next test starts or test_done()
+ * is called. Returns 1 if the test was actually started, 0 if it was
+ * skipped because test_skip_all() had been called.
+ */
+int test_start(const char *location, const char *format, ...);
+
+#define TEST_START(...) test_start(TEST_LOCATION(), __VA_ARGS__)
+
/*
* test_done() must be called at the end of main(). It will print the
* plan if plan() was not called at the beginning of the test program
René Scharfe
