summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Fagerburg <pfagerburg@google.com>2020-10-23 18:10:48 -0600
committerCommit Bot <commit-bot@chromium.org>2020-10-28 19:41:00 +0000
commit53962368a6c70d4e2e885df211c96950c8b26d25 (patch)
tree52be883057ca1b1383b77d2fdad1ef4ae45eb258
parent3c79b8fbb955f18fc02d1c33449056f0b154d6b5 (diff)
downloadchrome-ec-53962368a6c70d4e2e885df211c96950c8b26d25.tar.gz
test: support building base32 as a Zephyr test
With the Ztest API supported now, add the files to build the base32 unit test as a Zephyr test (instead of an EC test). BUG=b:168032590 BRANCH=None TEST=follow instructions in docs/ztest.md to build as a Zephyr test Signed-off-by: Paul Fagerburg <pfagerburg@google.com> Change-Id: I06dd7864f2de48aab5776950d4840f81728137f1 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2500465 Tested-by: Paul Fagerburg <pfagerburg@chromium.org> Reviewed-by: Jett Rink <jettrink@chromium.org> Commit-Queue: Paul Fagerburg <pfagerburg@chromium.org>
-rw-r--r--docs/sitemap.md1
-rw-r--r--docs/unit_tests.md88
-rw-r--r--docs/ztest.md166
-rw-r--r--zephyr/test/base32/CMakeLists.txt7
-rw-r--r--zephyr/test/base32/prj.conf6
5 files changed, 230 insertions, 38 deletions
diff --git a/docs/sitemap.md b/docs/sitemap.md
index 38049c1b7d..12f2f14b04 100644
--- a/docs/sitemap.md
+++ b/docs/sitemap.md
@@ -34,6 +34,7 @@
## Testing
* [Unit Tests](./unit_tests.md)
+ * [Porting EC unit tests to Ztest](./ztest.md)
* [Code Coverage](./code_coverage.md)
## Updaters
diff --git a/docs/unit_tests.md b/docs/unit_tests.md
index 898c682fda..eeb618fbb7 100644
--- a/docs/unit_tests.md
+++ b/docs/unit_tests.md
@@ -30,9 +30,20 @@ Build and run all unit tests:
Unit tests live in the [`test`] subdirectory of the CrOS EC codebase.
-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.
+All new unit tests should be written to use the Zephyr Ztest
+[API](https://docs.zephyrproject.org/latest/guides/test/ztest.html).
+If you are making significant changes to an existing test, you should also
+look at porting the test from the EC test API to the Ztest API.
+
+Using the Ztest API 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.
+
+### File headers
+
+Include [`test_util.h`] and any other required includes. In this example,
+the function being tested is defined in the test, but a real unit test would
+include the header file for the module that defines `some_function`.
`test/my_test.c`:
@@ -44,7 +55,17 @@ static bool some_function(void)
{
return true;
}
+```
+
+[`test_util.h`] includes `ztest.h` if `CONFIG_ZEPHYR` is defined,
+or defines a mapping from the `zassert` macros to the EC
+`TEST_ASSERT` macros if `CONFIG_ZEPHYR` is not defined.
+### Test cases
+
+Define the test cases. Use the `EC_TEST_RETURN` return type on these functions.
+
+```c
/* Write a function with the following signature: */
test_static EC_TEST_RETURN test_my_function(void)
{
@@ -52,26 +73,15 @@ test_static EC_TEST_RETURN test_my_function(void)
bool condition = some_function();
/* Check that the expected condition is correct. */
- TEST_EQ(condition, true, "%d");
+ zassert_true(condition, NULL);
return EC_SUCCESS;
}
```
-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)
{
@@ -79,43 +89,30 @@ test_static EC_TEST_RETURN test_my_function(void)
bool condition = some_function();
/* Check that the expected condition is correct. */
- zassert_true(condition, NULL);
+ TEST_EQ(condition, true, "%d");
return EC_SUCCESS;
}
```
-Note that the only difference between those two versions of `test/my_test.c`
-is the assertion:
+The only difference between those two versions of `test/my_test.c` is the
+assertion:
```c
- TEST_EQ(condition, true, "%d");
+ zassert_true(condition, NULL);
```
versus
```c
- zassert_true(condition, NULL);
+ TEST_EQ(condition, true, "%d");
```
-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`:
+### Specify the test cases to run
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`.
+`test/my_test.c`:
+
```c
#ifdef CONFIG_ZEPHYR
void test_main(void)
@@ -137,6 +134,12 @@ void run_test(int argc, char **argv)
#endif /* CONFIG_ZEPHYR */
```
+### Task List
+
+EC unit tests can run additional tasks besides the main test thread. The EC unit
+test implementation provides a phtreads-based implementation of the EC task API.
+We do not yet support running additional tasks in Ztest-based tests.
+
In the [`test`] subdirectory, create a `tasklist` file for your test that lists
the tasks that should run as part of the test:
@@ -149,7 +152,9 @@ the tasks that should run as part of the test:
#define CONFIG_TEST_TASK_LIST
```
-Add the test to the `Makefile`:
+### Makefile
+
+Add the test to the `Makefile` so that it can build as an EC unit test:
`test/build.mk`:
@@ -171,12 +176,18 @@ host-my_test
run-my_test
```
-Build and run the test:
+### Build and Run
+
+Build and run the test as an EC unit test:
```bash
(chroot) $ make run-my_test
```
+For building the test as a Zephyr Ztest unit test, follow the instructions in
+[Porting EC unit tests to Ztest](./ztest.md) to build the unit test for
+Zephyr's "native_posix" host-based target.
+
*** note
**TIP**: Unit tests should be independent from each other as much as possible.
This keeps the test (and any system state) simple to reason about and also
@@ -186,6 +197,7 @@ allows running unit tests in parallel. You can use the
## Mocks
+We do not yet support mocks for Zephyr Ztest-based tests.
[Mocks][`mock`] enable you to simulate behavior for parts of the system that
you're not directly testing. They can also be useful for testing specific edge
cases that are hard to exercise during normal use (e.g., error conditions).
diff --git a/docs/ztest.md b/docs/ztest.md
new file mode 100644
index 0000000000..2cb552e170
--- /dev/null
+++ b/docs/ztest.md
@@ -0,0 +1,166 @@
+# Porting EC unit tests to Ztest
+
+[TOC]
+
+This HOWTO shows the process for porting the EC's `base32` unit test to
+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
+porting an EC unit test to the Ztest API.
+
+## Determine source files being tested
+
+Determine which C files the unit test requires by finding the test in
+`test/test_config.h`:
+```
+#ifdef TEST_BASE32
+#define CONFIG_BASE32
+#endif
+```
+Locate the `CONFIG` item(s) in `common/build.mk`:
+```
+common-$(CONFIG_BASE32)+=base32.o
+```
+So for the `base32` test, we only need to shim `common/base32.c`.
+
+Add the C files to `zephyr/shim/CMakeLists.txt`, in the "Shimmed modules"
+section:
+
+```
+# Shimmed modules
+zephyr_sources_ifdef(CONFIG_PLATFORM_EC "${PLATFORM_EC}/common/base32.c")
+```
+
+Refer to [zephyr: shim in base32.c](https://crrev.com/c/2468631).
+
+## Create test directory
+
+Create a new directory for the unit test in `zephyr/test/base32`.
+
+Create `zephyr/test/base32/prj.conf` with these contents:
+```
+CONFIG_ZTEST=y
+CONFIG_PLATFORM_EC=y
+```
+
+Create `zephyr/test/base32/CMakeLists.txt` with these contents:
+```
+cmake_minimum_required(VERSION 3.13.1)
+project(base32)
+find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
+
+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.
+
+Change `RUN_TEST` to `ztest_unit_test` and add the `ztest_test_suite` wrapper
+plus the call to `ztest_run_test_suite`.
+
+```
+/*
+ * 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();
+
+ 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.
+
+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
+intelligent find-and-replace.
+
+* `TEST_ASSERT(n)` to `zassert_true(n, NULL)`
+* `TEST_EQ(a, b, fmt)` to `zassert_equal(a, b, fmt ## ", " ## fmt, a, b)`
+ * e.g. `TEST_EQ(a, b, "%d")` becomes `zassert_equal(a, b, "%d, %d", a, b)`
+* `TEST_NE(a, b, fmt)` to `zassert_not_equal(a, b, fmt ## ", " ## fmt, a, b)`
+* `TEST_LT(a, b, fmt)` to `zassert_true(a < b, fmt ## ", " ## fmt, a, b)`
+* `TEST_LE(a, b, fmt)` to `zassert_true(a <= b, fmt ## ", " ## fmt, a, b)`
+* `TEST_GT(a, b, fmt)` to `zassert_true(a > b, fmt ## ", " ## fmt, a, b)`
+* `TEST_GE(a, b, fmt)` tp `zassert_true(a >= b, fmt ## ", " ## fmt, a, b)`
+* `TEST_BITS_SET(a, bits)` to `zassert_true(a & (int)bits == (int)bits, "%u, %u", a & (int)bits, (int)bits)`
+* `TEST_BITS_CLEARED(a, bits)` to `zassert_true(a & (int)bits == 0, "%u, 0", a & (int)bits)`
+* `TEST_ASSERT_ARRAY_EQ(s, d, n)` to `zassert_mem_equal(s, d, b, NULL)`
+* `TEST_CHECK(n)` to `zassert_true(n, NULL)`
+* `TEST_NEAR(a, b, epsilon, fmt)` to `zassert_true(fabs(a-b) < epsilon, "%f, %f, %f", a, b, epsilon)`
+ * Currently, every usage of `TEST_NEAR` involves floating point values
+* `TEST_ASSERT_ABS_LESS(n, t)` to `zassert_true(abs(n) < t, "%d, %d", n, t)`
+ * Currently, every usage of `TEST_ASSERT_ANS_LESS` involves signed integers.
+
+There isn't a good replacement for `TEST_ASSERT_MEMSET(d, c, n)`, but it is
+only used in two tests, `printf.c` and `utils.c`. If you need this test,
+you'll need to code up a loop over the `n` bytes starting at `d`, and
+`zassert_equal` that each byte is equal to `c`.
+
+Also note that some tests use constructs like `TEST_ASSERT(var == const)`,
+which would have been better write as `TEST_EQ(var, const)`. These should be
+rewritten to use `zassert_equal`.
+
+Refer to
+[test: Allow EC unit test to use Ztest API](https://crrev.com/c/2492527) for
+the changes to the base32.c source code.
+
+## Build and run
+
+Use `cmake` and `ninja` to build 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
+UART_0 connected to pseudotty: /dev/pts/1
+*** Booting Zephyr OS build zephyr-v2.4.0-1-g63b2330a85cd ***
+Running test suite test_base32_lib
+===================================================================
+START - test_crc5
+ PASS - test_crc5
+===================================================================
+START - test_encode
+ PASS - test_encode
+===================================================================
+START - test_decode
+ PASS - test_decode
+===================================================================
+Test suite test_base32_lib succeeded
+===================================================================
+PROJECT EXECUTION SUCCESSFUL
+(cr) $
+```
+
diff --git a/zephyr/test/base32/CMakeLists.txt b/zephyr/test/base32/CMakeLists.txt
new file mode 100644
index 0000000000..3db4cee753
--- /dev/null
+++ b/zephyr/test/base32/CMakeLists.txt
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: Apache-2.0
+
+cmake_minimum_required(VERSION 3.13.1)
+project(base32)
+find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
+
+target_sources(app PRIVATE ${PLATFORM_EC}/test/base32.c)
diff --git a/zephyr/test/base32/prj.conf b/zephyr/test/base32/prj.conf
new file mode 100644
index 0000000000..4d4bf11d07
--- /dev/null
+++ b/zephyr/test/base32/prj.conf
@@ -0,0 +1,6 @@
+# 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.
+
+CONFIG_ZTEST=y
+CONFIG_PLATFORM_EC=y