| Message ID | 20201003152534.3184504-16-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | dm: test: Add unit tests for SPL | expand |
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? > 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? 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.
Hi Stephen, On Mon, 5 Oct 2020 at 13:39, Stephen Warren <swarren@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. > > 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? Regards, Simon
On 10/5/20 1:51 PM, Simon Glass wrote: > Hi Stephen, > > On Mon, 5 Oct 2020 at 13:39, Stephen Warren <swarren@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.
Hi Stephen, On Mon, 5 Oct 2020 at 15:35, Stephen Warren <swarren@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@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. Could you please have another look at this? Regards, Simon
On 10/23/20 1:27 PM, Simon Glass wrote: > Hi Stephen, > > On Mon, 5 Oct 2020 at 15:35, Stephen Warren <swarren@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@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 >
Hi Stephen, On Mon, 26 Oct 2020 at 09:34, Stephen Warren <swarren@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@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@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
diff --git a/test/py/conftest.py b/test/py/conftest.py index 30920474b36..28fde347c00 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -227,7 +227,7 @@ def pytest_configure(config): console = u_boot_console_exec_attach.ConsoleExecAttach(log, ubconfig) re_ut_test_list = re.compile(r'_u_boot_list_2_(.*)_test_2_\1_test_(.*)\s*$') -def generate_ut_subtest(metafunc, fixture_name): +def generate_ut_subtest(metafunc, fixture_name, sym_path): """Provide parametrization for a ut_subtest fixture. Determines the set of unit tests built into a U-Boot binary by parsing the @@ -237,12 +237,13 @@ def generate_ut_subtest(metafunc, fixture_name): Args: metafunc: The pytest test function. fixture_name: The fixture name to test. + sym_path: Relative path to the symbol file with preceding '/' + (e.g. '/u-boot.sym') Returns: Nothing. """ - - fn = console.config.build_dir + '/u-boot.sym' + fn = console.config.build_dir + sym_path try: with open(fn, 'rt') as f: lines = f.readlines() @@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc): Returns: Nothing. """ - + #print('name', metafunc.fixturenames) for fn in metafunc.fixturenames: if fn == 'ut_subtest': - generate_ut_subtest(metafunc, fn) + generate_ut_subtest(metafunc, fn, '/u-boot.sym') + continue + if fn == 'ut_spl_subtest': + generate_ut_subtest(metafunc, fn, '/spl/u-boot-spl.sym') continue generate_config(metafunc, fn) diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py new file mode 100644 index 00000000000..58a851e5ec8 --- /dev/null +++ b/test/py/tests/test_spl.py @@ -0,0 +1,29 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright 2020 Google LLC +# Written by Simon Glass <sjg@chromium.org> + +import os.path +import pytest + +def test_spl(u_boot_console, ut_spl_subtest): + """Execute a "ut" subtest. + + The subtests are collected in function generate_ut_subtest() from linker + generated lists by applying a regular expression to the lines of file + spl/u-boot-spl.sym. The list entries are created using the C macro + UNIT_TEST(). + + Strict naming conventions have to be followed to match the regular + expression. Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in + test suite foo that can be executed via command 'ut foo bar' and is + implemented in C function foo_test_bar(). + + Args: + u_boot_console (ConsoleBase): U-Boot console + ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to + execute command 'ut foo bar' + """ + cons = u_boot_console + cons.restart_uboot_with_flags(['-u', ut_spl_subtest]) + output = cons.get_spawn_output().replace('\r', '') + assert 'Failures: 0' in output
Add a new test_spl fixture to handle running SPL unit tests. Signed-off-by: Simon Glass <sjg@chromium.org> --- test/py/conftest.py | 14 +++++++++----- test/py/tests/test_spl.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 test/py/tests/test_spl.py