[PATCH v3 06/19] test: Avoid failing skipped tests

[Simon Glass]

Hi Tom,

On Wed, 26 Jun 2024 at 02:00, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Tom,
>
> On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Tue, Jun 25, 2024 at 01:38:00PM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> > > >
> > > > > When a test returns -EAGAIN this should not be considered a failure.
> > > > > Fix what seems to be a problem case, where the pytests see a failure
> > > > > when a test has merely been skipped.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > ---
> > > > >
> > > > > (no changes since v1)
> > > > >
> > > > >  test/test-main.c | 16 +++++++++++-----
> > > > >  1 file changed, 11 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/test/test-main.c b/test/test-main.c
> > > > > index 3fa6f6e32ec..cda1a186390 100644
> > > > > --- a/test/test-main.c
> > > > > +++ b/test/test-main.c
> > > > > @@ -448,7 +448,7 @@ static int ut_run_test(struct unit_test_state *uts, struct unit_test *test,
> > > > >  static int ut_run_test_live_flat(struct unit_test_state *uts,
> > > > >                                struct unit_test *test)
> > > > >  {
> > > > > -     int runs;
> > > > > +     int runs, ret;
> > > > >
> > > > >       if ((test->flags & UT_TESTF_OTHER_FDT) && !IS_ENABLED(CONFIG_SANDBOX))
> > > > >               return skip_test(uts);
> > > > > @@ -458,8 +458,11 @@ static int ut_run_test_live_flat(struct unit_test_state *uts,
> > > > >       if (CONFIG_IS_ENABLED(OF_LIVE)) {
> > > > >               if (!(test->flags & UT_TESTF_FLAT_TREE)) {
> > > > >                       uts->of_live = true;
> > > > > -                     ut_assertok(ut_run_test(uts, test, test->name));
> > > > > -                     runs++;
> > > > > +                     ret = ut_run_test(uts, test, test->name);
> > > > > +                     if (ret != -EAGAIN) {
> > > > > +                             ut_assertok(ret);
> > > > > +                             runs++;
> > > > > +                     }
> > > > >               }
> > > > >       }
> > > > >
> > > > > @@ -483,8 +486,11 @@ static int ut_run_test_live_flat(struct unit_test_state *uts,
> > > > >           (!runs || ut_test_run_on_flattree(test)) &&
> > > > >           !(gd->flags & GD_FLG_FDT_CHANGED)) {
> > > > >               uts->of_live = false;
> > > > > -             ut_assertok(ut_run_test(uts, test, test->name));
> > > > > -             runs++;
> > > > > +             ret = ut_run_test(uts, test, test->name);
> > > > > +             if (ret != -EAGAIN) {
> > > > > +                     ut_assertok(ret);
> > > > > +                     runs++;
> > > > > +             }
> > > > >       }
> > > > >
> > > > >       return 0;
> > > >
> > > > How did you trigger this case exactly?
> > >
> > > I noticed this in CI, where some skipped tests were shown as failed in
> > > the log, even though they were not counted as failures in the final
> > > results.
> >
> > That's really really strange, do you have an example log or something
> > around still?
>
> This happens on snow, which is (maybe) the only real board that
> defines CONFIG_UNIT_TEST
>
> test/py/tests/test_ut.py sssnow # ut bdinfo bdinfo_test_eth
> Test: bdinfo_test_eth: bdinfo.c
> Skipping: Console recording disabled
> test/test-main.c:486, ut_run_test_live_flat(): 0 == ut_run_test(uts,
> test, test->name): Expected 0x0 (0), got 0xfffffff5 (-11)
> Test bdinfo_test_eth failed 1 times
> Skipped: 1, Failures: 1
> snow # F+u-boot-test-reset snow snow
>
> For this particular mechanism (-EAGAIN returned by test_pre_run()) , I
> think a better fix would be to squash the error in ut_run_test(), as
> is done when -EAGAIN is returned in the body of the test. I'll update
> that. I cannot see any other way this could happen, but we can always
> deal with it later if it does.

No, that doesn't work, since the failure counting happens in the
caller. This patch seems to be the right fix, so I'll send it again
with a better commit message.

Regards,
Simon