| Message ID | 20200622160722.124838-1-xypron.glpk@gmx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] gitlab: show skipped Python tests | expand |
Hi Heinrich, On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > Call pytest3 with argument -ra to display reason why Python tests are > skipped. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > --- > .gitlab-ci.yml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > index f2e491c117..f53098ea5f 100644 > --- a/.gitlab-ci.yml > +++ b/.gitlab-ci.yml > @@ -46,7 +46,7 @@ stages: > # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > --build-dir "$UBOOT_TRAVIS_BUILD_DIR" Do you have a link showing the current output with this patch? Regards, Simon
On 22.06.20 18:17, Simon Glass wrote: > Hi Heinrich, > > On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >> >> Call pytest3 with argument -ra to display reason why Python tests are >> skipped. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >> --- >> .gitlab-ci.yml | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml >> index f2e491c117..f53098ea5f 100644 >> --- a/.gitlab-ci.yml >> +++ b/.gitlab-ci.yml >> @@ -46,7 +46,7 @@ stages: >> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not >> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; >> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; >> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} >> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} >> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} >> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > > Do you have a link showing the current output with this patch? Hello Simon, here is an example output: https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 Some of the skips are ok. But some we should really fix like: SKIPPED [1] /builds/u-boot/custodians/u-boot-efi/test/py/tests/test_efi_loader.py:100: No static network configuration is defined Best regards Heinrich
Hi Heinrich, On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > On 22.06.20 18:17, Simon Glass wrote: > > Hi Heinrich, > > > > On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > >> > >> Call pytest3 with argument -ra to display reason why Python tests are > >> skipped. > >> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > >> --- > >> .gitlab-ci.yml | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > >> index f2e491c117..f53098ea5f 100644 > >> --- a/.gitlab-ci.yml > >> +++ b/.gitlab-ci.yml > >> @@ -46,7 +46,7 @@ stages: > >> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > >> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > >> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > >> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > >> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > >> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > >> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > > > > Do you have a link showing the current output with this patch? > > Hello Simon, > > here is an example output: > > https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 That's what I was afraid of. The skip output is more than the normal output, and if we don't intend to fix it, I'd rather not have unactionable warnings in the output. Having said that, we need to enable SPI flash, FPGA and MMC environment tests by the look of it. > > Some of the skips are ok. But some we should really fix like: > > SKIPPED [1] > /builds/u-boot/custodians/u-boot-efi/test/py/tests/test_efi_loader.py:100: > No static network configuration is defined Also, I suspect that the warnings will be huge when running on real hardware. Regards, Simon
On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: > Hi Heinrich, > > On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > On 22.06.20 18:17, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > >> > > >> Call pytest3 with argument -ra to display reason why Python tests are > > >> skipped. > > >> > > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > >> --- > > >> .gitlab-ci.yml | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > > >> index f2e491c117..f53098ea5f 100644 > > >> --- a/.gitlab-ci.yml > > >> +++ b/.gitlab-ci.yml > > >> @@ -46,7 +46,7 @@ stages: > > >> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > > >> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > > >> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > > >> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > > >> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > > >> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > > >> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > > > > > > Do you have a link showing the current output with this patch? > > > > Hello Simon, > > > > here is an example output: > > > > https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 > > That's what I was afraid of. The skip output is more than the normal > output, and if we don't intend to fix it, I'd rather not have > unactionable warnings in the output. > > Having said that, we need to enable SPI flash, FPGA and MMC > environment tests by the look of it. So, looking at the output. 40 lines of test output, 32 lines of explanation of why skips. I think it's still readable. It also shows where perhaps we could group / re-org things a little as it already does smart things like: SKIPPED [131] /builds/u-boot/custodians/u-boot-efi/test/py/conftest.py:468: board "qemu_arm64" not supported Looking at the spi flash tests (and also the mmc read/write tests) we should have a generic check in the tests for them be configured and bail if not, as that would reduce the skip summary lines and collect them all in one spot. > > Some of the skips are ok. But some we should really fix like: > > > > SKIPPED [1] > > /builds/u-boot/custodians/u-boot-efi/test/py/tests/test_efi_loader.py:100: > > No static network configuration is defined > > Also, I suspect that the warnings will be huge when running on real hardware. Well, that's beside the point with this patch. It's very specifically only doing it on GitLab (and if/when we add this, v2 should cover travis/Azure) as it's CI and logs are cheap but recreation can be annoying. But it's also not true at least here. I see a few more lines than the above example on my am335x_evm config, but I also have a few more skips.
On 6/22/20 10:07 AM, Heinrich Schuchardt wrote: > Call pytest3 with argument -ra to display reason why Python tests are > skipped. Acked-by: Stephen Warren <swarren at nvidia.com>
On 22.06.20 20:23, Simon Glass wrote: > Hi Heinrich, > > On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >> >> On 22.06.20 18:17, Simon Glass wrote: >>> Hi Heinrich, >>> >>> On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >>>> >>>> Call pytest3 with argument -ra to display reason why Python tests are >>>> skipped. >>>> >>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >>>> --- >>>> .gitlab-ci.yml | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml >>>> index f2e491c117..f53098ea5f 100644 >>>> --- a/.gitlab-ci.yml >>>> +++ b/.gitlab-ci.yml >>>> @@ -46,7 +46,7 @@ stages: >>>> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not >>>> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; >>>> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; >>>> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} >>>> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} >>>> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} >>>> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" >>> >>> Do you have a link showing the current output with this patch? >> >> Hello Simon, >> >> here is an example output: >> >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 > > That's what I was afraid of. The skip output is more than the normal > output, and if we don't intend to fix it, I'd rather not have > unactionable warnings in the output. > > Having said that, we need to enable SPI flash, FPGA and MMC > environment tests by the look of it. > >> >> Some of the skips are ok. But some we should really fix like: >> >> SKIPPED [1] >> /builds/u-boot/custodians/u-boot-efi/test/py/tests/test_efi_loader.py:100: >> No static network configuration is defined > > Also, I suspect that the warnings will be huge when running on real hardware. Hello Simon, this patch does not apply to real hardware. I am changing .gitlab-ci.yml and not test/run. So the additional lines will only show up in Gitlab CI. Best regards Heinrich
On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: > Hi Heinrich, > > On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > On 22.06.20 18:17, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > >> > > >> Call pytest3 with argument -ra to display reason why Python tests are > > >> skipped. > > >> > > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > >> --- > > >> .gitlab-ci.yml | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > > >> index f2e491c117..f53098ea5f 100644 > > >> --- a/.gitlab-ci.yml > > >> +++ b/.gitlab-ci.yml > > >> @@ -46,7 +46,7 @@ stages: > > >> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > > >> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > > >> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > > >> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > > >> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > > >> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > > >> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > > > > > > Do you have a link showing the current output with this patch? > > > > Hello Simon, > > > > here is an example output: > > > > https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 > > That's what I was afraid of. The skip output is more than the normal > output, and if we don't intend to fix it, I'd rather not have > unactionable warnings in the output. > > Having said that, we need to enable SPI flash, FPGA and MMC > environment tests by the look of it. On a different note, I think we should look at: https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112376 and: https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112380 as it shows that the reason we probably skip the test_fs/test_mkdir.py tests is that since board is literal, we don't match sandbox on sandbox_flattree. That answers one outstanding question about why we skip some tests and not others at least.
Hi, On Mon, 22 Jun 2020 at 12:46, Tom Rini <trini at konsulko.com> wrote: > > On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: > > Hi Heinrich, > > > > On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > > > On 22.06.20 18:17, Simon Glass wrote: > > > > Hi Heinrich, > > > > > > > > On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > >> > > > >> Call pytest3 with argument -ra to display reason why Python tests are > > > >> skipped. > > > >> > > > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > > >> --- > > > >> .gitlab-ci.yml | 2 +- > > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >> > > > >> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > > > >> index f2e491c117..f53098ea5f 100644 > > > >> --- a/.gitlab-ci.yml > > > >> +++ b/.gitlab-ci.yml > > > >> @@ -46,7 +46,7 @@ stages: > > > >> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > > > >> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > > > >> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > > > >> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > > > >> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > > > >> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > > > >> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > > > > > > > > Do you have a link showing the current output with this patch? > > > > > > Hello Simon, > > > > > > here is an example output: > > > > > > https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 > > > > That's what I was afraid of. The skip output is more than the normal > > output, and if we don't intend to fix it, I'd rather not have > > unactionable warnings in the output. > > > > Having said that, we need to enable SPI flash, FPGA and MMC > > environment tests by the look of it. > > On a different note, I think we should look at: > https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112376 > and: > https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112380 > > as it shows that the reason we probably skip the test_fs/test_mkdir.py > tests is that since board is literal, we don't match sandbox on > sandbox_flattree. That answers one outstanding question about why we > skip some tests and not others at least. Hmm yes. It is definitely good to have this output so we can figure out what should not be skipped. But outputting things which we know should be skipped just means we won't notice those that are not supposed to be skipped. How do we handle that? Regards, Simon
On 24.06.20 15:49, Simon Glass wrote: > Hi, > > On Mon, 22 Jun 2020 at 12:46, Tom Rini <trini at konsulko.com> wrote: >> >> On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: >>> Hi Heinrich, >>> >>> On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >>>> >>>> On 22.06.20 18:17, Simon Glass wrote: >>>>> Hi Heinrich, >>>>> >>>>> On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >>>>>> >>>>>> Call pytest3 with argument -ra to display reason why Python tests are >>>>>> skipped. >>>>>> >>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >>>>>> --- >>>>>> .gitlab-ci.yml | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml >>>>>> index f2e491c117..f53098ea5f 100644 >>>>>> --- a/.gitlab-ci.yml >>>>>> +++ b/.gitlab-ci.yml >>>>>> @@ -46,7 +46,7 @@ stages: >>>>>> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not >>>>>> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; >>>>>> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; >>>>>> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} >>>>>> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} >>>>>> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} >>>>>> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" >>>>> >>>>> Do you have a link showing the current output with this patch? >>>> >>>> Hello Simon, >>>> >>>> here is an example output: >>>> >>>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 >>> >>> That's what I was afraid of. The skip output is more than the normal >>> output, and if we don't intend to fix it, I'd rather not have >>> unactionable warnings in the output. >>> >>> Having said that, we need to enable SPI flash, FPGA and MMC >>> environment tests by the look of it. >> >> On a different note, I think we should look at: >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112376 >> and: >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112380 >> >> as it shows that the reason we probably skip the test_fs/test_mkdir.py >> tests is that since board is literal, we don't match sandbox on >> sandbox_flattree. That answers one outstanding question about why we >> skip some tests and not others at least. > > Hmm yes. > > It is definitely good to have this output so we can figure out what > should not be skipped. > > But outputting things which we know should be skipped just means we > won't notice those that are not supposed to be skipped. How do we > handle that? > > Regards, > Simon > If you have a lines like: .config feature "cmd_fpga_loadbp" not enabled board "qemu_arm64" not supported you know the test is skipped due to configuration. Other messages clearly tell you that something is not correctly set up: No env__efi_loader_grub_file binary specified in environment got empty parameter set ['env__mmc_dev_config'] Best regards Heinrich
Hi Heinrich, On Wed, 24 Jun 2020 at 07:56, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > On 24.06.20 15:49, Simon Glass wrote: > > Hi, > > > > On Mon, 22 Jun 2020 at 12:46, Tom Rini <trini at konsulko.com> wrote: > >> > >> On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: > >>> Hi Heinrich, > >>> > >>> On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > >>>> > >>>> On 22.06.20 18:17, Simon Glass wrote: > >>>>> Hi Heinrich, > >>>>> > >>>>> On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > >>>>>> > >>>>>> Call pytest3 with argument -ra to display reason why Python tests are > >>>>>> skipped. > >>>>>> > >>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > >>>>>> --- > >>>>>> .gitlab-ci.yml | 2 +- > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > >>>>>> index f2e491c117..f53098ea5f 100644 > >>>>>> --- a/.gitlab-ci.yml > >>>>>> +++ b/.gitlab-ci.yml > >>>>>> @@ -46,7 +46,7 @@ stages: > >>>>>> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > >>>>>> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > >>>>>> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > >>>>>> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > >>>>>> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > >>>>>> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > >>>>>> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > >>>>> > >>>>> Do you have a link showing the current output with this patch? > >>>> > >>>> Hello Simon, > >>>> > >>>> here is an example output: > >>>> > >>>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 > >>> > >>> That's what I was afraid of. The skip output is more than the normal > >>> output, and if we don't intend to fix it, I'd rather not have > >>> unactionable warnings in the output. > >>> > >>> Having said that, we need to enable SPI flash, FPGA and MMC > >>> environment tests by the look of it. > >> > >> On a different note, I think we should look at: > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112376 > >> and: > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112380 > >> > >> as it shows that the reason we probably skip the test_fs/test_mkdir.py > >> tests is that since board is literal, we don't match sandbox on > >> sandbox_flattree. That answers one outstanding question about why we > >> skip some tests and not others at least. > > > > Hmm yes. > > > > It is definitely good to have this output so we can figure out what > > should not be skipped. > > > > But outputting things which we know should be skipped just means we > > won't notice those that are not supposed to be skipped. How do we > > handle that? > > > > Regards, > > Simon > > > If you have a lines like: > > .config feature "cmd_fpga_loadbp" not enabled > board "qemu_arm64" not supported > > you know the test is skipped due to configuration. OK then can we avoid printing this useless information by default? > > Other messages clearly tell you that something is not correctly set up: > > No env__efi_loader_grub_file binary specified in environment > got empty parameter set ['env__mmc_dev_config'] OK then this is what we should display. Regards, Simon
On 24.06.20 17:17, Simon Glass wrote: > Hi Heinrich, > > On Wed, 24 Jun 2020 at 07:56, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >> >> On 24.06.20 15:49, Simon Glass wrote: >>> Hi, >>> >>> On Mon, 22 Jun 2020 at 12:46, Tom Rini <trini at konsulko.com> wrote: >>>> >>>> On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: >>>>> Hi Heinrich, >>>>> >>>>> On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >>>>>> >>>>>> On 22.06.20 18:17, Simon Glass wrote: >>>>>>> Hi Heinrich, >>>>>>> >>>>>>> On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >>>>>>>> >>>>>>>> Call pytest3 with argument -ra to display reason why Python tests are >>>>>>>> skipped. >>>>>>>> >>>>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >>>>>>>> --- >>>>>>>> .gitlab-ci.yml | 2 +- >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml >>>>>>>> index f2e491c117..f53098ea5f 100644 >>>>>>>> --- a/.gitlab-ci.yml >>>>>>>> +++ b/.gitlab-ci.yml >>>>>>>> @@ -46,7 +46,7 @@ stages: >>>>>>>> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not >>>>>>>> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; >>>>>>>> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; >>>>>>>> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} >>>>>>>> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} >>>>>>>> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} >>>>>>>> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" >>>>>>> >>>>>>> Do you have a link showing the current output with this patch? >>>>>> >>>>>> Hello Simon, >>>>>> >>>>>> here is an example output: >>>>>> >>>>>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 >>>>> >>>>> That's what I was afraid of. The skip output is more than the normal >>>>> output, and if we don't intend to fix it, I'd rather not have >>>>> unactionable warnings in the output. >>>>> >>>>> Having said that, we need to enable SPI flash, FPGA and MMC >>>>> environment tests by the look of it. >>>> >>>> On a different note, I think we should look at: >>>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112376 >>>> and: >>>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112380 >>>> >>>> as it shows that the reason we probably skip the test_fs/test_mkdir.py >>>> tests is that since board is literal, we don't match sandbox on >>>> sandbox_flattree. That answers one outstanding question about why we >>>> skip some tests and not others at least. >>> >>> Hmm yes. >>> >>> It is definitely good to have this output so we can figure out what >>> should not be skipped. >>> >>> But outputting things which we know should be skipped just means we >>> won't notice those that are not supposed to be skipped. How do we >>> handle that? >>> >>> Regards, >>> Simon >>> >> If you have a lines like: >> >> .config feature "cmd_fpga_loadbp" not enabled >> board "qemu_arm64" not supported >> >> you know the test is skipped due to configuration. > > OK then can we avoid printing this useless information by default? > >> >> Other messages clearly tell you that something is not correctly set up: >> >> No env__efi_loader_grub_file binary specified in environment >> got empty parameter set ['env__mmc_dev_config'] > > OK then this is what we should display. There is no switch in pytest to display this selectively. Best regards Heinrich
On Wed, Jun 24, 2020 at 09:17:51AM -0600, Simon Glass wrote: > Hi Heinrich, > > On Wed, 24 Jun 2020 at 07:56, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > On 24.06.20 15:49, Simon Glass wrote: > > > Hi, > > > > > > On Mon, 22 Jun 2020 at 12:46, Tom Rini <trini at konsulko.com> wrote: > > >> > > >> On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: > > >>> Hi Heinrich, > > >>> > > >>> On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > >>>> > > >>>> On 22.06.20 18:17, Simon Glass wrote: > > >>>>> Hi Heinrich, > > >>>>> > > >>>>> On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > >>>>>> > > >>>>>> Call pytest3 with argument -ra to display reason why Python tests are > > >>>>>> skipped. > > >>>>>> > > >>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > >>>>>> --- > > >>>>>> .gitlab-ci.yml | 2 +- > > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > > >>>>>> > > >>>>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > > >>>>>> index f2e491c117..f53098ea5f 100644 > > >>>>>> --- a/.gitlab-ci.yml > > >>>>>> +++ b/.gitlab-ci.yml > > >>>>>> @@ -46,7 +46,7 @@ stages: > > >>>>>> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > > >>>>>> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > > >>>>>> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > > >>>>>> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > > >>>>>> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > > >>>>>> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > > >>>>>> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > > >>>>> > > >>>>> Do you have a link showing the current output with this patch? > > >>>> > > >>>> Hello Simon, > > >>>> > > >>>> here is an example output: > > >>>> > > >>>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 > > >>> > > >>> That's what I was afraid of. The skip output is more than the normal > > >>> output, and if we don't intend to fix it, I'd rather not have > > >>> unactionable warnings in the output. > > >>> > > >>> Having said that, we need to enable SPI flash, FPGA and MMC > > >>> environment tests by the look of it. > > >> > > >> On a different note, I think we should look at: > > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112376 > > >> and: > > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112380 > > >> > > >> as it shows that the reason we probably skip the test_fs/test_mkdir.py > > >> tests is that since board is literal, we don't match sandbox on > > >> sandbox_flattree. That answers one outstanding question about why we > > >> skip some tests and not others at least. > > > > > > Hmm yes. > > > > > > It is definitely good to have this output so we can figure out what > > > should not be skipped. > > > > > > But outputting things which we know should be skipped just means we > > > won't notice those that are not supposed to be skipped. How do we > > > handle that? > > > > > > Regards, > > > Simon > > > > > If you have a lines like: > > > > .config feature "cmd_fpga_loadbp" not enabled > > board "qemu_arm64" not supported > > > > you know the test is skipped due to configuration. > > OK then can we avoid printing this useless information by default? It's not useless information. It's what I pointed to in another part of the thread as to why we're skipping tests we didn't expect to skip. > > Other messages clearly tell you that something is not correctly set up: > > > > No env__efi_loader_grub_file binary specified in environment > > got empty parameter set ['env__mmc_dev_config'] > > OK then this is what we should display. This one is actually one I dug in to a bit, and I don't like how pytest handles. You can't add a custom parameter checker, AFAICT, you can only have empty params be skip or xfail. I think we could condense the output a little bit as @pytest.mark things get condensed, but pytest.skip in a test do not (as it counts line number). That's what got me looking for a way to mark that a config needs to exist, but that isn't supported. But we could condense some of the network related stuff by having a single test / helper for network configuration rather than duplicating it, and then mark network tests as depending on it.
Hi Tom, On Wed, 24 Jun 2020 at 09:53, Tom Rini <trini at konsulko.com> wrote: > > On Wed, Jun 24, 2020 at 09:17:51AM -0600, Simon Glass wrote: > > Hi Heinrich, > > > > On Wed, 24 Jun 2020 at 07:56, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > > > On 24.06.20 15:49, Simon Glass wrote: > > > > Hi, > > > > > > > > On Mon, 22 Jun 2020 at 12:46, Tom Rini <trini at konsulko.com> wrote: > > > >> > > > >> On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: > > > >>> Hi Heinrich, > > > >>> > > > >>> On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > >>>> > > > >>>> On 22.06.20 18:17, Simon Glass wrote: > > > >>>>> Hi Heinrich, > > > >>>>> > > > >>>>> On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > >>>>>> > > > >>>>>> Call pytest3 with argument -ra to display reason why Python tests are > > > >>>>>> skipped. > > > >>>>>> > > > >>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > > >>>>>> --- > > > >>>>>> .gitlab-ci.yml | 2 +- > > > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >>>>>> > > > >>>>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > > > >>>>>> index f2e491c117..f53098ea5f 100644 > > > >>>>>> --- a/.gitlab-ci.yml > > > >>>>>> +++ b/.gitlab-ci.yml > > > >>>>>> @@ -46,7 +46,7 @@ stages: > > > >>>>>> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > > > >>>>>> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > > > >>>>>> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > > > >>>>>> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > > > >>>>>> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > > > >>>>>> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > > > >>>>>> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > > > >>>>> > > > >>>>> Do you have a link showing the current output with this patch? > > > >>>> > > > >>>> Hello Simon, > > > >>>> > > > >>>> here is an example output: > > > >>>> > > > >>>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 > > > >>> > > > >>> That's what I was afraid of. The skip output is more than the normal > > > >>> output, and if we don't intend to fix it, I'd rather not have > > > >>> unactionable warnings in the output. > > > >>> > > > >>> Having said that, we need to enable SPI flash, FPGA and MMC > > > >>> environment tests by the look of it. > > > >> > > > >> On a different note, I think we should look at: > > > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112376 > > > >> and: > > > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112380 > > > >> > > > >> as it shows that the reason we probably skip the test_fs/test_mkdir.py > > > >> tests is that since board is literal, we don't match sandbox on > > > >> sandbox_flattree. That answers one outstanding question about why we > > > >> skip some tests and not others at least. > > > > > > > > Hmm yes. > > > > > > > > It is definitely good to have this output so we can figure out what > > > > should not be skipped. > > > > > > > > But outputting things which we know should be skipped just means we > > > > won't notice those that are not supposed to be skipped. How do we > > > > handle that? > > > > > > > > Regards, > > > > Simon > > > > > > > If you have a lines like: > > > > > > .config feature "cmd_fpga_loadbp" not enabled > > > board "qemu_arm64" not supported > > > > > > you know the test is skipped due to configuration. > > > > OK then can we avoid printing this useless information by default? > > It's not useless information. It's what I pointed to in another part of > the thread as to why we're skipping tests we didn't expect to skip. I thought these ones were intended to be skipped? I'm perhaps just confused about what is going on here. > > > > Other messages clearly tell you that something is not correctly set up: > > > > > > No env__efi_loader_grub_file binary specified in environment > > > got empty parameter set ['env__mmc_dev_config'] > > > > OK then this is what we should display. > > This one is actually one I dug in to a bit, and I don't like how pytest > handles. You can't add a custom parameter checker, AFAICT, you can only > have empty params be skip or xfail. > > I think we could condense the output a little bit as @pytest.mark things > get condensed, but pytest.skip in a test do not (as it counts line > number). That's what got me looking for a way to mark that a config > needs to exist, but that isn't supported. But we could condense some of > the network related stuff by having a single test / helper for network > configuration rather than duplicating it, and then mark network tests as > depending on it. That sounds good. But in general, my point is that we should avoid displaying a message when things are working as intended, only when some action has to be taken. When everything is right, there should not be any warnings or failures IMO. Regards, Simon
On Wed, Jun 24, 2020 at 02:04:37PM -0600, Simon Glass wrote: > Hi Tom, > > On Wed, 24 Jun 2020 at 09:53, Tom Rini <trini at konsulko.com> wrote: > > > > On Wed, Jun 24, 2020 at 09:17:51AM -0600, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Wed, 24 Jun 2020 at 07:56, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > > > > > On 24.06.20 15:49, Simon Glass wrote: > > > > > Hi, > > > > > > > > > > On Mon, 22 Jun 2020 at 12:46, Tom Rini <trini at konsulko.com> wrote: > > > > >> > > > > >> On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: > > > > >>> Hi Heinrich, > > > > >>> > > > > >>> On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > >>>> > > > > >>>> On 22.06.20 18:17, Simon Glass wrote: > > > > >>>>> Hi Heinrich, > > > > >>>>> > > > > >>>>> On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > >>>>>> > > > > >>>>>> Call pytest3 with argument -ra to display reason why Python tests are > > > > >>>>>> skipped. > > > > >>>>>> > > > > >>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > > > >>>>>> --- > > > > >>>>>> .gitlab-ci.yml | 2 +- > > > > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > >>>>>> > > > > >>>>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > > > > >>>>>> index f2e491c117..f53098ea5f 100644 > > > > >>>>>> --- a/.gitlab-ci.yml > > > > >>>>>> +++ b/.gitlab-ci.yml > > > > >>>>>> @@ -46,7 +46,7 @@ stages: > > > > >>>>>> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > > > > >>>>>> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > > > > >>>>>> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > > > > >>>>>> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > > > > >>>>>> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > > > > >>>>>> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > > > > >>>>>> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > > > > >>>>> > > > > >>>>> Do you have a link showing the current output with this patch? > > > > >>>> > > > > >>>> Hello Simon, > > > > >>>> > > > > >>>> here is an example output: > > > > >>>> > > > > >>>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 > > > > >>> > > > > >>> That's what I was afraid of. The skip output is more than the normal > > > > >>> output, and if we don't intend to fix it, I'd rather not have > > > > >>> unactionable warnings in the output. > > > > >>> > > > > >>> Having said that, we need to enable SPI flash, FPGA and MMC > > > > >>> environment tests by the look of it. > > > > >> > > > > >> On a different note, I think we should look at: > > > > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112376 > > > > >> and: > > > > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112380 > > > > >> > > > > >> as it shows that the reason we probably skip the test_fs/test_mkdir.py > > > > >> tests is that since board is literal, we don't match sandbox on > > > > >> sandbox_flattree. That answers one outstanding question about why we > > > > >> skip some tests and not others at least. > > > > > > > > > > Hmm yes. > > > > > > > > > > It is definitely good to have this output so we can figure out what > > > > > should not be skipped. > > > > > > > > > > But outputting things which we know should be skipped just means we > > > > > won't notice those that are not supposed to be skipped. How do we > > > > > handle that? > > > > > > > > > > Regards, > > > > > Simon > > > > > > > > > If you have a lines like: > > > > > > > > .config feature "cmd_fpga_loadbp" not enabled > > > > board "qemu_arm64" not supported > > > > > > > > you know the test is skipped due to configuration. > > > > > > OK then can we avoid printing this useless information by default? > > > > It's not useless information. It's what I pointed to in another part of > > the thread as to why we're skipping tests we didn't expect to skip. > > I thought these ones were intended to be skipped? I'm perhaps just > confused about what is going on here. We have a number of "sandbox" targets which should be running the FS tests, but do not. Figuring out why has been low on my TODO list, but is very clear with this extra information. Still haven't however figured out why we do see this: https://travis-ci.org/github/u-boot/u-boot/jobs/702114991 (which is, all FS tests run). > > > > Other messages clearly tell you that something is not correctly set up: > > > > > > > > No env__efi_loader_grub_file binary specified in environment > > > > got empty parameter set ['env__mmc_dev_config'] > > > > > > OK then this is what we should display. > > > > This one is actually one I dug in to a bit, and I don't like how pytest > > handles. You can't add a custom parameter checker, AFAICT, you can only > > have empty params be skip or xfail. > > > > I think we could condense the output a little bit as @pytest.mark things > > get condensed, but pytest.skip in a test do not (as it counts line > > number). That's what got me looking for a way to mark that a config > > needs to exist, but that isn't supported. But we could condense some of > > the network related stuff by having a single test / helper for network > > configuration rather than duplicating it, and then mark network tests as > > depending on it. > > That sounds good. > > But in general, my point is that we should avoid displaying a message > when things are working as intended, only when some action has to be > taken. When everything is right, there should not be any warnings or > failures IMO. Sure. But this is CI where it's better to have more information rather than less and explaining why we're skipping tests but in perhaps a bit too verbose fashion will perhaps motivate cleaning up and re-organizing our tests. That we have N "no network configured" items rather than perhaps 2 (one for "not configured", one line of N-1 skipped due to not configured test was skipped) today is because there's little reason to do otherwise. An 's' is an 's'. Even if it means that the EFI tests duplicate the network stuff. Even if it means that the EFI tests about networking can be fragile, and the networking tests can also be fragile, as when you pass in specific tests to run you can end up without the network configured. I also suspect that having a valid run where we have 93 runs and 213 skips (qemu-ppce500) is violating some best practices for pytest. I very much see why we don't want to include this in "make qcheck" and similar. But for CI this is something we want.
Hi Tom, On Fri, 26 Jun 2020 at 14:16, Tom Rini <trini at konsulko.com> wrote: > > On Wed, Jun 24, 2020 at 02:04:37PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 24 Jun 2020 at 09:53, Tom Rini <trini at konsulko.com> wrote: > > > > > > On Wed, Jun 24, 2020 at 09:17:51AM -0600, Simon Glass wrote: > > > > Hi Heinrich, > > > > > > > > On Wed, 24 Jun 2020 at 07:56, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > > > > > > > On 24.06.20 15:49, Simon Glass wrote: > > > > > > Hi, > > > > > > > > > > > > On Mon, 22 Jun 2020 at 12:46, Tom Rini <trini at konsulko.com> wrote: > > > > > >> > > > > > >> On Mon, Jun 22, 2020 at 12:23:35PM -0600, Simon Glass wrote: > > > > > >>> Hi Heinrich, > > > > > >>> > > > > > >>> On Mon, 22 Jun 2020 at 10:40, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > > >>>> > > > > > >>>> On 22.06.20 18:17, Simon Glass wrote: > > > > > >>>>> Hi Heinrich, > > > > > >>>>> > > > > > >>>>> On Mon, 22 Jun 2020 at 10:07, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > > >>>>>> > > > > > >>>>>> Call pytest3 with argument -ra to display reason why Python tests are > > > > > >>>>>> skipped. > > > > > >>>>>> > > > > > >>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > > > > >>>>>> --- > > > > > >>>>>> .gitlab-ci.yml | 2 +- > > > > > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > >>>>>> > > > > > >>>>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > > > > > >>>>>> index f2e491c117..f53098ea5f 100644 > > > > > >>>>>> --- a/.gitlab-ci.yml > > > > > >>>>>> +++ b/.gitlab-ci.yml > > > > > >>>>>> @@ -46,7 +46,7 @@ stages: > > > > > >>>>>> # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not > > > > > >>>>>> - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; > > > > > >>>>>> export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; > > > > > >>>>>> - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} > > > > > >>>>>> + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} > > > > > >>>>>> ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} > > > > > >>>>>> --build-dir "$UBOOT_TRAVIS_BUILD_DIR" > > > > > >>>>> > > > > > >>>>> Do you have a link showing the current output with this patch? > > > > > >>>> > > > > > >>>> Hello Simon, > > > > > >>>> > > > > > >>>> here is an example output: > > > > > >>>> > > > > > >>>> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112385 > > > > > >>> > > > > > >>> That's what I was afraid of. The skip output is more than the normal > > > > > >>> output, and if we don't intend to fix it, I'd rather not have > > > > > >>> unactionable warnings in the output. > > > > > >>> > > > > > >>> Having said that, we need to enable SPI flash, FPGA and MMC > > > > > >>> environment tests by the look of it. > > > > > >> > > > > > >> On a different note, I think we should look at: > > > > > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112376 > > > > > >> and: > > > > > >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/112380 > > > > > >> > > > > > >> as it shows that the reason we probably skip the test_fs/test_mkdir.py > > > > > >> tests is that since board is literal, we don't match sandbox on > > > > > >> sandbox_flattree. That answers one outstanding question about why we > > > > > >> skip some tests and not others at least. > > > > > > > > > > > > Hmm yes. > > > > > > > > > > > > It is definitely good to have this output so we can figure out what > > > > > > should not be skipped. > > > > > > > > > > > > But outputting things which we know should be skipped just means we > > > > > > won't notice those that are not supposed to be skipped. How do we > > > > > > handle that? > > > > > > > > > > > > Regards, > > > > > > Simon > > > > > > > > > > > If you have a lines like: > > > > > > > > > > .config feature "cmd_fpga_loadbp" not enabled > > > > > board "qemu_arm64" not supported > > > > > > > > > > you know the test is skipped due to configuration. > > > > > > > > OK then can we avoid printing this useless information by default? > > > > > > It's not useless information. It's what I pointed to in another part of > > > the thread as to why we're skipping tests we didn't expect to skip. > > > > I thought these ones were intended to be skipped? I'm perhaps just > > confused about what is going on here. > > We have a number of "sandbox" targets which should be running the FS > tests, but do not. Figuring out why has been low on my TODO list, but > is very clear with this extra information. > > Still haven't however figured out why we do see this: > https://travis-ci.org/github/u-boot/u-boot/jobs/702114991 (which is, all > FS tests run). > > > > > > Other messages clearly tell you that something is not correctly set up: > > > > > > > > > > No env__efi_loader_grub_file binary specified in environment > > > > > got empty parameter set ['env__mmc_dev_config'] > > > > > > > > OK then this is what we should display. > > > > > > This one is actually one I dug in to a bit, and I don't like how pytest > > > handles. You can't add a custom parameter checker, AFAICT, you can only > > > have empty params be skip or xfail. > > > > > > I think we could condense the output a little bit as @pytest.mark things > > > get condensed, but pytest.skip in a test do not (as it counts line > > > number). That's what got me looking for a way to mark that a config > > > needs to exist, but that isn't supported. But we could condense some of > > > the network related stuff by having a single test / helper for network > > > configuration rather than duplicating it, and then mark network tests as > > > depending on it. > > > > That sounds good. > > > > But in general, my point is that we should avoid displaying a message > > when things are working as intended, only when some action has to be > > taken. When everything is right, there should not be any warnings or > > failures IMO. > > Sure. But this is CI where it's better to have more information rather > than less and explaining why we're skipping tests but in perhaps a bit > too verbose fashion will perhaps motivate cleaning up and re-organizing > our tests. That we have N "no network configured" items rather than > perhaps 2 (one for "not configured", one line of N-1 skipped due to not > configured test was skipped) today is because there's little reason to > do otherwise. An 's' is an 's'. Even if it means that the EFI tests > duplicate the network stuff. Even if it means that the EFI tests about > networking can be fragile, and the networking tests can also be fragile, > as when you pass in specific tests to run you can end up without the > network configured. I also suspect that having a valid run where we > have 93 runs and 213 skips (qemu-ppce500) is violating some > best practices for pytest. > > I very much see why we don't want to include this in "make qcheck" and > similar. But for CI this is something we want. OK well let's go with it and see if we get the hoped-for clean-up. Merging tests is a double-edge sword though. It is nice to start a test from a known initial state rather than lumping lots of code together that depends on previous code. I think it would be better to collect together the tests blocked by a condition and show them in a list. Reviewed-by: Simon Glass <sjg at chromium.org> Regards, Simon
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f2e491c117..f53098ea5f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -46,7 +46,7 @@ stages: # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; - ./test/py/test.py --bd ${TEST_PY_BD} ${TEST_PY_ID} + ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID} ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"} --build-dir "$UBOOT_TRAVIS_BUILD_DIR"
Call pytest3 with argument -ra to display reason why Python tests are skipped. Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.27.0