Re: [PATCH v2 5/5] test: boot: add firmware-FDT source tests

Hi Carlo,

On 2026-07-28T13:20:42, Carlo Caione <[email protected]> wrote:
> test: boot: add firmware-FDT source tests
>
> Add sandbox coverage for firmware_fdt_load(). The Python fixture creates
> mmc11.img in persistent_data_dir with A/B firmware partitions. Its first
> FAT partition carries FITs for a base-plus-overlay configuration,
> compatible best-match, external-data rejection and corrupt-hash rejection.
>
> Construct the complete MMC provider and firmware-FDT source topology in
> each flat-tree test, including the provider phandle. Nothing is added to
> the shared sandbox control devicetree, and the sandbox test framework
> restores its FDT snapshot even when an assertion fails.
>
> Cover default and explicit configuration selection, compatible best-match,
> owned and borrowed assembled-FDT storage, EFI staging and its returned
> filename, and the fail-closed cases: corrupt base or overlay data,
> configuration chaining, a missing configuration or partition, an invalid
> source phandle, external data and mismatched partition selectors. Verify
> that only a genuinely absent source returns -ENOENT.
>
> Signed-off-by: Carlo Caione <[email protected]>
>
> configs/sandbox_defconfig |   1 +
>  test/boot/Makefile        |   1 +
>  test/boot/firmware_fdt.c  | 411 ++++++++++++++++++++++++++++++++++++++++++++++
>  test/py/tests/test_ut.py  | 181 ++++++++++++++++++++
>  4 files changed, 594 insertions(+)

> diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
> @@ -614,6 +614,186 @@ def setup_rauc_image(ubman):
> +def setup_firmware_fdt_image(ubman):
> +    """Create mmc11.img for the firmware_fdt tests
> +
> +    A GPT disk with two firmware partitions (A/B) sharing a firmware type
> +    UUID; partition 1 (label 'firmware') holds a FAT filesystem with the
> +    FIT (fdt.itb) carrying a base DTB and an overlay, with two
> +    configurations: the default applies the overlay, 'conf-base' does not.
> +    """

The docstring only mentions fdt.itb, but the fixture also builds
fdt-best.itb, fdt-ext.itb, fdt-corrupt.itb and
fdt-corrupt-overlay.itb. Please extend it so a reader knows the FAT
holds five FITs without digging through the body.

> diff --git a/test/boot/firmware_fdt.c b/test/boot/firmware_fdt.c
> @@ -0,0 +1,411 @@
> +/* A corrupted base fails hash verification and cannot fall back */
> +static int firmware_fdt_test_corrupt(struct unit_test_state *uts)

There is no coverage for the case where the source is configured, the
partition is found, but the named FIT file does not exist on the
filesystem. That drops through fs_size() in fw_fdt_read_fit() and is
remapped to -ENODEV by fw_fdt_load_source(); worth pinning that
behaviour so a future refactor cannot silently turn "FIT missing" back
into a fall-back -ENOENT. Please add one, e.g. by setting filename to
something not present on the FAT.

> diff --git a/test/boot/firmware_fdt.c b/test/boot/firmware_fdt.c
> @@ -0,0 +1,411 @@
> +/*
> + * Without a source node, -ENOENT is the only result which lets callers fall
> + * back. This also proves the provider alone does not configure the feature.
> + */
> +static int firmware_fdt_test_no_source(struct unit_test_state *uts)
> +{
> +     struct firmware_fdt fw;
> +
> +     ut_assertok(fwfdt_configure(uts, false));
> +     ut_assertok(fwfdt_bind_mmc(uts));
> +
> +     ut_asserteq(-ENOENT, firmware_fdt_load(&fw));
> +     /* Source detection precedes address validation in the EFI helper */
> +     ut_asserteq(-ENOENT, efi_stage_firmware_fdt(0, NULL, NULL));
> +
> +     return 0;
> +}

This is the only test which omits the trailing fwfdt_clear_env() call.
Harmless here since nothing was set, but worth keeping the pattern
uniform so a future change that starts poking at the environment does
not leak between tests.

Regards,
Simon