common:test: refactor test_util.h to accommodate Zephyr

This change refactors test functionality in test_util.h to better
accomomdate zTests. This is done by:
* Removing the shim version of test_util.h. This was causing a
  conflict that made it harder to tell what's being used. This
  involved migrating some needed code over:
  - Defining different TASK_PARAMS.
  - Defining test_pass for Zephyr tests.
* Creating a macro (DECLARE_EC_TEST) that will automatically
  handle creating the individual test functions for both
  platform and Zephyr tests.
* Creating a macro (TEST_MAIN) that will automatically handle
  creating the main test entry function. This use to be
  test_main(void) for Zephyr and run_test(int, char**) for
  platform/ec. To do this we'll be removing the int, char**
  arguments from platform/ec. This may result in some tests
  having to be refactored, but overall should improve the
  test codebase as tests should remain deterministic (i.e.
  not depend on any outside arguments/parameters).
* Creating some common ztest_ function/macros that will
  allow writing platform/ec tests in a zephyr like style.
  see test/base32.c for an example.
* Update the type of __shared_mem_buf to match Zephyr. This
  was causing an issue now with the full test_util.h in
  zephyr/test/system/.

BRANCH=none
BUG=b:168032590
TEST=make runhosttests
TEST=zmake configure --test -B build/host/base32 zephyr/test/base32

Signed-off-by: Yuval Peress <peress@chromium.org>
Change-Id: I72173a3e94c7df09a2966e7ffeb9f5668d030f29
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2634401
Reviewed-by: Paul Fagerburg <pfagerburg@chromium.org>

6 files changed, 183 insertions, 93 deletions

diff --git a/common/test_util.c b/common/test_util.c
index 704fa1d018..8b3cbaad03 100644
--- a/common/test_util.c
+++ b/common/test_util.c
@@ -204,3 +204,20 @@ static int command_run_test(int argc, char **argv)
 }
 DECLARE_CONSOLE_COMMAND(runtest, command_run_test,
 			NULL, NULL);
+
+#ifndef CONFIG_ZEPHYR
+void z_ztest_run_test_suite(const char *name, struct unit_test *suite)
+{
+	test_reset();
+
+	while (suite->test) {
+		suite->setup();
+		RUN_TEST(suite->test);
+		suite->teardown();
+		suite++;
+	}
+
+	ccprintf("%s: ", name);
+	test_print_result();
+}
+#endif /* CONFIG_ZEPHYR */
diff --git a/docs/ztest.md b/docs/ztest.md
index 2cb552e170..6c67699aa9 100644
--- a/docs/ztest.md
+++ b/docs/ztest.md
@@ -7,7 +7,8 @@ Zephyr's Ztest framework. All of the work is done in `src/platform/ec`.
 
 See [Test Framework - Zephyr Project Documentation](https://docs.zephyrproject.org/1.12.0/subsystems/test/ztest.html#quick-start-unit-testing) for details about Zephyr's Ztest framework.
 
-See [chromium:2492527](https://crrev.com/c/2492527) for an example of
+See [chromium:2492527](https://crrev.com/c/2492527) and
+[chromium:2634401](https://crrev.com/c/2634401) for examples of
 porting an EC unit test to the Ztest API.
 
 ## Determine source files being tested
@@ -56,11 +57,8 @@ target_sources(app PRIVATE ${PLATFORM_EC}/test/base32.c)
 
 ### Modify test source code
 
-In the unit test, wrap `run_test` in the `#else` portion of an
-`#ifdef CONFIG_ZEPHYR`. Create `test_main` in the `#ifdef` portion.
-
-Copy the contents of `run_test` into `test_main`. You will need to keep the
-list of test cases in sync between the two functions.
+In the unit test, replace `run_test` with `TEST_MAIN()`. This will allow both
+platform/ec tests and Ztests to share the same entry point.
 
 Change `RUN_TEST` to `ztest_unit_test` and add the `ztest_test_suite` wrapper
 plus the call to `ztest_run_test_suite`.
@@ -71,8 +69,7 @@ plus the call to `ztest_run_test_suite`.
  * that Ztest uses, and again in the format the the EC test framework uses.
  * If you add a test to one of them, make sure to add it to the other.
  */
-#ifdef CONFIG_ZEPHYR
-void test_main(void)
+TEST_MAIN()
 {
 	ztest_test_suite(test_base32_lib,
 			 ztest_unit_test(test_crc5),
@@ -80,25 +77,11 @@ void test_main(void)
 			 ztest_unit_test(test_decode));
 	ztest_run_test_suite(test_base32_lib);
 }
-#else
-void run_test(int argc, char **argv)
-{
-	test_reset();
-
-	RUN_TEST(test_crc5);
-	RUN_TEST(test_encode);
-	RUN_TEST(test_decode);
-
-	test_print_result();
-}
-#endif /* CONFIG_ZEPHYR */
 ```
 
-Each function that is called by `ztest_unit_test` needs to change its
-return type to `EC_TEST_RETURN`. Keep the `return EC_SUCCESS;` at the end
-of the test function. If there are any `return` statements that return
-something other than `EC_SUCCESS`, you should use `ztest_test_fail` inside
-another `ifdef CONFIG_ZEPHYR` block.
+Each function that is called by `ztest_unit_test` needs to be declared using
+`DECLARE_EC_TEST`. Keep the `return EC_SUCCESS;` at the end
+of the test function.
 
 Change the `TEST_ASSERT` macros to `zassert` macros. There are plans to
 automate this process, but for now, it's a manual process involving some
@@ -136,15 +119,10 @@ the changes to the base32.c source code.
 
 ## Build and run
 
-Use `cmake` and `ninja` to build the test:
+Use `zmake` to build and run the test:
 ```
-(cr) $ export ZEPHYR_BASE=/mnt/host/source/src/third_party/zephyr/main/v2.4
-(cr) $ cd /mnt/host/source/src/platform/ec
-(cr) $ cmake -S zephyr/test/base32 -B build/base32 \
- -D ZEPHYR_MODULES=/mnt/host/source/src/platform/ec \
- -D ZEPHYR_TOOLCHAIN_VARIANT=host -D BOARD=native_posix -G Ninja
-(cr) $ ninja -C build/base32
-(cr) $ build/base32/zephyr/zephyr.exe
+(cr) $ zmake -l DEBUG configure --test -B build/ztest/base32 zephyr/test/base32
+...
 UART_0 connected to pseudotty: /dev/pts/1
 *** Booting Zephyr OS build zephyr-v2.4.0-1-g63b2330a85cd  ***
 Running test suite test_base32_lib
diff --git a/include/link_defs.h b/include/link_defs.h
index 4dc71d5f4f..360516eac1 100644
--- a/include/link_defs.h
+++ b/include/link_defs.h
@@ -106,7 +106,7 @@ extern const void *__irqhandler[];
 extern const struct irq_def __irq_data[], __irq_data_end[];
 
 /* Shared memory buffer.  Use via shared_mem.h interface. */
-extern uint8_t __shared_mem_buf[];
+extern char __shared_mem_buf[];
 
 /* Image sections used by the TPM2 library */
 extern uint8_t *__bss_libtpm2_start;
diff --git a/include/test_util.h b/include/test_util.h
index 60fa55b631..4cc150bb67 100644
--- a/include/test_util.h
+++ b/include/test_util.h
@@ -12,6 +12,11 @@
 #include "console.h"
 #include "stack_trace.h"
 
+#ifdef CONFIG_ZTEST
+#include <ztest.h>
+#include "ec_tasks.h"
+#endif /* CONFIG_ZTEST */
+
 /* This allows tests to be easily commented out in run_test for debugging */
 #define test_static static __attribute__((unused))
 
@@ -151,7 +156,11 @@ int test_fuzz_one_input(const uint8_t *data, unsigned int size);
 void test_reset(void);
 
 /* Reports test pass */
+#ifdef CONFIG_ZEPHYR
+#define test_pass ztest_test_pass
+#else
 void test_pass(void);
+#endif
 
 /* Reports test failure */
 void test_fail(void);
@@ -310,13 +319,150 @@ int test_attach_i2c(const int port, const uint16_t addr_flags);
  * EC test framework.
  *
  * EC unit tests return an EC_SUCCESS, or a failure code if one of the
- * asserts in the test fails.
+ * asserts in the test fails. This means that when building for Zephyr, we'll
+ * need to wrap the function returning an int with a Zephyr compatible void
+ * function. This function will simply test the result of the underlying
+ * function agains EC_SUCCESS.
+ *
+ * Usage:
+ * DECLARE_EC_TEST(test_it)
+ * {
+ *   ...
+ *   return EC_SUCCESS;
+ * }
  */
-#define EC_TEST_RETURN int
+#ifdef CONFIG_ZEPHYR
+#define DECLARE_EC_TEST(fname)                                                \
+	static int _stub_##fname(void);                                       \
+	static void fname(void)                                               \
+	{                                                                     \
+		zassert_equal(_stub_##fname(), EC_SUCCESS, #fname " failed"); \
+	}                                                                     \
+	static int _stub_##fname(void)
+#else
+#define DECLARE_EC_TEST(fname) static int fname(void)
+#endif
 
-/* An EC task only has one void parameter */
+/*
+ * Create a Zephyr compatible task function. An EC task only has one void
+ * parameter, while Zephyr takes in 3.
+ */
+#ifdef CONFIG_ZEPHYR
+#define TASK_PARAMS void *p1, void *p2, void *p3
+#else
 #define TASK_PARAMS void *p1
+#endif
+
+/*
+ * Create a TEST_MAIN macro to allow for Zephyr's test_main(void) to be used
+ * in Zephyr tests, while using run_test(int, char**) in platform/ec tests.
+ * This macro uses the lowest common denominator of the two (Zephyr) so tests
+ * that migrate from platform/ec to Zephyr will no longer be able to use the
+ * arguments (compile time checked).
+ *
+ * Usage:
+ * TEST_MAIN()
+ * {
+ *   ...
+ * }
+ */
+#ifdef CONFIG_ZEPHYR
+#define TEST_MAIN() void test_main(void)
+#else
+#define TEST_MAIN()                          \
+	void test_main(void);                \
+	void run_test(int argc, char **argv) \
+	{                                    \
+		test_main();                 \
+	}                                    \
+	void test_main(void)
+#endif
+
+/*
+ * Declare various Zephyr structs, functions, and macros so the same code can
+ * used in platform/ec tests.
+ */
+#ifndef CONFIG_ZEPHYR
+struct unit_test {
+	const char *name;
+	int (*test)(void);
+	void (*setup)(void);
+	void (*teardown)(void);
+};
+
+/**
+ * @brief void(*)(void) function that does nothing.
+ *
+ * This function should be used for setup or teardown when no work is required.
+ * Note that before_test() and after_test() will still be run to maintain
+ * compatibility.
+ */
+static inline void unit_test_noop(void)
+{
+}
+
+/**
+ * Create a unit test for a given function name with provided setup/teardown
+ * functions.
+ *
+ * @param fn The name of the function to run the test for (should be declared
+ * with DECLARE_EC_TEST).
+ * @param setup A function to call before this test function for setting data
+ * up.
+ * @param teardown A function to call after this test function for cleanup.
+ */
+#define ztest_unit_test_setup_teardown(fn, setup, teardown) \
+	{                                                   \
+		#fn, fn, setup, teardown                    \
+	}
+
+/**
+ * Create a unit test for a given function name with noop setup/teardown
+ * functions.
+ *
+ * @param fn The name of the function to run the test for (should be declared
+ * with DECLARE_EC_TEST).
+ * @see ztest_unit_test_setup_teardown
+ */
+#define ztest_unit_test(fn) \
+	ztest_unit_test_setup_teardown(fn, unit_test_noop, unit_test_noop)
+
+/**
+ * @brief Create a test suite
+ *
+ * Usage:
+ *   ztest_test_suite(my_tests,
+ *     ztest_unit_test(test0),
+ *     ztest_unit_test(test1));
+ *
+ * @param suite The name of the test suite (should be unique inside the given
+ * function).
+ */
+#define ztest_test_suite(suite, ...) \
+	static struct unit_test suite[] = { __VA_ARGS__, { 0 } }
+
+/**
+ * The primary entry point to run a test suite. This function should generally
+ * not be called directly, but should be invoked via
+ * ztest_run_test_suite(my_tests).
+ *
+ * @param name The name of the test suite.
+ * @param suite Pointer to the test suite array.
+ */
+void z_ztest_run_test_suite(const char *name, struct unit_test *suite);
+
+/**
+ * Run a test suite.
+ *
+ * Usage:
+ *   ztest_run_test_suite(my_tests);
+ *
+ * @param suite The name of the test suite to run.
+ */
+#define ztest_run_test_suite(suite) z_ztest_run_test_suite(#suite, suite)
+#endif /* CONFIG_ZEPHYR */
 
+#ifndef CONFIG_ZEPHYR
 /*
  * Map the Ztest assertions onto EC assertions. There are two significant
  * issues here.
@@ -341,5 +487,6 @@ int test_attach_i2c(const int port, const uint16_t addr_flags);
 #define zassert_within(a, b, d, msg, ...) TEST_NEAR((a), (b), (d), "0x%x")
 #define zassert_mem_equal(buf, exp, size, msg, ...) \
 	TEST_ASSERT_ARRAY_EQ(buf, exp, size)
+#endif /* CONFIG_ZEPHYR */
 
 #endif /* __CROS_EC_TEST_UTIL_H */
diff --git a/test/base32.c b/test/base32.c
index 54c5e15abc..faaefc266f 100644
--- a/test/base32.c
+++ b/test/base32.c
@@ -11,7 +11,7 @@
 #include "test_util.h"
 #include "util.h"
 
-static EC_TEST_RETURN test_crc5(void)
+DECLARE_EC_TEST(test_crc5)
 {
 	uint32_t seen;
 	int i, j, c;
@@ -71,7 +71,7 @@ static int enctest(const void *src, int srcbits, int crc_every,
 
 #define ENCTEST(a, b, c, d) zassert_equal(enctest(a, b, c, d), 0, NULL)
 
-static EC_TEST_RETURN test_encode(void)
+DECLARE_EC_TEST(test_encode)
 {
 	const uint8_t src1[5] = {0xff, 0x00, 0xff, 0x00, 0xff};
 	char enc[32];
@@ -148,7 +148,7 @@ static int dectest(const void *dec, int decbits, int crc_every, const char *enc)
 
 #define DECTEST(a, b, c, d) zassert_equal(dectest(a, b, c, d), 0, NULL)
 
-static EC_TEST_RETURN test_decode(void)
+DECLARE_EC_TEST(test_decode)
 {
 	uint8_t dec[32];
 
@@ -199,13 +199,7 @@ static EC_TEST_RETURN test_decode(void)
 	return EC_SUCCESS;
 }
 
-/*
- * Define the test cases to run. We need to do this twice, once in the format
- * that Ztest uses, and again in the format the the EC test framework uses.
- * If you add a test to one of them, make sure to add it to the other.
- */
-#ifdef CONFIG_ZEPHYR
-void test_main(void)
+TEST_MAIN()
 {
 	ztest_test_suite(test_base32_lib,
 			 ztest_unit_test(test_crc5),
@@ -213,15 +207,3 @@ void test_main(void)
 			 ztest_unit_test(test_decode));
 	ztest_run_test_suite(test_base32_lib);
 }
-#else
-void run_test(int argc, char **argv)
-{
-	test_reset();
-
-	RUN_TEST(test_crc5);
-	RUN_TEST(test_encode);
-	RUN_TEST(test_decode);
-
-	test_print_result();
-}
-#endif /* CONFIG_ZEPHYR */
diff --git a/zephyr/shim/include/test_util.h b/zephyr/shim/include/test_util.h
deleted file mode 100644
index cc28bcf697..0000000000
--- a/zephyr/shim/include/test_util.h
+++ /dev/null
@@ -1,34 +0,0 @@
-/* Copyright 2020 The Chromium OS Authors. All rights reserved.
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-/* Various utility for unit testing */
-
-#ifndef __CROS_EC_TEST_UTIL_H
-#define __CROS_EC_TEST_UTIL_H
-
-#include <ztest.h>
-#include "ec_tasks.h"
-
-/*
- * We need these macros so that a test can be built for either Ztest or the
- * EC test framework.
- *
- * Ztest unit tests are void and do not return a value. In the EC framework,
- * if none of the assertions fail, the test is supposed to return EC_SUCCESS,
- * so just define that as empty and `return EC_SUCCESS;` will get pre-processed
- * into `return ;`
- */
-#define EC_TEST_RETURN void
-#define EC_SUCCESS
-#define test_pass ztest_test_pass
-
-/* Zephyr threads have three void pointers as parameters */
-#define TASK_PARAMS void *p1, void *p2, void *p3
-
-uint32_t prng(uint32_t seed);
-
-uint32_t prng_no_seed(void);
-
-#endif /* __CROS_EC_TEST_UTIL_H */