On Thu, Aug 22, 2024 at 08:22:59AM -0600, Simon Glass wrote:
> Hi Tom,
>
> On Thu, 22 Aug 2024 at 08:11, Tom Rini <tr...@konsulko.com> wrote:
> >
> > On Wed, Aug 21, 2024 at 09:00:33PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 26 Jun 2024 at 02:00, Simon Glass <s...@chromium.org> wrote:
> > > >
> > > > Hi Tom,
> > > >
> > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <tr...@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 <tr...@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 <s...@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.
> >
> > And I'll try and test it on Pi as with the series I posted and so
> > UNIT_TEST compiles on there, I got what looks like the same failure.
>
> OK ta.
>
> It would be good to get the Labgrid series into -next as it fixes
> quite a few problems on snow for me. It will provide a better base for
> solving other problems.I want to pick some of it for sure. But it also broke my lab support (before an office power cycle left it in an odd state and I need to go and confirm what's needed / not needed again to bring the boards up). -- Tom
signature.asc
Description: PGP signature
