From 1b684a8e0473eff0f29ecabdc70fac3cf378d2bd Mon Sep 17 00:00:00 2001 From: Yuval Peress Date: Tue, 11 Oct 2022 12:58:11 -0600 Subject: docs: Update Zephyr testing documentation Update the documentation for testing on Zephyr to reflect the latest tools and features. Removed docs/zephyr/zephyr_testing.md since it was only intended to be used for the fix-it week. BRANCH=none BUG=none TEST=none Signed-off-by: Yuval Peress Change-Id: I54cb53af7d051e2ea1c3813f3907773c4e52f83a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3945693 Reviewed-by: Keith Short Commit-Queue: Keith Short --- docs/sitemap.md | 4 +- docs/zephyr/zephyr_testing.md | 53 ------- docs/zephyr/ztest.md | 360 ++++++++++++++++++++++-------------------- 3 files changed, 189 insertions(+), 228 deletions(-) delete mode 100644 docs/zephyr/zephyr_testing.md diff --git a/docs/sitemap.md b/docs/sitemap.md index 55df721854..cac7d4d1d4 100644 --- a/docs/sitemap.md +++ b/docs/sitemap.md @@ -36,7 +36,7 @@ ## Testing * [Unit Tests](./unit_tests.md) - * [Porting EC unit tests to Ztest](./zephyr/ztest.md) + * [Zephyr Testing](./zephyr/ztest.md) * [Code Coverage](./code_coverage.md) ## Updaters @@ -65,7 +65,7 @@ * [Initialization Order](./zephyr/zephyr_init.md) * [Proof-of-Concept-Device Bringup](./zephyr/zephyr_poc_device_bringup.md) * [Shimming](./zephyr/zephyr_shim.md) -* [Porting EC unit tests to Ztest](./zephyr/ztest.md) +* [Zephyr Testing](./zephyr/ztest.md) ## Miscellaneous diff --git a/docs/zephyr/zephyr_testing.md b/docs/zephyr/zephyr_testing.md deleted file mode 100644 index 5f5987686c..0000000000 --- a/docs/zephyr/zephyr_testing.md +++ /dev/null @@ -1,53 +0,0 @@ -# Zephyr Testing Resources - -This is a compilation of resources for developers participating in the Zephyr -EC Fix-it Week, running from 15 - 19 Aug 2022. - -Please note: many of the links in this document will only be accessible to -Googlers. - -[TOC] - -## Introductory materials - - * [Fix-it week Training Presentation](https://goto.google.com/cros-ec-fixit-week-presentation) - - by Yuval Peress (Start here - Googlers only) - * Ask questions during the live presentation using the - [Dory](https://goto.google.com/cros-ec-fixit-week-dory) (Google only) - * Sample CLs for your reference: - * [Writing a new emulator](https://crrev.com/c/2903206) - * [Writing a console command test ](https://crrev.com/c/3594484) - * [Writing a host command test](https://crrev.com/c/3530114) - * [Using test fixtures and local FFF mocks](https://crrev.com/c/3607055) - * [Defining global FFF mocks](https://crrev.com/c/3252365) - * [Resetting global FFF mocks](https://crrev.com/c/3500299) - -## Finding Work to do - -We have assembled a [hotlist](http://b/hotlists/4300616) of low-coverage areas -in the codebase. Please remember to assign bugs to yourself to avoid duplicate -work being performed and do not take bugs until you are ready to actively work -on it. - -We also encourage you to check our coverage reports to identify files needing -additional test coverage: - - * [Gitlab coverage reports](https://gitlab.com/zephyr-ec/ec/-/jobs/artifacts/main/file/build/all_builds_filtered_rpt/index.html?job=merged_coverage) - * [Internal Code Search](https://goto.google.com/cros-ec-fixit-week-cs) - (enable the coverage layer - Googlers only) - -## Submitting tests for review - -The fastest way to have your code reviewed is to add -`zephyr-test-eng@google.com` to your CL. This will randomly assign a -member of the Zephyr EC Testing team to your CL. The team will be monitoring -Gerrit extra closely during Fix-it week to streamline reviews. Please do _not_ -send CLs directly to individuals or to the wider Zephyr reviewers group. - -## Getting Help - -Questions on writing, running, and debugging tests should be asked in [YAQS with -the zephyr-rtos-test topic](https://goto.google.com/cros-ec-fixit-week-yaqs). -Part of our goal during Fix-it Week is to assemble a knowledge base of testing -information. Your questions and the resulting dialogue will be very beneficial -to future developers, so please ask away! (Googlers only) diff --git a/docs/zephyr/ztest.md b/docs/zephyr/ztest.md index d782631cc3..437c79d12e 100644 --- a/docs/zephyr/ztest.md +++ b/docs/zephyr/ztest.md @@ -1,199 +1,213 @@ -# Porting EC unit tests to Ztest +# Zephyr Testing [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. - -For examples of porting an EC unit test to the Ztest API, see: * -[base32](https://crrev.com/c/2492527) and -[improvements](https://crrev.com/c/2634401) * -[accel_cal](https://crrev.com/c/2645198) - -## Porting Considerations - -Not every EC unit test can be ported to Ztest. This section describes cases that -are not supported and cases where caveats apply. - -### EC Mocks Are Not Supported - -If a test has a `$TEST.mocklist` file associated with the unit test, it is using -the EC mocking framework, which is unsupported in the Ztest framework. Ztest has -its own mocking framework which the EC does not support. - -### Multiple Task Caveats - -The EC unit test framework starts a single task to call `run_test`, and this -task will then call the functions for the various test cases. Some unit tests -have multiple threads of execution, which is enabled by a `$TEST.tasklist` file -associated with the unit test. The test runner task has a task ID of -`TASK_ID_TEST_RUNNER`, which can be used as an argument to any of the task -functions. See for example the -[`charge_ramp` unit test](https://chromium.googlesource.com/chromiumos/platform/ec/+/HEAD/test/charge_ramp.c#81) -and the -[`host_command` unit test](https://chromium.googlesource.com/chromiumos/platform/ec/+/HEAD/test/host_command.c#32) - -When a unit test is ported to Ztest, `test_main` doesn't have a thread ID, so -`TASK_ID_TEST_RUNNER` is undefined. `charge_ramp` and `host_command` cannot be -ported at this time. `test_main` also cannot call any of the task functions that -must be called from a task, such as `task_wake`; these functions can pend the -calling task, but since `test_main` doesn't have a thread ID, the pend will -fail. See the -[`mutex` unit test](https://chromium.googlesource.com/chromiumos/platform/ec/+/HEAD/test/mutex.c#116) -for an example. - -## 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 +Before starting, developers should read the following: +- [Zephyr Test Framework](https://docs.zephyrproject.org/latest/develop/test/ztest.html) +- [Zephyr Test Runner (Twister)](https://docs.zephyrproject.org/latest/develop/test/twister.html) + +## Getting help + +Get your tests reviewed by adding [zephyr-test-eng@google.com](mailto:zephyr-test-eng@google.com) +as a reviewer on your CL. + +Ask a question: +- If you're a googler, please post using our YAQS label [zephyr-rtos-test](http://yaqs/eng/t/zephyr-rtos-test) +- External contributors should email to [zephyr-test-eng@google.com](mailto:zephyr-test-eng@google.com) + or ask on the public Zephyr discord [#testing](https://discord.com/channels/720317445772017664/733037944922964069) + channel for generic Zephyr questions. + +## Where to add tests? + +Tests currently reside in [zephyr/test](https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/test/). +When adding a new test it's possible to either create a new directory which +includes a `testcase.yaml` or add a new entry to an existing `testcase.yaml` if +the test is a slight variation of an existing test. + +### How to decide where your tests should go? + +If you're adding a new compilational unit, check your dependencies. Is this unit +tied to an existing Kconfig? If so, it's possible that another test is already +enabling it and linking your `.c` file. It's probably worth your time to simply +add a new test suite to that existing test binary, or at the very least, +creating a variant of the binary by adding a new entry in the `testcase.yaml` +file. + +If this doesn't work, it's possible that you might need to create a new test +from scratch. First, decide if this will be an integration test or a unit test. +Integration tests will build the full system (see below) and thus are sometimes +easier to set up (since they look and feel like an application). For the same +reason that makes them easier to set up, the lack of mocks can make things more +difficult when trying to force execution down a specific code path. + +In the case of integration tests, any test under `zephyr/test` will serve as a +good example. Alternatively, an example of a unit test can be found under +[common/spi/flash_reg](https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/common/spi/flash_reg/). + +## The structure of the `testcase.yaml` + +The top level `testcase.yaml` uses the `test:` block to define the attributes +for each test case. The optional `common:` block uses the same attributes as the +test cases, but the attributes are applied to all test cases in the file. See +[here](https://docs.zephyrproject.org/latest/develop/test/twister.html#test-cases) +for more details. + +Some common attributes include: +- `extra_configs` which is a list of Kconfigs to add to the test. +- `extra_args` which is a string containing additional arguments to pass to + CMake + +## Integration tests + +Integration tests build the full EC. They require devicetree and all Kconfigs to +be set appropriately. To build an integration test, simply use the following +CMakeLists.txt template: + +```cmake +cmake_minimum_required(VERSION 3.20.0) +find_package(Zephyr REQUIRED HITS $ENV{ZEPHYR_BASE}) +project() +target_sources(app + PRIVATE + src/... +) + +# Add global FFF declaration +add_subdirectory(${PLATFORM_EC}/zephyr/test/test_utils test_utils) ``` -Locate the `CONFIG` item(s) in `common/build.mk`: +Then, you'll need a default `prj.conf` file to set the default configs. Keep in +mind that this file will be automatically included in all the tests unless it is +overridden by the test's `extra_args` so you only want to put things that will +apply to all tests here. It will look like: ``` -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: +CONFIG_ZTEST=y +CONFIG_ZTEST_NEW_API=y +# Enable the shim of the legacy CrosEC (You’ll need both) +CONFIG_CROS_EC=y +CONFIG_PLATFORM_EC=y ``` -# 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 +## Mocking + +We're using [FFF](http://github.com/meekrosoft/fff) for our mocking framework. +For most use cases these are the things to remember: +1. Reset your fakes in your test suite's `before` function using `RESET_FAKE()`. + If multiple suites need to reset fakes, consider using a + [test rule](https://docs.zephyrproject.org/latest/develop/test/ztest.html#test-rules). +2. You'll need *1* instance of `DEFINE_FFF_GLOBALS;` in your binary. This is + done in the sample above via including `test_utils` into your binary as a + subdirectory. +3. Use C++ for better `custom_fake` handling by setting the following before + including `fff.h`: + ```c + #define CUSTOM_FFF_FUNCTION_TEMPLATE(RETURN, FUNCNAME, ...) \ + std::function FUNCNAME + ``` + This will enable the following: + ```c + ZTEST_F(suite, test_x) + { + my_function_fake.custom_fake = [fixture]() { + /* Capturing lambda has access to 'fixture' */ + }; + ... + } + ``` + + +## Running twister + +Run all tests under a specific directory: + +```shell +platform/ec$ ./twister -T path/to/my/tests ``` -Create `zephyr/test/base32/CMakeLists.txt` with these contents: +Run a specific test: +```shell +platform/ec$ ./twister -s path/to/my/tests/my.test.case +``` +Run all tests with coverage (get more info on code coverage at +[Zephyr ztest code coverage](../code_coverage.md#Zephyr_ztest_code_coverage): +```shell +platform/ec$ ./twister -p native_posix -p unit_testing --coverage ``` -target_sources(app PRIVATE ${PLATFORM_EC}/test/base32.c) + +Get more info on twister: +```shell +platform/ec$ ./twister --help ``` -## Modify test source code +Other useful flags: +- `-i` Print inline logs to STDOUT on error (as well as log file) +- `-n` No clean (incremental builds) +- `-c` Clobber, don't create a new `twister-out/` directory +- `-v` Verbose logging (can use `-vv` or `-vvv` for more logging) +- `-b` Build only + +## Using assumptions -### Test cases +The `zassume_*` API is used to minimize failures while allowing us to find out +exactly what's going wrong. When writing a test, only assert on code you're +testing. Any dependencies should use the `zassume_*` API. If the assumption +fails, your test will be marked as skipped and Twister will report an error. +Generally speaking, if an assumption fails, either the test wasn't set up +correctly, or there should be another test that's testing the dependency. -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. +### Example: when to use an assumption -Change `RUN_TEST` to `ztest_unit_test` and add the `ztest_test_suite` wrapper -plus the call to `ztest_run_test_suite`. +In a given project layout we might have several components (A, B, C, and D). In +this scenario, components B, C, and D all depend on A to function. In each test +for B, C, and D we'll include the following: ```c -/* - * 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. - */ -TEST_MAIN() +static void test_suite_before(void *f) { - 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); + struct my_suite_fixture *fixture = f; + + zassume_ok(f->a->init(), "Failed to initialize A, see test suite 'a_init'"); } ``` -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. Note that for the EC build, `TEST_MAIN` will call `test_reset` before -running the test cases, and `test_print_result` after. - -### Assert macros - -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_NEAR(a, b, epsilon, fmt)` to `zassert_within(a, b, epsilon, fmt, a)` - * 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. - -### Tasklist - -For any test that has a corresponding `${TESTNAME}.tasklist`, add the file -`shimmed_test_tasks.h` in the zephyr test directory, and in that file, -`#include` the tasklist file. See [accel_cal](https://crrev.com/c/2645198) for -an example. - -Add `CONFIG_HAS_TEST_TASKS=y` to the `prj.conf` file, as well as the appropriate -`CONFIG_PLATFORM_EC` defines to include or exclude code that the unit under test -uses. - -## Build and run - -Use `zmake` to build and run the test: - -``` -(cr) $ zmake -l DEBUG test 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 -=================================================================== -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) $ +The above will call A's init function and assume that it returned 0 (status OK). +If this assumption fails, then B, C, and D will all be marked as skipped along +with a log message telling us to look at the test suite 'a_init'. + +Key takeaways: +1. If it's code that you depend on (module/library/logic), use assume. It's not + what you're testing, you shouldn't raise false failures. +2. Document why/where you believe that the logic should have been tested. If we + end up skipping this test, we should know where the tests that should have + caught the error belong. + +## Debugging + +If the test targets `native_posix` or `unit_testins` platforms, you can run it +through your choice of debugger (lldb is provided in the chroot). Additionally, +it's possible to run only a subset of the tests via: + +```shell +$ ./twister-out/native_posix/zephyr/test/.../zephyr/zephyr.exe -list +# List of all tests in the binary in the format of +$ ./twister-out/native_posix/zephyr/test/.../zephyr/zephyr.exe -test=suite0:*,suite1:test0 +# Runs all tests under suite0 and test0 from suite1 +$ lldb ./twister-out/unit_testing/.../testbinary +# Starts lldb for the test binary ``` + +## Deflaking + +zTest allows deflaking tests via shuffling. To enable this feature, simply turn +on `CONFIG_ZTEST_SHUFFLE=y`. Your test binary's test suites and test order will +be random as well as run multiple times. To fine-tune the execution: +- Set `CONFIG_ZTEST_SHUFFLE_SUITE_REPEAT_COUNT` to an `int` controlling how many + times the test suites will be shuffled and run. +- Set `CONFIG_ZTEST_SHUFFLE_TEST_REPEAT_COUNT` to an `int` controlling how many + times the individual tests will be shuffled and run (per suite run). + +Note that the total number of times that tests will run is the result of +multiplying those two Kconfig values. -- cgit v1.2.1