test: Remove legacy cmocka tests after ztest migration verification#10367
Conversation
tmleman
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR removes legacy cmocka test files that have been successfully migrated to the ztest framework, eliminating test duplication after verifying coverage preservation. The cleanup includes removing test implementations for list operations and fast-get functionality, along with their associated build configuration files.
- Removes 7 legacy cmocka test files for list operations (init, is_empty, item_append, item_del, item_is_last, item_prepend, list_item)
- Removes cmocka fast-get test implementation
- Updates CMakeLists.txt files to remove references to deleted test directories
- Adds new test_list_relink test case to ztest suite
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/ztest/unit/list/test_list_ztest.c | Adds new test_list_relink test to verify list relinking functionality |
| test/cmocka/src/list/*.c | Removes 7 legacy cmocka list test files |
| test/cmocka/src/list/CMakeLists.txt | Removes entire CMakeLists.txt file for legacy list tests |
| test/cmocka/src/lib/fast-get/fast-get-tests.c | Removes legacy fast-get test implementation |
| test/cmocka/src/lib/fast-get/CMakeLists.txt | Removes build configuration for fast-get tests |
| test/cmocka/src/lib/CMakeLists.txt | Removes fast-get subdirectory reference |
| test/cmocka/src/CMakeLists.txt | Removes list subdirectory reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tmleman
requested review from
abonislawski,
serhiy-katsyuba-intel,
singalsu,
softwarecki and
wjablon1
Coverage analysis confirms ztest implementation provides superior coverage: - Lines: 100% (25/25) vs 100% (22/22) - 3 more lines covered - Functions: 100% (5/5) vs no data - function tracking added - All 6 core list functions maintained with equivalent test coverage No regression detected. Safe to remove legacy cmocka list tests. Removed files: - sof/test/cmocka/src/list/ directory and all contained test files - CMakeLists.txt references to legacy list tests Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Looks good! For convenience to other reviewers, direct link to results of running the modified ztest set with this PR:
https://github.com/thesofproject/sof/actions/runs/19364479685/job/55404079833?pr=10367
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Lets also keep cmocka for our more complex APIs where it makes sense being able to run them in valgrind.
|
|
||
| add_subdirectory(alloc) | ||
| add_subdirectory(lib) | ||
| add_subdirectory(fast-get) |
There was a problem hiding this comment.
We should keep fast-get() and any other memory APIs since its very valuable being able to run them in userspace with valgrind.
There was a problem hiding this comment.
The commit removing fast-get tests has been dropped. However, in the long term I will still aim to eliminate duplication, as in my opinion, over a longer time period this may cause unnecessary confusion and increase maintenance costs.
Modern toolchains offer Memory/Address/Leak Sanitizer which should cover all the Valgrind use cases you're concerned about.
There was a problem hiding this comment.
I'm fine to remove over time, we just need a way for runtime heap, stack checking and this may be with the ALSA plugin as its very easy to find and fix memory errors with valgrind.
tmleman
force-pushed
the
topic/upstream/pr/unit_test/cmocka/remove/list_fastget
branch
from
f01cb22 to
2ef6433
Compare
Add ztest coverage for list_relink function that was missing from the unit test suite. The test validates both empty list and non-empty list relinking scenarios. The test covers: - Empty list relinking ensuring proper initialization - Non-empty list relinking verifying that item back-references to the list head are correctly updated when the head structure is moved - List integrity validation after relinking operations This completes the test coverage for all list.h functions and ensures the list_relink functionality is properly validated in the ztest framework. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
4 participants
Remove legacy cmocka tests that have been successfully migrated to ztest framework with verified coverage preservation. This cleanup eliminates test duplication and completes the modernization of SOF test infrastructure.
Changes
Removed Legacy Tests
Fast-get tests: sof/test/cmocka/src/lib/fast-get/ - fast-get functionality testsCoverage Verification Completed
Removed