On 5/31/23 23:16, Simon Glass wrote:
Hi Heinrich,
On Wed, 31 May 2023 at 14:22, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 5/31/23 21:52, Simon Glass wrote:
Hi Heinrich,
On Wed, 31 May 2023 at 13:27, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 5/31/23 20:24, Tom Rini wrote:
On Wed, May 31, 2023 at 08:05:18PM +0200, Heinrich Schuchardt wrote:
On 5/31/23 19:13, Simon Glass wrote: > Hi Heinrich, > > On Wed, 31 May 2023 at 09:52, Tom Rini trini@konsulko.com wrote: >> >> On Wed, May 31, 2023 at 04:25:46PM +0200, Heinrich Schuchardt wrote: >>> Tom Rini trini@konsulko.com schrieb am Mi., 31. Mai 2023, 16:02: >>> >>>> On Wed, May 31, 2023 at 10:50:52AM +0200, Heinrich Schuchardt wrote: >>>> >>>>> qemu_arm64_defconfig with UNIT_TEST=y does not build. >>>>> >>>>> CONFIG_EXPO depends on CONFIG_VIDEO. Hence on platforms without video we >>>>> must not enable unit tests invoking expo functions. >>>>> >>>>> Fixes: fb1451bec2a5 ("bootstd: Add tests for bootstd including all >>>> uclasses") >>>>> Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com >>>>> --- >>>>> test/boot/Makefile | 5 ++++- >>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/test/boot/Makefile b/test/boot/Makefile >>>>> index 22ed61c8fa..665017ba3d 100644 >>>>> --- a/test/boot/Makefile >>>>> +++ b/test/boot/Makefile >>>>> @@ -2,7 +2,10 @@ >>>>> # >>>>> # Copyright 2021 Google LLC >>>>> >>>>> -obj-$(CONFIG_BOOTSTD) += bootdev.o bootstd_common.o bootflow.o >>>> bootmeth.o >>>>> +ifdef CONFIG_BOOTSTD >>>>> +obj-y += bootstd_common.o bootmeth.o >>>>> +obj-$(VIDEO) += bootdev.o bootflow.o >>>>> +endif >>>>> obj-$(CONFIG_FIT) += image.o >>>>> >>>>> obj-$(CONFIG_EXPO) += expo.o >>>> >>>> OK, but the functionality itself doesn't depend on video, it looks like >>>> maybe only the bootflow_menu_theme test itself, so lets try and solve >>>> this differently. >>>> >>>> -- >>>> Tom >>>> >>> >>> Yes I should have used CONFIG_EXPO not CONFIG_VIDEO . >> >> Well no, at first glance it's 1 out of 15 tests in that file that depend >> on CONFIG_EXPO, so we should just ifdef that test.
Yes, there is only one test in bootflow.o depending on EXPO. There are multiple tests in bootdev.o depending on CONFIG_DM_MMC.
And the tests presumably check for DM_MMC, or should.
> You could put something like this at the top of bootflow_cmd_menu(): > > /* 'bootflow menu' currently requires a video console */ > if (!IS_ENABLED(CONFIG_VIDEO)) > return -EAGAIN;
With this change I still get:
aarch64-linux-gnu-ld.bfd: boot/bootflow_menu.o: in function`bootflow_menu_new':
Looks like there's an extra line in boot/Makefile then, we already include bootflow_menu.o on EXPO.
Hello Simon,
the bootstd tests are in complete disarray
On the sandbox invoked with
./u-boot -T
=> ut bootstd Can't map file 'mmc1.img': Invalid argument mmc1: Unable to map file 'mmc1.img' Failed to set up for bootstd tests (err=-5)
It I create a file mmc1.img:
test/boot/bootdev.c:460, bootdev_test_bootable(): -EINVAL == bootdev_find_in_blk(iter.dev, blk, &iter, &bflow): Expected 0xffffffea (-22), got 0xffffffa3 (-93) test/boot/bootdev.c:465, bootdev_test_bootable(): -ENOENT == bootdev_find_in_blk(iter.dev, blk, &iter, &bflow): Expected 0xfffffffe (-2), got 0xffffffa3 (-93) test/boot/bootdev.c:466, bootdev_test_bootable(): 1 == iter.first_bootable: Expected 0x1 (1), got 0x0 (0) test/boot/bootdev.c:470, bootdev_test_bootable(): -EINVAL == bootdev_find_in_blk(iter.dev, blk, &iter, &bflow): Expected 0xffffffea (-22), got 0xffffffa3 (-93) Test: bootdev_test_bootable: bootdev.c (flat tree) test/boot/bootdev.c:460, bootdev_test_bootable(): -EINVAL == bootdev_find_in_blk(iter.dev, blk, &iter, &bflow): Expected 0xffffffea (-22), got 0xffffffa3 (-93) test/boot/bootdev.c:465, bootdev_test_bootable(): -ENOENT == bootdev_find_in_blk(iter.dev, blk, &iter, &bflow): Expected 0xfffffffe (-2), got 0xffffffa3 (-93) test/boot/bootdev.c:466, bootdev_test_bootable(): 1 == iter.first_bootable: Expected 0x1 (1), got 0x0 (0) test/boot/bootdev.c:470, bootdev_test_bootable(): -EINVAL == bootdev_find_in_blk(iter.dev, blk, &iter, &bflow): Expected 0xffffffea (-22), got 0xffffffa3 (-93) Test <NULL> failed 8 times
I normally run these tests with an alias: pyt bootstd
# Run a pytest on sandbox # $1: Name of test to run (optional, else run all)
function pyt { local tests local para
if [ "$1" = "-b" ]; then crosfw $b || return 1 shift fi if [ "$1" = "-p" ]; then para="-n$(nproc) -q" shift fi tests="$@" echo $para $tests shift test/py/test.py -B sandbox --build-dir /tmp/b/sandbox ${para}${tests:+"-k $tests"} $@ }
Once I do that once (for setup), I can then run individual tests directly: rt bootdev_test_bootable
function rt_get_suite_and_name() { local arg #echo arg $arg suite= name=
if [ "$1" = "-f" ]; then force="-f" shift fi arg="$1" rest="$2" # The symbol is something like this: # _u_boot_list_2_ut_bootstd_test_2_vbe_simple_test_base # Split it into the suite name (bootstd) and test name # (vbe_simple_test_base) read suite name < \ <(nm /tmp/b/$exec/u-boot |grep "list_2_ut.*$arg.*" \ | cut -d' ' -f3 \ | head -1 \ | sed -n 's/_u_boot_list_2_ut_\(.*\)_test_2_/\1 /p') #echo suite $suite #echo name $name}
# Run a test function rt() { local exec=sandbox local suite name force rest rt_get_suite_and_name $*
/tmp/b/$exec/u-boot -T $rest -c "ut $suite $force $name"}
As discussed previously:
The general expectation is that unit tests can be run successfully on any physical or emulated board. Should a test not comply with this requirement the test must be deactivated automatically upon build.
A test like ut_assert_skip_to_line( "sandbox: continuing, as we cannot run Linux"); will never succeed on other architectures.
Most of the tests are written for sandbox, since:
- they are testing logic which is not arch- or board-specific
- they can access state which is not available on other boards
- they can make use of host files
- they are 1000s of times faster to run
Why are you wanting to run these tests on 'real' boards? There has to be a reason, beyond just an academic exercise.
Regards, Simon
Different architectures have restrictions that the sandbox does not have, e.g. concerning unaligned access, 32bit size. Therefore we should be able to run most tests on any architecture.
I don't disagree in principle, but in my experience there are very few bugs that are not found by sandbox. In fact, sandbox being able to segfault finds quite a few bugs that are invisible on real hardware.
For the 32-bit size, we could have a 32-bit sandbox version if you like. It does automatically build as 32-bit on 32-bit machines, but we use 64-bit in our CI.
I completely agree there will be bugs that are found only on real hardware, but often just running it once on hardware is enough to flush those out and add more sandbox test coverage for them.
If a test can only be run on the sandbox, CONFIG_UNIT_TEST=y should not build it.
At the moment UNIT_TEST is somewhat synonymous with SANDBOX. I enabled UNIT_TEST on snow to at least avoid obvious build errors, but there is no expectation that the unit tests all run on snow. To enforce this we really need real boards in CI. Perhaps QEMU could be the way to go, to resolve this?
Yes our target should be to enable CONFIG_UNIT_TEST on QEMU.
The ut command should succeed wherever it is run.
Yes that would be good. I think you will need quite a few checks added though.
For the bootstd test it obvious that they currently have to be restricted to the sandbox as they won't pass anywhere else.
Best regards
Heinrich
Regards, Simon