From: "René Scharfe" <l.s.r@web•de>
To: phillip.wood@dunelm•org.uk, git@vger•kernel.org
Cc: Josh Steadmon <steadmon@google•com>,
Achu Luma <ach.lumap@gmail•com>,
Christian Couder <christian.couder@gmail•com>
Subject: Re: [PATCH v2 4/4] t-ctype: avoid duplicating class names
Date: Sun, 10 Mar 2024 13:48:22 +0100 [thread overview]
Message-ID: <69e24019-1fff-40df-9aef-cdfd058124c9@web.de> (raw)
In-Reply-To: <4f41f509-1d44-4476-92b0-9bb643f64576@gmail.com>
Hi Phillip,
Am 09.03.24 um 12:28 schrieb Phillip Wood:
> 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.
> 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.
> 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é
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
next prev parent reply other threads:[~2024-03-10 12:48 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-25 11:27 [PATCH 0/3] t-ctype: simplify unit test definitions René Scharfe
2024-02-25 11:27 ` [PATCH 1/3] t-ctype: allow NUL anywhere in the specification string René Scharfe
2024-02-25 18:05 ` Eric Sunshine
2024-02-25 18:28 ` René Scharfe
2024-02-25 18:41 ` Eric Sunshine
2024-02-25 21:00 ` Jeff King
2024-02-25 21:02 ` Eric Sunshine
2024-02-25 11:27 ` [PATCH 2/3] t-ctype: avoid duplicating class names René Scharfe
2024-02-25 11:27 ` [PATCH 3/3] t-ctype: do one test per class and char René Scharfe
2024-02-26 9:28 ` Christian Couder
2024-02-26 17:26 ` René Scharfe
2024-02-26 17:44 ` Junio C Hamano
2024-02-26 18:58 ` Josh Steadmon
2024-02-27 10:04 ` Christian Couder
2024-03-02 22:00 ` René Scharfe
2024-03-04 10:00 ` Christian Couder
2024-03-06 18:16 ` René Scharfe
2024-03-04 18:35 ` Josh Steadmon
2024-03-04 18:46 ` Junio C Hamano
2024-03-03 10:13 ` [PATCH v2 0/4] t-ctype: simplify unit test definitions René Scharfe
2024-03-03 10:13 ` [PATCH v2 1/4] t-ctype: allow NUL anywhere in the specification string René Scharfe
2024-03-03 10:13 ` [PATCH v2 2/4] t-ctype: simplify EOF check René Scharfe
2024-03-03 10:13 ` [PATCH v2 3/4] t-ctype: align output of i René Scharfe
2024-03-03 10:13 ` [PATCH v2 4/4] t-ctype: avoid duplicating class names René Scharfe
2024-03-04 9:51 ` Phillip Wood
2024-03-06 18:16 ` René Scharfe
2024-03-08 14:05 ` Phillip Wood
2024-03-09 11:28 ` Phillip Wood
2024-03-10 12:48 ` René Scharfe [this message]
2024-03-04 9:25 ` [PATCH v2 0/4] t-ctype: simplify unit test definitions Christian Couder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=69e24019-1fff-40df-9aef-cdfd058124c9@web.de \
--to=l.s.r@web$(echo .)de \
--cc=ach.lumap@gmail$(echo .)com \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=steadmon@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox