Re: [PATCH 2/2] test: hook up test of allowing control DTB to act as FIT image

[Rasmus Villemoes <ravi@prevas.dk>]

On Fri, May 15 2026, Simon Glass <sjg@chromium.org> wrote:

> Hi Rasmus,
>
> On 2026-05-12T16:16:29, Rasmus Villemoes <ravi@prevas.dk> wrote:
>> test: hook up test of allowing control DTB to act as FIT image
>>
>> Add a test demonstrating how one can embed various scripts in the
>> control DTB and verifying that the source command can be used with
>> ${fdtcontroladdr}.
>>
>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
>>
>> arch/sandbox/dts/sandbox-test1.sh     |  4 ++++
>>  arch/sandbox/dts/sandbox-test2.sh     |  4 ++++
>>  arch/sandbox/dts/sandbox_scripts.dtsi | 18 ++++++++++++++++++
>>  configs/sandbox_defconfig             |  1 +
>>  test/py/tests/test_source.py          | 12 ++++++++++++
>>  5 files changed, 39 insertions(+)
>
>> diff --git a/arch/sandbox/dts/sandbox_scripts.dtsi b/arch/sandbox/dts/sandbox_scripts.dtsi
>> @@ -0,0 +1,18 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/ {
>> +     images {
>> +             default = 'test';
>> +             test1 {
>> +                     description = 'Testscript';
>> +                     data = /incbin/('sandbox-test1.sh');
>> +                     type = 'script';
>> +                     compression = 'none';
>> +             };
>> +             test2 {
>> +                     description = "Another testscript";
>> +                     data = /incbin/('sandbox-test2.sh');
>> +                     type = 'script';
>> +                     compression = 'none';
>> +             };
>> +     };
>> +};
>
> There is no image called 'test', so default = 'test' points at
> nothing. Did you mean test1? Fixing it would also let you exercise the
> no-uname case (see below).

Good catch, thanks! I mixed this up with the .dtsi I used for the actual
board I was testing this on.

>> diff --git a/test/py/tests/test_source.py b/test/py/tests/test_source.py
>> @@ -34,3 +34,15 @@ def test_source(ubman):
>> +    output = ubman.run_command('source ${fdtcontroladdr}:test1')
>> +    assert 'aaa' in output
>> +    assert 'bbb' in output
>> +    assert 'ccc' in output
>> +
>> +    output = ubman.run_command('source ${fdtcontroladdr}:test2')
>> +    assert 'aaa' not in output
>> +    assert 'bbb' in output
>> +    assert 'ccc' not in output
>> +
>> +    assert 'Fail' in ubman.run_command('source ${fdtcontroladdr}:no-such-script || echo Fail')
>
> The cover letter calls out 'source ${fdtcontroladdr}' (no name,
> falling through to /images/default) as the headline use case, but
> nothing here exercises it. Please add a case for that once default
> points at a real image.

Done.

>> diff --git a/arch/sandbox/dts/sandbox-test1.sh b/arch/sandbox/dts/sandbox-test1.sh
>> @@ -0,0 +1,4 @@
>> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +echo 'aaa'
>> +source ${fdtcontroladdr}:test2
>> +echo 'ccc'
>
> The commit message is very terse for three new files plus a defconfig
> change. Please mention that the test also verifies recursive sourcing
> from the control DTB (test1 invoking test2), since that is a
> non-trivial property worth flagging.

Agreed, done.

> Also, could you add to doc/ ?

Absolutely.

Thanks,
Rasmus