[PATCH 1/1] test: bootflow test depends on CONFIG_VIDEO

[Simon Glass]

Hi Heinrich,

On Wed, 31 May 2023 at 13:27, Heinrich Schuchardt
<heinrich.schuchardt at 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 at konsulko.com> wrote:
> >>>>
> >>>> On Wed, May 31, 2023 at 04:25:46PM +0200, Heinrich Schuchardt wrote:
> >>>>> Tom Rini <trini at 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 at 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