Message-ID: <bc8ab54d427e62f17f46022980bfcaf392e0a0c3.camel@kernel.org>
Subject: Re: [pynfs RFC PATCH] nfs4.0/testserver.py: don't return an error
 when tests fail
From:   Jeff Layton <jlayton@kernel.org>
To:     Frank Filz <ffilzlnx@mindspring.com>, bfields@fieldses.org,
        dai.ngo@oracle.com
Cc:     linux-nfs@vger.kernel.org
Date:   Thu, 23 Feb 2023 12:08:19 -0500
In-Reply-To: <029901d947a3$0dd00c00$29702400$@mindspring.com>
References: <20230222134952.32851-1-jlayton@kernel.org>
         <029901d947a3$0dd00c00$29702400$@mindspring.com>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Thu, 2023-02-23 at 08:22 -0800, Frank Filz wrote:
> > From: Jeff Layton [mailto:jlayton@kernel.org]
> =A0
> > This script was originally changed in eb3ba0b60055 ("Have
> > testserver.py
> have
> > non-zero exit code if any tests fail"), but the same change wasn't
> > made to
> the
> > 4.1 testserver.py script.
> >=20
> > There also wasn't much explanation for it, and it makes it difficult
> > to
> tell
> > whether the test harness itself failed, or whether there was a
> > failure in
> a
> > requested test.
> >=20
> > Stop the 4.0 testserver.py from exiting with an error code when a
> > test
> fails, so
> > that a successful return means only that the test harness itself
> > worked,
> not that
> > every requested test passed.
> >=20
> > Cc: Frank Filz <ffilzlnx@mindspring.com>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > =A0nfs4.0/testserver.py | 2 --
> > =A01 file changed, 2 deletions(-)
> >=20
> > I'm not sure about this one. I've worked around this in kdevops for
> > now,
> but it
> > would really be preferable if it worked this way, imo. If this isn't
> acceptable,
> > maybe we can add a new option that enables this behavior?
> >=20
> > Frank, what was the original rationale for eb3ba0b60055 ?
>=20
> We needed a way for CI to easily detect failure of pynfs. I'm not sure
> how
> helpful it is since Ganesha does fail some tests...
>=20
> It might be helpful to have some helpers for CI to use, or an option
> that
> causes pynfs to report in a way that's much easier for CI to determine
> if
> pynfs succeeded or not.
>=20

That's exactly the issue I had when working with this for kdevops. It
runs testserver.py via ansible, and when it gets a non-zero exit code,
the run aborts without doing anything further. I have it ignoring the
return code from testserver.py for now, but that's not ideal since I
can't catch actual problems with the test harness.

I have testserver.py output the result to JSON, and then analyze that to
see if anything failed. That also gives us what you were asking for in
your other email -- the ability to filter out known failures. Here's
what I have so far, but I'd like to expand it to highlight other
behavior changes:
=20
https://github.com/linux-kdevops/kdevops/blob/master/scripts/workflows/pynf=
s/check_pynfs_results.py

It may make sense to move that script into pynfs itself.

If there is CI that depends on this behavior, then I'd be interested to
hear how they are dealing with failed tests. Do they just not run the
tests that always fail?

> Hmm, one thing that would help is to be able to flag a set of tests
> that
> should not constitute a CI failure (known errors) but we want to keep
> running them because of what they exercise, or to more readily detect
> that
> they have been fixed.
>=20

The right way to do that is the same way that xfstests works. You test
for the conditions being favorable for a test and then just skip it if
they aren't.

> > diff --git a/nfs4.0/testserver.py b/nfs4.0/testserver.py index
> > f2c41568e5c7..4f4286daa657 100755
> > --- a/nfs4.0/testserver.py
> > +++ b/nfs4.0/testserver.py
> > @@ -387,8 +387,6 @@ def main():
> >=20
> > =A0=A0=A0=A0=A0if nfail < 0:
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0sys.exit(3)
> > -    if nfail > 0:
> > -        sys.exit(2)
> >=20
> > =A0if __name__ =3D=3D "__main__":
> > =A0=A0=A0=A0=A0main()
> > --
> > 2.39.2
>=20

--=20
Jeff Layton <jlayton@kernel.org>