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

[Stephen Warren]

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?

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?
> 
> Regards,
> Simon
>