Migrate part of nolibc-test.c to the new test harness.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 106 ++++++++++++++-------------
1 file changed, 56 insertions(+), 50 deletions(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index e173014f6b66..6c1b42b58e3e 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -42,6 +42,7 @@
#endif
#include "nolibc-test-linkage.h"
+#include "nolibc-harness.h"
/* for the type of int_fast16_t and int_fast32_t, musl differs from glibc and nolibc */
#define SINT_MAX_OF_TYPE(type) (((type)1 << (sizeof(type) * 8 - 2)) - (type)1 + ((type)1 << (sizeof(type) * 8 - 2)))
@@ -130,15 +131,6 @@ static const char *errorname(int err)
}
}
-static void putcharn(char c, size_t n)
-{
- char buf[64];
-
- memset(buf, c, n);
- buf[n] = '\0';
- fputs(buf, stdout);
-}
-
enum RESULT {
OK,
FAIL,
@@ -599,6 +591,19 @@ int expect_strne(const char *expr, int llen, const char *cmp)
#define CASE_TEST(name) \
case __LINE__: llen += printf("%d %s", test, #name);
+#if defined(NOLIBC)
+
+#define ASSUME_NOLIBC(stmt)
+
+#else /* defined(NOLIBC) */
+
+/* differ from nolibc, both glibc and musl have no global _auxv */
+unsigned long *_auxv = (void *)-1;
+#define ASSUME_NOLIBC(stmt) SKIP(stmt)
+
+#endif /* defined(NOLIBC) */
+
+
/* constructors validate that they are executed in definition order */
__attribute__((constructor))
static void constructor1(void)
@@ -612,53 +617,54 @@ static void constructor2(void)
constructor_test_value *= 2;
}
-int run_startup(int min, int max)
+static const void *pbrk(void)
{
- int test;
- int ret = 0;
- /* kernel at least passes HOME and TERM, shell passes more */
- int env_total = 2;
- /* checking NULL for argv/argv0, environ and _auxv is not enough, let's compare with sbrk(0) or &end */
extern char end;
- char *brk = sbrk(0) != (void *)-1 ? sbrk(0) : &end;
- /* differ from nolibc, both glibc and musl have no global _auxv */
- const unsigned long *test_auxv = (void *)-1;
-#ifdef NOLIBC
- test_auxv = _auxv;
-#endif
+ static char *brk;
- for (test = min; test >= 0 && test <= max; test++) {
- int llen = 0; /* line length */
+ if (brk)
+ return brk;
- /* avoid leaving empty lines below, this will insert holes into
- * test numbers.
- */
- switch (test + __LINE__ + 1) {
- CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
- CASE_TEST(argv_addr); EXPECT_PTRGT(1, test_argv, brk); break;
- CASE_TEST(argv_environ); EXPECT_PTRLT(1, test_argv, environ); break;
- CASE_TEST(argv_total); EXPECT_EQ(1, environ - test_argv - 1, test_argc ?: 1); break;
- CASE_TEST(argv0_addr); EXPECT_PTRGT(1, argv0, brk); break;
- CASE_TEST(argv0_str); EXPECT_STRNZ(1, argv0 > brk ? argv0 : NULL); break;
- CASE_TEST(argv0_len); EXPECT_GE(1, argv0 > brk ? strlen(argv0) : 0, 1); break;
- CASE_TEST(environ_addr); EXPECT_PTRGT(1, environ, brk); break;
- CASE_TEST(environ_envp); EXPECT_PTREQ(1, environ, test_envp); break;
- CASE_TEST(environ_auxv); EXPECT_PTRLT(test_auxv != (void *)-1, environ, test_auxv); break;
- CASE_TEST(environ_total); EXPECT_GE(test_auxv != (void *)-1, (void *)test_auxv - (void *)environ - 1, env_total); break;
- CASE_TEST(environ_HOME); EXPECT_PTRNZ(1, getenv("HOME")); break;
- CASE_TEST(auxv_addr); EXPECT_PTRGT(test_auxv != (void *)-1, test_auxv, brk); break;
- CASE_TEST(auxv_AT_UID); EXPECT_EQ(1, getauxval(AT_UID), getuid()); break;
- CASE_TEST(constructor); EXPECT_EQ(1, constructor_test_value, 2); break;
- CASE_TEST(linkage_errno); EXPECT_PTREQ(1, linkage_test_errno_addr(), &errno); break;
- CASE_TEST(linkage_constr); EXPECT_EQ(1, linkage_test_constructor_test_value, 6); break;
- case __LINE__:
- return ret; /* must be last */
- /* note: do not set any defaults so as to permit holes above */
- }
- }
- return ret;
+ brk = sbrk(0);
+
+ if (brk == (void *)-1)
+ brk = &end;
+
+ return brk;
}
+TEST(startup, argc) { ASSERT_GE(test_argc, 1); }
+TEST(startup, argv_addr) { ASSERT_GT((void *)test_argv, pbrk()); }
+TEST(startup, argv_environ) { ASSERT_LT(test_argv, environ); }
+TEST(startup, argv_total) { ASSERT_EQ(environ - test_argv - 1, test_argc ?: 1); }
+TEST(startup, argv0_addr) { ASSERT_GT((void *)argv0, pbrk()); }
+TEST(startup, argv0_str) { ASSERT_STRNZ((void *)argv0 > pbrk() ? argv0 : NULL); }
+TEST(startup, argv0_len) { ASSERT_GE((void *)argv0 > pbrk() ? strlen(argv0) : 0U, 1U); }
+TEST(startup, environ_addr) { ASSERT_GT((void *)environ, pbrk()); }
+TEST(startup, environ_envp) { ASSERT_EQ(environ, test_envp); }
+TEST(startup, environ_auxv) {
+ ASSUME_NOLIBC(return);
+ ASSERT_LT((void *)environ, (void *)_auxv);
+}
+TEST(startup, environ_total) {
+ ASSUME_NOLIBC(return);
+ /* kernel at least passes HOME and TERM, shell passes more */
+ ASSERT_GE((void *)_auxv - (void *)environ - 1, 2);
+}
+TEST(startup, environ_HOME) { ASSERT_NE(getenv("HOME"), NULL); }
+TEST(startup, auxv_addr) {
+ ASSUME_NOLIBC(return);
+ ASSERT_GT((void *)_auxv, pbrk());
+}
+TEST(startup, auxv_AT_UID) { ASSERT_EQ(getauxval(AT_UID), getuid()); }
+TEST(startup, constructor) { ASSERT_EQ(constructor_test_value, 2); }
+TEST(startup, linkage_errno) { ASSERT_EQ(linkage_test_errno_addr(), &errno); }
+TEST(startup, linkage_constr) { ASSERT_EQ(linkage_test_constructor_test_value, 6); }
+
+int run_startup(int min, int max)
+{
+ return run_test_suite("startup", min, max);
+}
/* used by some syscall tests below */
int test_getdents64(const char *dir)
--
2.42.1
On Wed, Nov 15, 2023 at 10:08:20PM +0100, Thomas Wei?schuh wrote:
> Migrate part of nolibc-test.c to the new test harness.
>
> Signed-off-by: Thomas Wei?schuh <[email protected]>
A few points, mostly questions and food for thoughts.
> -static void putcharn(char c, size_t n)
> -{
> - char buf[64];
> -
> - memset(buf, c, n);
> - buf[n] = '\0';
> - fputs(buf, stdout);
> -}
> -
Ah now I see how the other one came from :-) My comment about the size
check still stands anyway, especially when placed in an include file.
> +#if defined(NOLIBC)
> +
> +#define ASSUME_NOLIBC(stmt)
> +
> +#else /* defined(NOLIBC) */
> +
> +/* differ from nolibc, both glibc and musl have no global _auxv */
> +unsigned long *_auxv = (void *)-1;
> +#define ASSUME_NOLIBC(stmt) SKIP(stmt)
> +
> +#endif /* defined(NOLIBC) */
> +
I've seen below how it's used and don't find this very clear. In general,
passing a statement as an argument to a macro, especially control statements
such as "return" is a bit difficult to grasp. If the macro is only used for
this, maybe it should integrate the return statement and be called something
like "RETURN_UNLESS_NOLIBC()" which is quite explicit this time. If you really
need to keep the statement adjustable, then most likely that calling the
macro "UNLESS_NOLIBC()" would help, because I understand more naturally
that the following will perform a return if we're not on nolibc:
UNLESS_NOLIBC(return);
than:
ASSUME_NOLIBC(return);
> - for (test = min; test >= 0 && test <= max; test++) {
> - int llen = 0; /* line length */
> + if (brk)
> + return brk;
>
> - /* avoid leaving empty lines below, this will insert holes into
> - * test numbers.
> - */
> - switch (test + __LINE__ + 1) {
> - CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
> - CASE_TEST(argv_addr); EXPECT_PTRGT(1, test_argv, brk); break;
> - CASE_TEST(argv_environ); EXPECT_PTRLT(1, test_argv, environ); break;
> - CASE_TEST(argv_total); EXPECT_EQ(1, environ - test_argv - 1, test_argc ?: 1); break;
> - CASE_TEST(argv0_addr); EXPECT_PTRGT(1, argv0, brk); break;
> - CASE_TEST(argv0_str); EXPECT_STRNZ(1, argv0 > brk ? argv0 : NULL); break;
> - CASE_TEST(argv0_len); EXPECT_GE(1, argv0 > brk ? strlen(argv0) : 0, 1); break;
> - CASE_TEST(environ_addr); EXPECT_PTRGT(1, environ, brk); break;
> - CASE_TEST(environ_envp); EXPECT_PTREQ(1, environ, test_envp); break;
> - CASE_TEST(environ_auxv); EXPECT_PTRLT(test_auxv != (void *)-1, environ, test_auxv); break;
> - CASE_TEST(environ_total); EXPECT_GE(test_auxv != (void *)-1, (void *)test_auxv - (void *)environ - 1, env_total); break;
> - CASE_TEST(environ_HOME); EXPECT_PTRNZ(1, getenv("HOME")); break;
> - CASE_TEST(auxv_addr); EXPECT_PTRGT(test_auxv != (void *)-1, test_auxv, brk); break;
> - CASE_TEST(auxv_AT_UID); EXPECT_EQ(1, getauxval(AT_UID), getuid()); break;
> - CASE_TEST(constructor); EXPECT_EQ(1, constructor_test_value, 2); break;
> - CASE_TEST(linkage_errno); EXPECT_PTREQ(1, linkage_test_errno_addr(), &errno); break;
> - CASE_TEST(linkage_constr); EXPECT_EQ(1, linkage_test_constructor_test_value, 6); break;
> - case __LINE__:
> - return ret; /* must be last */
> - /* note: do not set any defaults so as to permit holes above */
> - }
> - }
> - return ret;
> + brk = sbrk(0);
> +
> + if (brk == (void *)-1)
> + brk = &end;
> +
> + return brk;
> }
>
> +TEST(startup, argc) { ASSERT_GE(test_argc, 1); }
> +TEST(startup, argv_addr) { ASSERT_GT((void *)test_argv, pbrk()); }
> +TEST(startup, argv_environ) { ASSERT_LT(test_argv, environ); }
> +TEST(startup, argv_total) { ASSERT_EQ(environ - test_argv - 1, test_argc ?: 1); }
> +TEST(startup, argv0_addr) { ASSERT_GT((void *)argv0, pbrk()); }
> +TEST(startup, argv0_str) { ASSERT_STRNZ((void *)argv0 > pbrk() ? argv0 : NULL); }
> +TEST(startup, argv0_len) { ASSERT_GE((void *)argv0 > pbrk() ? strlen(argv0) : 0U, 1U); }
> +TEST(startup, environ_addr) { ASSERT_GT((void *)environ, pbrk()); }
> +TEST(startup, environ_envp) { ASSERT_EQ(environ, test_envp); }
> +TEST(startup, environ_auxv) {
> + ASSUME_NOLIBC(return);
> + ASSERT_LT((void *)environ, (void *)_auxv);
> +}
> +TEST(startup, environ_total) {
> + ASSUME_NOLIBC(return);
> + /* kernel at least passes HOME and TERM, shell passes more */
> + ASSERT_GE((void *)_auxv - (void *)environ - 1, 2);
> +}
> +TEST(startup, environ_HOME) { ASSERT_NE(getenv("HOME"), NULL); }
> +TEST(startup, auxv_addr) {
> + ASSUME_NOLIBC(return);
> + ASSERT_GT((void *)_auxv, pbrk());
> +}
> +TEST(startup, auxv_AT_UID) { ASSERT_EQ(getauxval(AT_UID), getuid()); }
> +TEST(startup, constructor) { ASSERT_EQ(constructor_test_value, 2); }
> +TEST(startup, linkage_errno) { ASSERT_EQ(linkage_test_errno_addr(), &errno); }
> +TEST(startup, linkage_constr) { ASSERT_EQ(linkage_test_constructor_test_value, 6); }
I do appreciate the much lower indent level that still manages to
enumerate tests easily. But given that test suites are grouped, shouldn't
we go a bit further and state that TEST() operates on the suite defined
by the TEST_SUITE macro that must be defined before it ? This way you would
have:
#define TEST_SUITE startup
TEST(argc) { ASSERT_GE(test_argc, 1); }
TEST(argv_addr) { ASSERT_GT((void *)test_argv, pbrk()); }
...
#undef TEST_SUITE
One thing that was not immediately obvious to me upon first read was
if TEST() defines or executes a test (i.e. "test" is both a noun and a
verb). Of course, spending 10 more seconds on the patch makes it obvious
it's a definition, but maybe following the same logic we have with
run_test_suite(), we should place the verb in front, for example
"DEF_TEST()" which then makes it quite unambiguous. Any opinion ?
Willy
On 2023-11-16 08:33:27+0100, Willy Tarreau wrote:
> On Wed, Nov 15, 2023 at 10:08:20PM +0100, Thomas Weißschuh wrote:
> > Migrate part of nolibc-test.c to the new test harness.
> >
> > Signed-off-by: Thomas Weißschuh <[email protected]>
>
> A few points, mostly questions and food for thoughts.
>
> > -static void putcharn(char c, size_t n)
> > -{
> > - char buf[64];
> > -
> > - memset(buf, c, n);
> > - buf[n] = '\0';
> > - fputs(buf, stdout);
> > -}
> > -
>
> Ah now I see how the other one came from :-) My comment about the size
> check still stands anyway, especially when placed in an include file.
>
> > +#if defined(NOLIBC)
> > +
> > +#define ASSUME_NOLIBC(stmt)
> > +
> > +#else /* defined(NOLIBC) */
> > +
> > +/* differ from nolibc, both glibc and musl have no global _auxv */
> > +unsigned long *_auxv = (void *)-1;
> > +#define ASSUME_NOLIBC(stmt) SKIP(stmt)
> > +
> > +#endif /* defined(NOLIBC) */
> > +
>
> I've seen below how it's used and don't find this very clear. In general,
> passing a statement as an argument to a macro, especially control statements
> such as "return" is a bit difficult to grasp. If the macro is only used for
> this, maybe it should integrate the return statement and be called something
> like "RETURN_UNLESS_NOLIBC()" which is quite explicit this time. If you really
> need to keep the statement adjustable, then most likely that calling the
> macro "UNLESS_NOLIBC()" would help, because I understand more naturally
> that the following will perform a return if we're not on nolibc:
>
> UNLESS_NOLIBC(return);
>
> than:
>
> ASSUME_NOLIBC(return);
The statement arguments is modelled after SKIP() from
kselftest_harness.h.
But the wrapper you proposed is indeed much better,
I'll switch to that.
>
> > - for (test = min; test >= 0 && test <= max; test++) {
> > - int llen = 0; /* line length */
> > + if (brk)
> > + return brk;
> >
> > - /* avoid leaving empty lines below, this will insert holes into
> > - * test numbers.
> > - */
> > - switch (test + __LINE__ + 1) {
> > - CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
> > - CASE_TEST(argv_addr); EXPECT_PTRGT(1, test_argv, brk); break;
> > - CASE_TEST(argv_environ); EXPECT_PTRLT(1, test_argv, environ); break;
> > - CASE_TEST(argv_total); EXPECT_EQ(1, environ - test_argv - 1, test_argc ?: 1); break;
> > - CASE_TEST(argv0_addr); EXPECT_PTRGT(1, argv0, brk); break;
> > - CASE_TEST(argv0_str); EXPECT_STRNZ(1, argv0 > brk ? argv0 : NULL); break;
> > - CASE_TEST(argv0_len); EXPECT_GE(1, argv0 > brk ? strlen(argv0) : 0, 1); break;
> > - CASE_TEST(environ_addr); EXPECT_PTRGT(1, environ, brk); break;
> > - CASE_TEST(environ_envp); EXPECT_PTREQ(1, environ, test_envp); break;
> > - CASE_TEST(environ_auxv); EXPECT_PTRLT(test_auxv != (void *)-1, environ, test_auxv); break;
> > - CASE_TEST(environ_total); EXPECT_GE(test_auxv != (void *)-1, (void *)test_auxv - (void *)environ - 1, env_total); break;
> > - CASE_TEST(environ_HOME); EXPECT_PTRNZ(1, getenv("HOME")); break;
> > - CASE_TEST(auxv_addr); EXPECT_PTRGT(test_auxv != (void *)-1, test_auxv, brk); break;
> > - CASE_TEST(auxv_AT_UID); EXPECT_EQ(1, getauxval(AT_UID), getuid()); break;
> > - CASE_TEST(constructor); EXPECT_EQ(1, constructor_test_value, 2); break;
> > - CASE_TEST(linkage_errno); EXPECT_PTREQ(1, linkage_test_errno_addr(), &errno); break;
> > - CASE_TEST(linkage_constr); EXPECT_EQ(1, linkage_test_constructor_test_value, 6); break;
> > - case __LINE__:
> > - return ret; /* must be last */
> > - /* note: do not set any defaults so as to permit holes above */
> > - }
> > - }
> > - return ret;
> > + brk = sbrk(0);
> > +
> > + if (brk == (void *)-1)
> > + brk = &end;
> > +
> > + return brk;
> > }
> >
> > +TEST(startup, argc) { ASSERT_GE(test_argc, 1); }
> > +TEST(startup, argv_addr) { ASSERT_GT((void *)test_argv, pbrk()); }
> > +TEST(startup, argv_environ) { ASSERT_LT(test_argv, environ); }
> > +TEST(startup, argv_total) { ASSERT_EQ(environ - test_argv - 1, test_argc ?: 1); }
> > +TEST(startup, argv0_addr) { ASSERT_GT((void *)argv0, pbrk()); }
> > +TEST(startup, argv0_str) { ASSERT_STRNZ((void *)argv0 > pbrk() ? argv0 : NULL); }
> > +TEST(startup, argv0_len) { ASSERT_GE((void *)argv0 > pbrk() ? strlen(argv0) : 0U, 1U); }
> > +TEST(startup, environ_addr) { ASSERT_GT((void *)environ, pbrk()); }
> > +TEST(startup, environ_envp) { ASSERT_EQ(environ, test_envp); }
> > +TEST(startup, environ_auxv) {
> > + ASSUME_NOLIBC(return);
> > + ASSERT_LT((void *)environ, (void *)_auxv);
> > +}
> > +TEST(startup, environ_total) {
> > + ASSUME_NOLIBC(return);
> > + /* kernel at least passes HOME and TERM, shell passes more */
> > + ASSERT_GE((void *)_auxv - (void *)environ - 1, 2);
> > +}
> > +TEST(startup, environ_HOME) { ASSERT_NE(getenv("HOME"), NULL); }
> > +TEST(startup, auxv_addr) {
> > + ASSUME_NOLIBC(return);
> > + ASSERT_GT((void *)_auxv, pbrk());
> > +}
> > +TEST(startup, auxv_AT_UID) { ASSERT_EQ(getauxval(AT_UID), getuid()); }
> > +TEST(startup, constructor) { ASSERT_EQ(constructor_test_value, 2); }
> > +TEST(startup, linkage_errno) { ASSERT_EQ(linkage_test_errno_addr(), &errno); }
> > +TEST(startup, linkage_constr) { ASSERT_EQ(linkage_test_constructor_test_value, 6); }
>
> I do appreciate the much lower indent level that still manages to
> enumerate tests easily. But given that test suites are grouped, shouldn't
> we go a bit further and state that TEST() operates on the suite defined
> by the TEST_SUITE macro that must be defined before it ? This way you would
> have:
>
> #define TEST_SUITE startup
> TEST(argc) { ASSERT_GE(test_argc, 1); }
> TEST(argv_addr) { ASSERT_GT((void *)test_argv, pbrk()); }
> ...
> #undef TEST_SUITE
>
> One thing that was not immediately obvious to me upon first read was
> if TEST() defines or executes a test (i.e. "test" is both a noun and a
> verb). Of course, spending 10 more seconds on the patch makes it obvious
> it's a definition, but maybe following the same logic we have with
> run_test_suite(), we should place the verb in front, for example
> "DEF_TEST()" which then makes it quite unambiguous. Any opinion ?
The TEST() macro is modelled after kselftest_harness
(which only takes one argument, as it doesn't support suites)
and google test which works the same as the new TEST().
So I would prefer to keep the name.
As for specifying the suite via a macro:
I like that it saves even more indentation but at the same time it feels
a bit too implicit.
I'm not sure...