[PATCH v11 06/29] test: boot: fix bootflow_cmd_label for when DSA_SANDBOX is disabled

[Jerome Forissier]

On 10/7/24 17:23, Simon Glass wrote:
> Hi Jerome,
> 
> On Fri, 4 Oct 2024 at 06:01, Jerome Forissier
> <jerome.forissier at linaro.org> wrote:
>>
>>
>>
>> On 10/4/24 11:37, Ilias Apalodimas wrote:
>>> On Fri, 4 Oct 2024 at 11:46, Jerome Forissier
>>> <jerome.forissier at linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 10/4/24 08:55, Ilias Apalodimas wrote:
>>>>> Hi Jerome,
>>>>>
>>>>> On Thu, 3 Oct 2024 at 18:23, Jerome Forissier
>>>>> <jerome.forissier at linaro.org> wrote:
>>>>>>
>>>>>> When DSA_SANDBOX is not set, the sandbox tests fail as follows:
>>>>>>
>>>>>>  $ ./test/py/test.py --build-dir=$(pwd) -k bootdev_test_any
>>>>>>  [...]
>>>>>>  Scanning for bootflows with label '9'
>>>>>>  [...]
>>>>>>  Cannot find '9' (err=-19)
>>>>>>
>>>>>> This is due to the device list containing two less entries than
>>>>>> expected. Therefore, look for label '7' when DSA_SANDBOX is disabled.
>>>>>>
>>>>>> The actual use case is NET_LWIP=y (to be introduced in later patches)
>>>>>> which implies DSA_SANDBOX=n for the time being.
>>>>>>
>>>>>> Signed-off-by: Jerome Forissier <jerome.forissier at linaro.org>
>>>>>> ---
>>>>>>  test/boot/bootflow.c | 7 +++++--
>>>>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
>>>>>> index 6ad63afe90a..c440b8eb778 100644
>>>>>> --- a/test/boot/bootflow.c
>>>>>> +++ b/test/boot/bootflow.c
>>>>>> @@ -109,9 +109,12 @@ static int bootflow_cmd_label(struct unit_test_state *uts)
>>>>>>          * 8   [   ]      OK  mmc       mmc2.bootdev
>>>>>>          * 9   [ + ]      OK  mmc       mmc1.bootdev
>>>>>>          * a   [   ]      OK  mmc       mmc0.bootdev
>>>>>> +        *
>>>>>> +        * However with CONFIG_DSA_SANDBOX=n we have two less (dsa-test at 0 and
>>>>>> +        * dsa-test at 1).
>>>>>>          */
>>>>>> -       ut_assertok(run_command("bootflow scan -lH 9", 0));
>>>>>> -       ut_assert_nextline("Scanning for bootflows with label '9'");
>>>>>
>>>>> Shouldn't this under and #ifdef, IS_ENABLED etc?
>>>>
>>>> In theory yes, but we can avoid the conditional by using index 7 which is always
>>>> valid, i.e., in all configurations we have at least 7 devices (even 8 actually).
>>>
>>> Ok, but I *think* Simon was trying to match the exact out put here,
>>> not 'at least 7'.
>>>
>>> I think we are better off being strict on this test
>>
>> No because there are 10 entries according to the comment ("a" hex being
>> mmc0.bootdev). Simon, what do you suggest?
> 
> I don't think this is a huge deal.
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>

Unfortunately this patch breaks the default config (NET=y and therefore
DSA_SANDBOX=y):

=> ut bootstd bootflow_cmd_label
Test: bootflow_cmd_label: bootflow.c
Scanning for bootflows with label 'mmc1'
Seq  Method       State   Uclass    Part  Name                      Filename
---  -----------  ------  --------  ----  ------------------------  ----------------
Scanning bootdev 'mmc1.bootdev':
  0  extlinux     ready   mmc          1  mmc1.bootdev.part_1       /extlinux/extlinux.conf
No more bootdevs
---  -----------  ------  --------  ----  ------------------------  ----------------
(1 bootflow, 1 valid)
Scanning for bootflows with label '0'
Seq  Method       State   Uclass    Part  Name                      Filename
---  -----------  ------  --------  ----  ------------------------  ----------------
---  -----------  ------  --------  ----  ------------------------  ----------------
(0 bootflows, 0 valid)
Scanning for bootflows with label '7'
Seq  Method       State   Uclass    Part  Name                      Filename
---  -----------  ------  --------  ----  ------------------------  ----------------
---  -----------  ------  --------  ----  ------------------------  ----------------
(0 bootflows, 0 valid)
test/boot/bootflow.c:118, bootflow_cmd_label(): console:
Expected '(1 bootflow, 1 valid)',
     got to '(0 bootflows, 0 valid)'
[...]


So for v12 I'll do what Ilias suggested:

-       ut_assertok(run_command("bootflow scan -lH 9", 0));
-       ut_assert_nextline("Scanning for bootflows with label '9'");
+       if (CONFIG_IS_ENABLED(DSA_SANDBOX)) {
+               ut_assertok(run_command("bootflow scan -lH 9", 0));
+               ut_assert_nextline("Scanning for bootflows with label '9'");
+       } else {
+               ut_assertok(run_command("bootflow scan -lH 7", 0));
+               ut_assert_nextline("Scanning for bootflows with label '7'");
+       }
        ut_assert_skip_to_line("(1 bootflow, 1 valid)");


Tested OK with DSA_SANDBOX=y as well as DSA_SANDBOX=n.

 
> BTW, 'fewer', not 'less', if you can count them

Sure :)


Thanks,
-- 
Jerome