Re: [PATCH v2 0/2] ubifs: add KUnit tests for scan and key helpers

[Zhihao Cheng <chengzhihao1@huawei.com>]

在 2026/9/11 11:09, lichen.wang 写道:
> This series adds KUnit tests for fs/ubifs/scan.c and for the helpers in
> fs/ubifs/key.h, so that the node parsing code can be validated offline
> without an MTD device, a UBI volume or a development board.
> 
> Changes in v2:
>   - Use KUNIT_ASSERT_EQ() instead of KUNIT_EXPECT_EQ() for the
>     ubifs_add_snod() return value checks.  On -ENOMEM the function leaves
>     sleb->nodes empty, and list_first_entry()/list_next_entry() on an empty
>     list yield a pointer before the sleb allocation, so continuing after a
>     failed add would read out of bounds.  This affects expect_snod_keyed(),
>     add_snod_pad_node_gets_invalid_key(), add_snod_trun_node_gets_invalid_key()
>     and add_snod_appends_nodes_in_order().
>   - Assert on sleb->nodes_cnt in add_snod_appends_nodes_in_order() as well,
>     because the following list_next_entry() is only safe when both nodes were
>     really linked.
>   - Value checks that cannot be followed by a crash keep using KUNIT_EXPECT_*()
>     so that a single run still reports every mismatch.
>   - Link to v1: https://lore.kernel.org/all/178909365090.247910.11265261850560199708.p0-of-2@gmail.com/
> 
> Baseline:
> 
>    commit df2908090cda368b01ff43709f51890076c56157 ("Linux 7.3-rc2")
>    git describe --always of that commit: v7.3-rc2
>    (the describe string is that of the base commit itself; the patch
>    commits sit on top of it, so a describe run at their tip reports
>    v7.3-rc2-2-g3bcc6fa04 instead)
> 
> Motivation:
> 
> UBIFS has no test coverage for its on-flash parsing code: checking a
> change to scan.c or key.h currently means bringing up a real NAND
> device, a UBI volume and a board. KUnit makes the same checks run in a
> plain QEMU boot within seconds, and it is the established in-tree
> framework for this kind of unit testing, so the tests are reusable by
> anyone touching this code.

Hi lichen,
UBIFS testing can be deployed on QEMU with a nandsim or mtdram.
Current xfstests-dev and mtd-utils/tests/fs-tests could cover scanning 
and kinds ot processes. I am thinking that the ubifs scan logic(actually 
  it depends on disk data structure) hasn't been updated for a long 
time, do we still need to maintain a kunit test to detect issues arising 
from future code changes in ubifs scan?
> 
> Approach:
> 
>   - The tests are compiled into the ubifs module itself (ubifs-y) so
>     that they can call the non-exported ubifs_scan_a_node(),
>     ubifs_check_node(), ubifs_add_snod() and ubifs_end_scan() directly.
>     No EXPORT_SYMBOL() is added to production code.
>   - kunit_helpers.h builds a minimal fake struct ubifs_info with
>     kunit_kzalloc(); only the fields read by the tested paths are
>     initialised. The node length ranges mirror init_constants_early()
>     in super.c, because a zeroed range means "fixed length" for
>     ubifs_check_node() and would make the length checks pass for the
>     wrong reason.
>   - The node constructor fills the common header and payload first and
>     computes the CRC last with the kernel crc32(), over exactly the
>     bytes that ubifs_check_node() checksums.
> 
> Testing:
> 
> The whole suite runs with a single command:
> 
>    ./tools/testing/kunit/kunit.py run --arch=x86_64 --timeout=300 'ubifs*'
> 
> kunit.py exits 0 on the final tree and reports 5 suites and 80 cases,
> all passing:
> 
>    Testing complete. Ran 80 tests: passed: 80
> 
> with zero "not ok" lines in the KTAP output.
> 
> The fake ubifs_info environment was additionally checked against the
> real stack: a nandsim smoke test booted a kernel built from the
> baseline over a complete MTD -> UBI -> UBIFS stack, attached mtd0,
> created and mounted the volume and wrote and read back a file
> successfully.
> 
> Known limitations:
> 
>   - ubifs_scan() and ubifs_start_scan() are not covered by KUnit: the
>     UBI layer is not stubbed, so they would read from a real UBI volume
>     through ubi_leb_read(). They are exercised by the nandsim smoke test
>     instead.
>   - The tests build nodes in memory, so they cannot catch bugs in the
>     on-media layout itself; they only pin down the behaviour of the
>     parsing code for the layouts that UBIFS currently writes.
> 
> Notes:
> 
> checkpatch.pl --strict reports 0 errors for both patches. Each patch
> also carries the generic "added, moved or deleted file(s), does
> MAINTAINERS need updating?" warning; that reminder is emitted for every
> patch that adds a file, and the new files live under fs/ubifs/, which
> is already covered by the "UBI FILE SYSTEM (UBIFS)" entry in
> MAINTAINERS ("F: fs/ubifs/"), so no MAINTAINERS change is needed.
> 
> The cover letter itself draws one more warning from checkpatch.pl, a
> "Prefer a maximum 75 chars per line (possible unwrapped commit
> description?)" hint for a single line of the Changes in v2 list. That
> line is left unwrapped so that the four affected test names stay on
> one line and the list stays easy to scan; it is prose in the cover
> letter, not code or a changelog entry that a tool will reformat.
> 
> Signed-off-by: lichen.wang <wanglichen9110@gmail.com>
> 
> lichen.wang (2):
>    ubifs: add KUnit test scaffolding for scan.c
>    ubifs: add KUnit tests for key helpers
> 
>   fs/ubifs/Kconfig               |  16 +
>   fs/ubifs/Makefile              |   1 +
>   fs/ubifs/tests/key_kunit.c     | 555 ++++++++++++++++++
>   fs/ubifs/tests/kunit_helpers.h | 222 ++++++++
>   fs/ubifs/tests/scan_kunit.c    | 988 +++++++++++++++++++++++++++++++++
>   5 files changed, 1782 insertions(+)
>   create mode 100644 fs/ubifs/tests/key_kunit.c
>   create mode 100644 fs/ubifs/tests/kunit_helpers.h
>   create mode 100644 fs/ubifs/tests/scan_kunit.c
> 
> --
> 2.43.0
> 
> 
> .
>