test: Allow EC unit test to use Ztest API

Provide a translation layer in test_utils.h for the Zephyr zassert
macros to map onto EC's TEST_ASSERT macros. With a little bit of
duplicated-but-not-quite-duplicated code (test_main vs. run_test)
and some extra macros for the return type of the test cases, the
tests can build for either the EC framework (by default) or the
Zephyr Ztest framework (#ifdef CONFIG_ZEPHYR).

Update the unit test documentation to explain why (and a brief
"how") developers should use the Ztest API for new unit tests.

BUG=b:168032590
BRANCH=none
TEST=`TEST_LIST_HOST=base32 make runhosttests`

Signed-off-by: Paul Fagerburg <pfagerburg@google.com>
Change-Id: I26e60788c1468e44fed565dd31162759c7587ea6
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2492527
Tested-by: Paul Fagerburg <pfagerburg@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>
Commit-Queue: Paul Fagerburg <pfagerburg@chromium.org>

3 files changed, 159 insertions, 22 deletions

diff --git a/docs/unit_tests.md b/docs/unit_tests.md
index 952d16b733..898c682fda 100644
--- a/docs/unit_tests.md
+++ b/docs/unit_tests.md
@@ -30,8 +30,9 @@ Build and run all unit tests:
 
 Unit tests live in the [`test`] subdirectory of the CrOS EC codebase.
 
-Test-related macros (e.g., `TEST_EQ`, `TEST_NE`) and functions are defined in
-[`test_util.h`].
+Existing EC unit tests will use the EC Test API, including test-related macros
+(e.g., `TEST_EQ`, `TEST_NE`) and functions defined in [`test_util.h`]. Note
+the `EC_TEST_RETURN` return type on the functions that are test cases.
 
 `test/my_test.c`:
 
@@ -45,7 +46,7 @@ static bool some_function(void)
 }
 
 /* Write a function with the following signature: */
-test_static int test_my_function(void)
+test_static EC_TEST_RETURN test_my_function(void)
 {
     /* Run some code */
     bool condition = some_function();
@@ -57,9 +58,73 @@ test_static int test_my_function(void)
 }
 ```
 
+New unit tests or significant changes to existing tests should use the Zephyr
+Ztest [API](https://docs.zephyrproject.org/latest/guides/test/ztest.html).
+
 `test/my_test.c`:
 
 ```c
+#include <stdbool.h>
+#include "test_util.h"
+
+static bool some_function(void)
+{
+    return true;
+}
+
+/* Write a function with the following signature: */
+test_static EC_TEST_RETURN test_my_function(void)
+{
+    /* Run some code */
+    bool condition = some_function();
+
+    /* Check that the expected condition is correct. */
+    zassert_true(condition, NULL);
+
+    return EC_SUCCESS;
+}
+```
+
+Note that the only difference between those two versions of `test/my_test.c`
+is the assertion:
+```c
+    TEST_EQ(condition, true, "%d");
+```
+versus
+```c
+    zassert_true(condition, NULL);
+```
+
+Currently, these tests using the Ztest API are still built with the EC test
+framework. [`test_util.h`] defines a mapping from the `zassert` macros to the
+EC `TEST_ASSERT` macros when `CONFIG_ZEPHYR` is not `#define`'d.
+
+Even though the tests are currently compiled only to the EC test framework,
+developers should still target the Ztest API for new unit tests. Future work
+will support building directly with the Ztest API. This makes the unit tests
+suitable for submitting upstream to the Zephyr project, and reduces the
+porting work when the EC transitions to the Zephyr RTOS. Similarly, when
+a development makes significant modifications to an existing unit test, they
+should consider porting the test to the Ztest API as part of the modifications.
+
+See [chromium:2492527](https://crrev.com/c/2492527) for a simple example of
+porting an EC unit test to the Ztest API.
+
+`test/my_test.c`:
+
+The EC test API enumerates the test cases using `RUN_TEST` in the `run_test`
+function, while the Ztest API enumerates the test cases using `ztest_unit_test`
+inside another macro for the test suite, inside of `test_main`.
+
+```c
+#ifdef CONFIG_ZEPHYR
+void test_main(void)
+{
+    ztest_test_suite(test_my_unit,
+             ztest_unit_test(test_my_function));
+    ztest_run_test_suite(test_my_unit);
+}
+#else
 /* The test framework will call the function named "run_test" */
 void run_test(int argc, char **argv)
 {
@@ -69,6 +134,7 @@ void run_test(int argc, char **argv)
     /* Report the results of all the tests at the end. */
     test_print_result();
 }
+#endif /* CONFIG_ZEPHYR */
 ```
 
 In the [`test`] subdirectory, create a `tasklist` file for your test that lists
diff --git a/include/test_util.h b/include/test_util.h
index 41299e09d3..d9195e4c97 100644
--- a/include/test_util.h
+++ b/include/test_util.h
@@ -8,6 +8,24 @@
 #ifndef __CROS_EC_TEST_UTIL_H
 #define __CROS_EC_TEST_UTIL_H
 
+#ifdef CONFIG_ZEPHYR
+
+#include <ztest_assert.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
+
+#else /* CONFIG_ZEPHYR */
+
 #include "common.h"
 #include "console.h"
 #include "stack_trace.h"
@@ -302,4 +320,40 @@ int test_detach_i2c(const int port, const uint16_t slave_addr_flags);
  */
 int test_attach_i2c(const int port, const uint16_t slave_addr_flags);
 
+/*
+ * We need these macros so that a test can be built for either Ztest or the
+ * EC test framework.
+ *
+ * EC unit tests return an EC_SUCCESS, or a failure code if one of the
+ * asserts in the test fails.
+ */
+#define EC_TEST_RETURN int
+
+/*
+ * Map the Ztest assertions onto EC assertions. There are two significant
+ * issues here.
+ * 1. zassert macros have extra printf-style arguments that the EC macros
+ * don't support, so we just have to drop that.
+ * 2. Some EC macros have an extra `fmt` parameter because they make their
+ * own printf-style string when the assertion fails. For some of them, we
+ * can add the correct format (the zassert_equal_ptr), but others we just
+ * don't know, so I'll just dump out the value in hex.
+ */
+#define zassert(cond, msg, ...) TEST_ASSERT(cond)
+#define zassert_unreachable(msg, ...) TEST_ASSERT(0)
+#define zassert_true(cond, msg, ...) TEST_ASSERT(cond)
+#define zassert_false(cond, msg, ...) TEST_ASSERT(!(cond))
+#define zassert_ok(cond, msg, ...) TEST_ASSERT(cond)
+#define zassert_is_null(ptr, msg, ...) TEST_ASSERT((ptr) == NULL)
+#define zassert_not_null(ptr, msg, ...) TEST_ASSERT((ptr) != NULL)
+#define zassert_equal(a, b, msg, ...) TEST_EQ((a), (b), "0x%x")
+#define zassert_not_equal(a, b, msg, ...) TEST_NE((a), (b), "0x%x")
+#define zassert_equal_ptr(a, b, msg, ...) \
+	TEST_EQ((void *)(a), (void *)(b), "0x%x")
+#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 433c874cba..54c5e15abc 100644
--- a/test/base32.c
+++ b/test/base32.c
@@ -11,7 +11,7 @@
 #include "test_util.h"
 #include "util.h"
 
-static int test_crc5(void)
+static EC_TEST_RETURN test_crc5(void)
 {
 	uint32_t seen;
 	int i, j, c;
@@ -25,7 +25,7 @@ static int test_crc5(void)
 		seen = 0;
 		for (j = 0; j < 32; j++)
 			seen |= 1 << crc5_sym(j, i);
-		TEST_ASSERT(seen == 0xffffffff);
+		zassert_equal(seen, 0xffffffff, NULL);
 	}
 
 	/*
@@ -36,7 +36,7 @@ static int test_crc5(void)
 		seen = 0;
 		for (j = 0; j < 32; j++)
 			seen |= 1 << crc5_sym(i, j);
-		TEST_ASSERT(seen == 0xffffffff);
+		zassert_equal(seen, 0xffffffff, NULL);
 	}
 
 	/* Transposing different symbols generates distinct CRCs */
@@ -49,7 +49,7 @@ static int test_crc5(void)
 			}
 		}
 	}
-	TEST_ASSERT(errors == 0);
+	zassert_equal(errors, 0, NULL);
 
 	return EC_SUCCESS;
 }
@@ -69,17 +69,17 @@ static int enctest(const void *src, int srcbits, int crc_every,
 	return 0;
 }
 
-#define ENCTEST(a, b, c, d) TEST_ASSERT(enctest(a, b, c, d) == 0)
+#define ENCTEST(a, b, c, d) zassert_equal(enctest(a, b, c, d), 0, NULL)
 
-static int test_encode(void)
+static EC_TEST_RETURN test_encode(void)
 {
 	const uint8_t src1[5] = {0xff, 0x00, 0xff, 0x00, 0xff};
 	char enc[32];
 
 	/* Test for enough space; error produces null string */
 	*enc = 1;
-	TEST_ASSERT(base32_encode(enc, 3, src1, 15, 0) == EC_ERROR_INVAL);
-	TEST_ASSERT(*enc == 0);
+	zassert_equal(base32_encode(enc, 3, src1, 15, 0), EC_ERROR_INVAL, NULL);
+	zassert_equal(*enc, 0, NULL);
 
 	/* Empty source */
 	ENCTEST("\x00", 0, 0, "");
@@ -104,9 +104,10 @@ static int test_encode(void)
 	/* CRC requires exact multiple of symbol count */
 	ENCTEST("\xff\x00\xff\x00\xff", 40, 4, "96ARU8AH9D");
 	ENCTEST("\xff\x00\xff\x00\xff", 40, 8, "96AR8AH9L");
-	TEST_ASSERT(
-	    base32_encode(enc, 16, (uint8_t *)"\xff\x00\xff\x00\xff", 40, 6)
-	    == EC_ERROR_INVAL);
+	zassert_equal(
+		base32_encode(enc, 16, (uint8_t *)"\xff\x00\xff\x00\xff",
+			40, 6),
+		EC_ERROR_INVAL, NULL);
 	/* But what matters is symbol count, not bit count */
 	ENCTEST("\xff\x00\xff\x00\xfe", 39, 4, "96ARU8AH8P");
 
@@ -139,15 +140,15 @@ static int dectest(const void *dec, int decbits, int crc_every, const char *enc)
 	int wantbits = decbits > 0 ? decbits : 5 * strlen(enc);
 	int gotbits = base32_decode(dest, destbits, enc, crc_every);
 
-	TEST_ASSERT(gotbits == wantbits);
+	zassert_equal(gotbits, wantbits, NULL);
 	if (gotbits != wantbits)
 		return -1;
 	return cmpbytes(dec, dest, (decbits + 7) / 8, "decode");
 }
 
-#define DECTEST(a, b, c, d) TEST_ASSERT(dectest(a, b, c, d) == 0)
+#define DECTEST(a, b, c, d) zassert_equal(dectest(a, b, c, d), 0, NULL)
 
-static int test_decode(void)
+static EC_TEST_RETURN test_decode(void)
 {
 	uint8_t dec[32];
 
@@ -165,7 +166,7 @@ static int test_decode(void)
 	DECTEST("\xff\x00\xff\x00\xff", 40, 0, " 96\tA-R\r8A H9\n");
 
 	/* Invalid symbol fails */
-	TEST_ASSERT(base32_decode(dec, 16, "AI", 0) == -1);
+	zassert_equal(base32_decode(dec, 16, "AI", 0), -1, NULL);
 
 	/* If dest buffer is big, use all the source bits */
 	DECTEST("", 0, 0, "");
@@ -186,18 +187,33 @@ static int test_decode(void)
 	DECTEST("\xff\x00\xff\x00\xff", 40, 8, "96AR8AH9L");
 
 	/* CRC requires exact multiple of symbol count */
-	TEST_ASSERT(base32_decode(dec, 40, "96ARL8AH9", 4) == -1);
+	zassert_equal(base32_decode(dec, 40, "96ARL8AH9", 4), -1, NULL);
 	/* But what matters is symbol count, not bit count */
 	DECTEST("\xff\x00\xff\x00\xfe", 39, 4, "96ARU8AH8P");
 
 	/* Detect errors in data, CRC, and transposition */
-	TEST_ASSERT(base32_decode(dec, 40, "96AQL", 4) == -1);
-	TEST_ASSERT(base32_decode(dec, 40, "96ARM", 4) == -1);
-	TEST_ASSERT(base32_decode(dec, 40, "96RAL", 4) == -1);
+	zassert_equal(base32_decode(dec, 40, "96AQL", 4), -1, NULL);
+	zassert_equal(base32_decode(dec, 40, "96ARM", 4), -1, NULL);
+	zassert_equal(base32_decode(dec, 40, "96RAL", 4), -1, NULL);
 
 	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)
+{
+	ztest_test_suite(test_base32_lib,
+			 ztest_unit_test(test_crc5),
+			 ztest_unit_test(test_encode),
+			 ztest_unit_test(test_decode));
+	ztest_run_test_suite(test_base32_lib);
+}
+#else
 void run_test(int argc, char **argv)
 {
 	test_reset();
@@ -208,3 +224,4 @@ void run_test(int argc, char **argv)
 
 	test_print_result();
 }
+#endif /* CONFIG_ZEPHYR */