On 10/7/24 17:23, Simon Glass wrote:
Hi Jerome,
On Fri, 4 Oct 2024 at 06:01, Jerome Forissier jerome.forissier@linaro.org wrote:
On 10/4/24 11:37, Ilias Apalodimas wrote:
On Fri, 4 Oct 2024 at 11:46, Jerome Forissier jerome.forissier@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@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@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@0 and* dsa-test@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@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,