[PATCH 15/17] pytest: Collect SPL unit tests

[Simon Glass]

Hi Stephen,

On Mon, 26 Oct 2020 at 09:34, Stephen Warren <swarren at wwwdotorg.org> wrote:
>
> On 10/23/20 1:27 PM, Simon Glass wrote:
> > Hi Stephen,
> >
> > On Mon, 5 Oct 2020 at 15:35, Stephen Warren <swarren at wwwdotorg.org> wrote:
> >>
> >> On 10/5/20 1:51 PM, Simon Glass wrote:
> >>> Hi Stephen,
> >>>
> >>> On Mon, 5 Oct 2020 at 13:39, Stephen Warren <swarren at wwwdotorg.org> wrote:
> >>>>
> >>>> On 10/3/20 9:25 AM, Simon Glass wrote:
> >>>>> Add a new test_spl fixture to handle running SPL unit tests.
> >>>>
> >>>>> diff --git a/test/py/conftest.py b/test/py/conftest.py
> >>>>
> >>>>> @@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc):
> >>>>>      Returns:
> >>>>>          Nothing.
> >>>>>      """
> >>>>> -
> >>>>> +    #print('name', metafunc.fixturenames)
> >>>>
> >>>> Revert that debug change?
> >>>
> >>> Will do.
> >>>
> >>>>
> >>>>> diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py
> >>>>
> >>>>> +    cons.restart_uboot_with_flags(['-u', ut_spl_subtest])
> >>>>
> >>>> How is that change reverted when the test runs, so that subsequent tests
> >>>> are run on the main U-Boot rather than this restarted U-Boot?
> >>>
> >>> Well actually at the moment it just continues into U-Boot. It will
> >>> mostly pass the tests, but probably not all of them.
> >>
> >> Looking at existing tests, there are quite a few that do this:
> >>
> >> try:
> >>     test body which might do something nasty to U-Boot
> >> finally:
> >>     u_boot_console.restart_uboot()
> >>
> >> ... so that might be applicable.
> >>
> >>>> It feels like it'd be better to start a separate top-level test run for
> >>>> this purpose, rather than swap out the U-Boot process in the middle of a
> >>>> test run.
> >>>
> >>> I was hoping that the fixture stuff would take care of that. How would
> >>> I do a separate top-level test run?
> >>
> >> That'd simply be just running test/py/test.py and passing it the
> >> relevant U-Boot binary/args rather than the main binary. I assume you'd
> >> want to pass relevant -k option to restrict the set of tests run to
> >> something relevant, and an appropriate --build-dir option to point at
> >> the binary.
> >
> > I'm fiddling with this a bit. I do already have a separate run of the
> > SPL tests using the mechanism you describe here, in test/run :
> >
> > # Run tests which require sandbox_spl
> > run_test "sandbox_spl" ./test/py/test.py --bd sandbox_spl --build \
> >    -k 'test_ofplatdata or test_handoff or test_spl'
> >
> > Also, the test_spl() collection function in this patch is used to read
> > out tests from spl/u-boot-spl and these tests are not present in
> > u-boot since they are only built for SPL (Makefile ensures this).
> >
> > So yes I can add a try...finally thing, but in fact all that does is
> > make every SPL test be followed by a reboot into U-Boot proper, for no
> > purpose.
>
> Why do you say for no purpose? The rest of the tests expect to test
> U-Boot proper, or at least whatever U-Boot binary the test system was
> invoked against. If one test switches to a different U-Boot binary than
> the user requests to test, and doesn't switch back, that means the user
> isn't getting what they asked for. Perhaps you can explain your use-case
> a bit more?

I sent a v2 which updates it as you suggest.

My point is that the SPL tests are run separately. This works by your
pytest harness starting u-boot-spl (which runs the tests) and then
u-boot-spl continues to boot into U-Boot proper. It would of course be
possible to quit from SPL, but quitting U-Boot of course confuses
pytest.

If you look at test/run you can see that I added a separate pytest
invocation for the SPL tests. Since sandbox_spl has different options
enabled (and does not use test.dts) it can't currently run some of the
tests in the suite. There is really no point anyway since those tests
already run with the normal and flattree sandbox.

Just to be clear, sandbox and sandbox_spl are somewhat different. By
running sandbox_spl with pytest we are precluded from running many of
the sandbox tests.

>
> In general, the philosophy for testing multiple U-Boot binaries should
> be to run different top-level test invocations on each binary, and each
> top-level test invocation should run all relevant tests against that one
> binary. If some test absolutely has to switch to a different binary,
> then it has to switch back to the "real" binary after it runs to avoid
> the other tests running on a different binary than the user requested.
> Note that binary switching is only possible for sandbox; when running on
> real targets, the flashing step is a one-time thing, not something that
> happens when the target is reset.
>
> > Could you please have another look at this?

Yes sandbox can switch binaries but it is not quite as flexible as you
suggest here. We are using a single build directory in pytest, so we
have to run one of the binaries in that directory. We can run
u-boot-spl but then of course it jumps into 'u-boot' after starting
up. But the 'u-boot' it jumps to is the sandbox_spl one, not the
sandbox oneee

Anyway I added the try...finally clause as you suggested in v2 and
sent it out, so I suppose we are covered both ways.

Regards,
Simon