On Mon, Mar 16, 2020 at 03:56:44PM -0700, Jakub Kicinski wrote:
> Now that all tests have a fixture object move from a global
> list of tests to a list of tests per fixture.
>
> Order of tests may change as we will now group and run test
> fixture by fixture, rather than in declaration order.
>
> Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx>
> ---
> tools/testing/selftests/kselftest_harness.h | 32 +++++++++++++--------
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index 0f68943d6f04..36ab1b92eb35 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -660,8 +660,11 @@
> }
>
> /* Contains all the information about a fixture */
> +struct __test_metadata;
> +
Comment should be moved under this (it applies to __fixture_metadata not
__test_metadata).
> struct __fixture_metadata {
> const char *name;
> + struct __test_metadata *tests;
> struct __fixture_metadata *prev, *next;
> } _fixture_global __attribute__((unused)) = {
> .name = "global",
> @@ -696,7 +699,6 @@ struct __test_metadata {
> };
>
> /* Storage for the (global) tests to be run. */
> -static struct __test_metadata *__test_list;
> static unsigned int __test_count;
>
> /*
> @@ -710,8 +712,10 @@ static unsigned int __test_count;
> */
> static inline void __register_test(struct __test_metadata *t)
> {
> + struct __fixture_metadata *f = t->fixture;
> +
> __test_count++;
> - __LIST_APPEND(__test_list, t);
> + __LIST_APPEND(f->tests, t);
Not a big deal, but why not just "f->fixture->tests" here instead of a
separate variable?
> }
>
> static inline int __bail(int for_realz, bool no_print, __u8 step)
> @@ -724,14 +728,15 @@ static inline int __bail(int for_realz, bool no_print, __u8 step)
> return 0;
> }
>
> -void __run_test(struct __test_metadata *t)
> +void __run_test(struct __fixture_metadata *f,
> + struct __test_metadata *t)
> {
> pid_t child_pid;
> int status;
>
> t->passed = 1;
> t->trigger = 0;
> - printf("[ RUN ] %s.%s\n", t->fixture->name, t->name);
> + printf("[ RUN ] %s.%s\n", f->name, t->name);
> alarm(t->timeout);
> child_pid = fork();
> if (child_pid < 0) {
> @@ -781,13 +786,14 @@ void __run_test(struct __test_metadata *t)
> }
> }
> printf("[ %4s ] %s.%s\n", (t->passed ? "OK" : "FAIL"),
> - t->fixture->name, t->name);
> + f->name, t->name);
> alarm(0);
> }
>
> static int test_harness_run(int __attribute__((unused)) argc,
> char __attribute__((unused)) **argv)
> {
> + struct __fixture_metadata *f;
> struct __test_metadata *t;
> int ret = 0;
> unsigned int count = 0;
> @@ -796,13 +802,15 @@ static int test_harness_run(int __attribute__((unused)) argc,
> /* TODO(wad) add optional arguments similar to gtest. */
> printf("[==========] Running %u tests from %u test cases.\n",
> __test_count, __fixture_count + 1);
> - for (t = __test_list; t; t = t->next) {
> - count++;
> - __run_test(t);
> - if (t->passed)
> - pass_count++;
> - else
> - ret = 1;
> + for (f = __fixture_list; f; f = f->next) {
> + for (t = f->tests; t; t = t->next) {
> + count++;
> + __run_test(f, t);
> + if (t->passed)
> + pass_count++;
> + else
> + ret = 1;
> + }
> }
> printf("[==========] %u / %u tests passed.\n", pass_count, count);
> printf("[ %s ]\n", (ret ? "FAILED" : "PASSED"));
> --
> 2.24.1
>
But, with at least the first comment moved:
Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>
--
Kees Cook
