[PATCH v2 2/5] lib: Add SBIUnit testing macros and functions
Andrew Jones
ajones at ventanamicro.com
Wed Feb 28 06:19:20 PST 2024
On Thu, Feb 15, 2024 at 04:16:22PM +0000, Ivan Orlov wrote:
> This patch introduces all of the SBIUnit macros and functions which
> can be used during the test development process. Also, it defines
> the 'run_all_tests' function, which is being called during the
> 'init_coldboot' right after printing the boot hart information.
>
> Also, add the CONFIG_SBIUNIT Kconfig entry in order to be able to
> turn the tests on and off. When the CONFIG_SBIUNIT is disabled,
> the tests and all related code is excluded completely on the
> compilation stage.
>
> Signed-off-by: Ivan Orlov <ivan.orlov0322 at gmail.com>
> ---
> V1 -> V2:
> - Rename the sources to have 'test' in the names: 'sbi_unit.c' =>
> 'sbi_unit_test.c', 'sbi_unit.h' => 'sbi_unit_test.h'.
> - Rewrite using the carray functionality instead of placing the
> pointers to the test suites into the source explicitly.
> - Add 'ifdef' into the sbi_unit_test header file so we could
> avoid writing a lot of 'ifdef' in other files.
> - Get rid of unused symbols and macros
> - Change the behaviour of SBIUNIT_ASSERT_* functions: now they trigger
> sbi_panic if the assertion fails.
> - Add a 'SBIUnit' prefix to the SBIUNIT_INFO macro message
> - Rename the sbiunit_test_case structure fields, and use true/false
> instead of 1/0
> - Change the logic of the test fail detection: now we have the 'failed'
> sbiunit_test_case structure field instead of the 'result'. 'failed'
> field gets set to 'true' if an assertion or an expectaion fails.
> - Fix codestyle issues
> - Add 'len' parameter to SBIUNIT_*_STREQ
>
> include/sbi/sbi_unit_test.h | 74 +++++++++++++++++++++++++++++++++++
> lib/sbi/Kconfig | 4 ++
> lib/sbi/objects.mk | 2 +
> lib/sbi/sbi_init.c | 3 ++
> lib/sbi/sbi_unit_test.c | 43 ++++++++++++++++++++
> lib/sbi/sbi_unit_tests.carray | 3 ++
> 6 files changed, 129 insertions(+)
> create mode 100644 include/sbi/sbi_unit_test.h
> create mode 100644 lib/sbi/sbi_unit_test.c
> create mode 100644 lib/sbi/sbi_unit_tests.carray
>
> diff --git a/include/sbi/sbi_unit_test.h b/include/sbi/sbi_unit_test.h
> new file mode 100644
> index 0000000..1538332
> --- /dev/null
> +++ b/include/sbi/sbi_unit_test.h
> @@ -0,0 +1,74 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Author: Ivan Orlov <ivan.orlov0322 at gmail.com>
> + */
> +#ifdef CONFIG_SBIUNIT
> +#ifndef __SBI_UNIT_H__
> +#define __SBI_UNIT_H__
> +
> +#include <sbi/sbi_types.h>
> +#include <sbi/sbi_console.h>
> +#include <sbi/sbi_string.h>
> +
> +struct sbiunit_test_case {
> + const char *name;
> + bool enabled;
> + bool failed;
> + void (*test_func)(struct sbiunit_test_case *test);
> + void (*onerr)(struct sbiunit_test_case *test);
Please rename to on_error()
> +};
> +
> +struct sbiunit_test_suite {
> + const char *name;
> + struct sbiunit_test_case *cases;
> +};
> +
> +#define SBIUNIT_TEST_CASE(func) \
> + { \
> + .name = #func, \
> + .enabled = true, \
> + .failed = false, \
> + .test_func = (func) \
> + }
> +
> +#define SBIUNIT_TEST_SUITE(suite_name, cases_arr) \
> + struct sbiunit_test_suite suite_name = { \
> + .name = #suite_name, \
> + .cases = cases_arr \
> + }
> +
> +#define _sbiunit_msg(test, msg) "[SBIUnit] [%s:%d]: %s: %s", __FILE__, \
> + __LINE__, test->name, msg
> +
> +#define SBIUNIT_INFO(test, msg) sbi_printf(_sbiunit_msg(test, msg))
> +#define SBIUNIT_PANIC(test, msg) sbi_panic(_sbiunit_msg(test, msg))
> +
> +#define SBIUNIT_EXPECT(test, cond) do { \
> + if (!(cond)) { \
> + test->failed = true; \
> + SBIUNIT_INFO(test, "Condition \"" #cond "\" expected to be true!\n"); \
> + } \
> +} while (0)
> +
> +#define SBIUNIT_ASSERT(test, cond) do { \
> + if (!(cond)) { \
> + test->failed = true; \
No need to set failed since the next line calls panic.
> + SBIUNIT_PANIC(test, "Condition \"" #cond "\" must be true!\n"); \
> + } \
> +} while (0)
> +
> +#define SBIUNIT_EXPECT_EQ(test, a, b) SBIUNIT_EXPECT(test, (a) == (b))
> +#define SBIUNIT_ASSERT_EQ(test, a, b) SBIUNIT_ASSERT(test, (a) == (b))
> +#define SBIUNIT_EXPECT_NE(test, a, b) SBIUNIT_EXPECT(test, (a) != (b))
> +#define SBIUNIT_ASSERT_NE(test, a, b) SBIUNIT_ASSERT(test, (a) != (b))
> +#define SBIUNIT_EXPECT_MEMEQ(test, a, b, len) SBIUNIT_EXPECT(test, !sbi_memcmp(a, b, len))
> +#define SBIUNIT_ASSERT_MEMEQ(test, a, b, len) SBIUNIT_ASSERT(test, !sbi_memcmp(a, b, len))
> +#define SBIUNIT_EXPECT_STREQ(test, a, b, len) SBIUNIT_EXPECT(test, !sbi_strncmp(a, b, len))
> +#define SBIUNIT_ASSERT_STREQ(test, a, b, len) SBIUNIT_ASSERT(test, !sbi_strncmp(a, b, len))
> +
> +void run_all_tests(void);
> +#endif
> +#else
> +#define run_all_tests()
> +#endif
> diff --git a/lib/sbi/Kconfig b/lib/sbi/Kconfig
> index 81dd2db..e3038ee 100644
> --- a/lib/sbi/Kconfig
> +++ b/lib/sbi/Kconfig
> @@ -50,4 +50,8 @@ config SBI_ECALL_DBTR
> bool "Debug Trigger Extension"
> default y
>
> +config SBIUNIT
> + bool "Enable SBIUNIT tests"
> + default n
> +
> endmenu
> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> index 0a50e95..08959f1 100644
> --- a/lib/sbi/objects.mk
> +++ b/lib/sbi/objects.mk
> @@ -11,6 +11,8 @@ libsbi-objs-y += riscv_asm.o
> libsbi-objs-y += riscv_atomic.o
> libsbi-objs-y += riscv_hardfp.o
> libsbi-objs-y += riscv_locks.o
> +libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o
> +libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o
>
> libsbi-objs-y += sbi_ecall.o
> libsbi-objs-y += sbi_ecall_exts.o
> diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
> index 804b01c..796cccc 100644
> --- a/lib/sbi/sbi_init.c
> +++ b/lib/sbi/sbi_init.c
> @@ -29,6 +29,7 @@
> #include <sbi/sbi_timer.h>
> #include <sbi/sbi_tlb.h>
> #include <sbi/sbi_version.h>
> +#include <sbi/sbi_unit_test.h>
>
> #define BANNER \
> " ____ _____ ____ _____\n" \
> @@ -398,6 +399,8 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
>
> sbi_boot_print_hart(scratch, hartid);
>
> + run_all_tests();
> +
> /*
> * Configure PMP at last because if SMEPMP is detected,
> * M-mode access to the S/U space will be rescinded.
> diff --git a/lib/sbi/sbi_unit_test.c b/lib/sbi/sbi_unit_test.c
> new file mode 100644
> index 0000000..f4988d8
> --- /dev/null
> +++ b/lib/sbi/sbi_unit_test.c
> @@ -0,0 +1,43 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Author: Ivan Orlov <ivan.orlov0322 at gmail.com>
> + */
> +#include <sbi/sbi_unit_test.h>
> +#include <sbi/sbi_types.h>
> +#include <sbi/sbi_console.h>
> +
> +extern struct sbiunit_test_suite *sbi_unit_tests[];
> +extern unsigned long sbi_unit_tests_size;
> +
> +static void run_test_suite(struct sbiunit_test_suite *suite)
> +{
> + struct sbiunit_test_case *s_case;
> + u32 count_pass = 0, count_fail = 0;
> +
> + sbi_printf("## Running test suite: %s\n", suite->name);
> +
> + s_case = suite->cases;
> + while (s_case->enabled) {
Can't test cases be enabled/disabled sparsely? Shouldn't we instead
loop while s_case->name is not NULL? And we need to ensure the cases
array always has an empty case at the end.
> + s_case->test_func(s_case);
> + if (s_case->failed)
> + count_fail++;
> + else
> + count_pass++;
> + sbi_printf("[%s] %s\n", s_case->failed ? "FAILED" : "PASSED",
> + s_case->name);
> + s_case++;
> + }
> + sbi_printf("%u PASSED / %u FAILED / %u TOTAL\n", count_pass, count_fail,
> + count_pass + count_fail);
> +}
> +
> +void run_all_tests(void)
> +{
> + u32 i;
> +
> + sbi_printf("\n# Running SBIUNIT tests #\n");
> +
> + for (i = 0; i < sbi_unit_tests_size; i++)
> + run_test_suite(sbi_unit_tests[i]);
> +}
> diff --git a/lib/sbi/sbi_unit_tests.carray b/lib/sbi/sbi_unit_tests.carray
> new file mode 100644
> index 0000000..8d6069b
> --- /dev/null
> +++ b/lib/sbi/sbi_unit_tests.carray
> @@ -0,0 +1,3 @@
> +HEADER: sbi/sbi_unit_test.h
> +TYPE: struct sbiunit_test_suite
> +NAME: sbi_unit_tests
> --
> 2.34.1
>
Thanks,
drew
More information about the opensbi
mailing list