[pynfs RFC PATCH] testserver.py: special-case the "all" flag

[pynfs RFC PATCH] testserver.py: special-case the "all" flag

[Jeff Layton]

The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0, the "all"
flag is supposed to run all of the "standard" tests. For v4.1 "all" is
documented to run all of the tests, but it actually doesn't since not
every tests has "all" in its FLAGS: field.

I move that we change this. If I say that I want to run "all", then I
really do want to run _all_ of the tests. Ensure that every test has the
"all" flag set.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 nfs4.1/testmod.py | 2 ++
 1 file changed, 2 insertions(+)

If this is unacceptable, then an alternative could be to add a new
(similarly special-cased) "everything" flag.

diff --git a/nfs4.1/testmod.py b/nfs4.1/testmod.py
index 11e759d673fd..7b3bac543084 100644
--- a/nfs4.1/testmod.py
+++ b/nfs4.1/testmod.py
@@ -386,6 +386,8 @@ def createtests(testdir):
     for t in tests:
 ##         if not t.flags_list:
 ##             raise RuntimeError("%s has no flags" % t.fullname)
+        if "all" not in t.flags_list:
+            t.flags_list.append("all")
         for f in t.flags_list:
             if f not in flag_dict:
                 flag_dict[f] = bit
-- 
2.39.2

Re: [pynfs RFC PATCH] testserver.py: special-case the "all" flag

[J. Bruce Fields]

On Wed, Feb 22, 2023 at 01:20:43PM -0500, Jeff Layton wrote:
> The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0, the "all"
> flag is supposed to run all of the "standard" tests. For v4.1 "all" is
> documented to run all of the tests, but it actually doesn't since not
> every tests has "all" in its FLAGS: field.
> 
> I move that we change this. If I say that I want to run "all", then I
> really do want to run _all_ of the tests. Ensure that every test has the
> "all" flag set.

In some (all?) cases where the "all" flag was left off, it was
intentional.

We try not to flag spec-compliant servers as failing, because people are
sometimes a little careless about "fixing" failures that in their
particular case really shouldn't be fixed.  But sometimes it's still
useful to have a test that goes somewhat beyond the spec.

There might be other ways to handle that kind of test, but it would need
some more thought.

--b.

> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  nfs4.1/testmod.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> If this is unacceptable, then an alternative could be to add a new
> (similarly special-cased) "everything" flag.
> 
> diff --git a/nfs4.1/testmod.py b/nfs4.1/testmod.py
> index 11e759d673fd..7b3bac543084 100644
> --- a/nfs4.1/testmod.py
> +++ b/nfs4.1/testmod.py
> @@ -386,6 +386,8 @@ def createtests(testdir):
>      for t in tests:
>  ##         if not t.flags_list:
>  ##             raise RuntimeError("%s has no flags" % t.fullname)
> +        if "all" not in t.flags_list:
> +            t.flags_list.append("all")
>          for f in t.flags_list:
>              if f not in flag_dict:
>                  flag_dict[f] = bit
> -- 
> 2.39.2

RE: [pynfs RFC PATCH] testserver.py: special-case the "all" flag

[Frank Filz]

> -----Original Message-----
> From: J. Bruce Fields [mailto:bfields@fieldses.org]
 
> On Wed, Feb 22, 2023 at 01:20:43PM -0500, Jeff Layton wrote:
> > The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0, the "all"
> > flag is supposed to run all of the "standard" tests. For v4.1 "all" is
> > documented to run all of the tests, but it actually doesn't since not
> > every tests has "all" in its FLAGS: field.
> >
> > I move that we change this. If I say that I want to run "all", then I
> > really do want to run _all_ of the tests. Ensure that every test has
> > the "all" flag set.
> 
> In some (all?) cases where the "all" flag was left off, it was
intentional.
> 
> We try not to flag spec-compliant servers as failing, because people are
> sometimes a little careless about "fixing" failures that in their
particular case
> really shouldn't be fixed.  But sometimes it's still useful to have a test
that goes
> somewhat beyond the spec.
> 
> There might be other ways to handle that kind of test, but it would need
some
> more thought.

There are also tests that are server specific, for example, there are tests
that are Ganesha specific. For example, look at LOCK11g. LOCK11 attempts to
make a bad stateid, and it sort of "knows" how the server formats its
stateids. There is a function makeBadIDganesha that "knows" how Ganesha
forms its stateids and forms a bad one that will definitely result in an
NSF4ERR_BAD_STATEID.

There are also a number of tests that require specific setup bits to work
that someone who just fires up a server won't have set up. I think there are
some that expect specific exports, or second exports or other specific
aspects of the pseudofs. There are tests that deal with server reboots.

At the least, I want a way to specify to run all the tests that are expected
to pass on a server that has been set up without any special cases, and that
are expected to pass on Linux.

The Ganesha tag, in addition to specifying a few Ganesha only tests,
specifies several tests that may not pass on Linux in general (so they
aren't in all) but do pass on Ganesha.

On the flip side, Ganesha lives with a handful of known failures...

Frank

> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  nfs4.1/testmod.py | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > If this is unacceptable, then an alternative could be to add a new
> > (similarly special-cased) "everything" flag.
> >
> > diff --git a/nfs4.1/testmod.py b/nfs4.1/testmod.py index
> > 11e759d673fd..7b3bac543084 100644
> > --- a/nfs4.1/testmod.py
> > +++ b/nfs4.1/testmod.py
> > @@ -386,6 +386,8 @@ def createtests(testdir):
> >      for t in tests:
> >  ##         if not t.flags_list:
> >  ##             raise RuntimeError("%s has no flags" % t.fullname)
> > +        if "all" not in t.flags_list:
> > +            t.flags_list.append("all")
> >          for f in t.flags_list:
> >              if f not in flag_dict:
> >                  flag_dict[f] = bit
> > --
> > 2.39.2

Re: [pynfs RFC PATCH] testserver.py: special-case the "all" flag

[Chuck Lever III]

> On Feb 23, 2023, at 10:19 AM, J. Bruce Fields <bfields@fieldses.org> wrote:
> 
> On Wed, Feb 22, 2023 at 01:20:43PM -0500, Jeff Layton wrote:
>> The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0, the "all"
>> flag is supposed to run all of the "standard" tests. For v4.1 "all" is
>> documented to run all of the tests, but it actually doesn't since not
>> every tests has "all" in its FLAGS: field.
>> 
>> I move that we change this. If I say that I want to run "all", then I
>> really do want to run _all_ of the tests. Ensure that every test has the
>> "all" flag set.
> 
> In some (all?) cases where the "all" flag was left off, it was
> intentional.
> 
> We try not to flag spec-compliant servers as failing, because people are
> sometimes a little careless about "fixing" failures that in their
> particular case really shouldn't be fixed.  But sometimes it's still
> useful to have a test that goes somewhat beyond the spec.
> 
> There might be other ways to handle that kind of test, but it would need
> some more thought.

We could use a different name for "all" since it doesn't actually
run /all/ tests. Jeff suggested "standard", which seems sensible.

Also, we could add test categories specifically for particular server
implementations, if that's interesting to folks.


> --b.
> 
>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>> ---
>> nfs4.1/testmod.py | 2 ++
>> 1 file changed, 2 insertions(+)
>> 
>> If this is unacceptable, then an alternative could be to add a new
>> (similarly special-cased) "everything" flag.
>> 
>> diff --git a/nfs4.1/testmod.py b/nfs4.1/testmod.py
>> index 11e759d673fd..7b3bac543084 100644
>> --- a/nfs4.1/testmod.py
>> +++ b/nfs4.1/testmod.py
>> @@ -386,6 +386,8 @@ def createtests(testdir):
>>     for t in tests:
>> ##         if not t.flags_list:
>> ##             raise RuntimeError("%s has no flags" % t.fullname)
>> +        if "all" not in t.flags_list:
>> +            t.flags_list.append("all")
>>         for f in t.flags_list:
>>             if f not in flag_dict:
>>                 flag_dict[f] = bit
>> -- 
>> 2.39.2

--
Chuck Lever

RE: [pynfs RFC PATCH] testserver.py: special-case the "all" flag

[Frank Filz]

> -----Original Message-----
> From: Chuck Lever III [mailto:chuck.lever@oracle.com]
> Sent: Thursday, February 23, 2023 8:22 AM
> To: Bruce Fields <bfields@fieldses.org>
> Cc: Jeff Layton <jlayton@kernel.org>; Dai Ngo <dai.ngo@oracle.com>; Linux
> NFS Mailing List <linux-nfs@vger.kernel.org>; Calum Mackay
> <calum.mackay@oracle.com>; Frank Filz <ffilzlnx@mindspring.com>
> Subject: Re: [pynfs RFC PATCH] testserver.py: special-case the "all" flag
> 
> 
> 
> > On Feb 23, 2023, at 10:19 AM, J. Bruce Fields <bfields@fieldses.org>
wrote:
> >
> > On Wed, Feb 22, 2023 at 01:20:43PM -0500, Jeff Layton wrote:
> >> The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0, the
"all"
> >> flag is supposed to run all of the "standard" tests. For v4.1 "all"
> >> is documented to run all of the tests, but it actually doesn't since
> >> not every tests has "all" in its FLAGS: field.
> >>
> >> I move that we change this. If I say that I want to run "all", then I
> >> really do want to run _all_ of the tests. Ensure that every test has
> >> the "all" flag set.
> >
> > In some (all?) cases where the "all" flag was left off, it was
> > intentional.
> >
> > We try not to flag spec-compliant servers as failing, because people
> > are sometimes a little careless about "fixing" failures that in their
> > particular case really shouldn't be fixed.  But sometimes it's still
> > useful to have a test that goes somewhat beyond the spec.
> >
> > There might be other ways to handle that kind of test, but it would
> > need some more thought.
> 
> We could use a different name for "all" since it doesn't actually run
/all/ tests.
> Jeff suggested "standard", which seems sensible.

The challenge with changing this is all the CI scripts and other testing
scripts out there that use all. If all suddenly changed, server maintainers
might get a deluge of bug reports for failing odd test cases no one
necessarily expected to work...

> Also, we could add test categories specifically for particular server
> implementations, if that's interesting to folks.

I have already done so with a ganesha tag...

Literally all anyone has to do is start using a new tag so it's a trivial
thing for developers to add.

> 
> > --b.
> >
> >> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> >> ---
> >> nfs4.1/testmod.py | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> If this is unacceptable, then an alternative could be to add a new
> >> (similarly special-cased) "everything" flag.
> >>
> >> diff --git a/nfs4.1/testmod.py b/nfs4.1/testmod.py index
> >> 11e759d673fd..7b3bac543084 100644
> >> --- a/nfs4.1/testmod.py
> >> +++ b/nfs4.1/testmod.py
> >> @@ -386,6 +386,8 @@ def createtests(testdir):
> >>     for t in tests:
> >> ##         if not t.flags_list:
> >> ##             raise RuntimeError("%s has no flags" % t.fullname)
> >> +        if "all" not in t.flags_list:
> >> +            t.flags_list.append("all")
> >>         for f in t.flags_list:
> >>             if f not in flag_dict:
> >>                 flag_dict[f] = bit
> >> --
> >> 2.39.2
> 
> --
> Chuck Lever
> 

Re: [pynfs RFC PATCH] testserver.py: special-case the "all" flag

[Jeff Layton]

On Thu, 2023-02-23 at 08:26 -0800, Frank Filz wrote:
> 
> > -----Original Message-----
> > From: Chuck Lever III [mailto:chuck.lever@oracle.com]
> > Sent: Thursday, February 23, 2023 8:22 AM
> > To: Bruce Fields <bfields@fieldses.org>
> > Cc: Jeff Layton <jlayton@kernel.org>; Dai Ngo <dai.ngo@oracle.com>; Linux
> > NFS Mailing List <linux-nfs@vger.kernel.org>; Calum Mackay
> > <calum.mackay@oracle.com>; Frank Filz <ffilzlnx@mindspring.com>
> > Subject: Re: [pynfs RFC PATCH] testserver.py: special-case the "all" flag
> > 
> > 
> > 
> > > On Feb 23, 2023, at 10:19 AM, J. Bruce Fields <bfields@fieldses.org>
> wrote:
> > > 
> > > On Wed, Feb 22, 2023 at 01:20:43PM -0500, Jeff Layton wrote:
> > > > The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0, the
> "all"
> > > > flag is supposed to run all of the "standard" tests. For v4.1 "all"
> > > > is documented to run all of the tests, but it actually doesn't since
> > > > not every tests has "all" in its FLAGS: field.
> > > > 
> > > > I move that we change this. If I say that I want to run "all", then I
> > > > really do want to run _all_ of the tests. Ensure that every test has
> > > > the "all" flag set.
> > > 
> > > In some (all?) cases where the "all" flag was left off, it was
> > > intentional.
> > > 
> > > We try not to flag spec-compliant servers as failing, because people
> > > are sometimes a little careless about "fixing" failures that in their
> > > particular case really shouldn't be fixed.  But sometimes it's still
> > > useful to have a test that goes somewhat beyond the spec.
> > > 
> > > There might be other ways to handle that kind of test, but it would
> > > need some more thought.
> > 
> > We could use a different name for "all" since it doesn't actually run
> /all/ tests.
> > Jeff suggested "standard", which seems sensible.
> 
> The challenge with changing this is all the CI scripts and other testing
> scripts out there that use all. If all suddenly changed, server maintainers
> might get a deluge of bug reports for failing odd test cases no one
> necessarily expected to work...
> 

Are those CI systems pulling down the upstream tree to run? How do they
contend with new tests that might suddenly show up as part of "all"?

> > Also, we could add test categories specifically for particular server
> > implementations, if that's interesting to folks.
> 
> I have already done so with a ganesha tag...
> 
> Literally all anyone has to do is start using a new tag so it's a trivial
> thing for developers to add.
> 

The problem is that you have to add the tag to hundreds of tests. It's
scriptable, sure, but if you want to be at all selective, it's not
trivial.

That said, I'm open to doing something different here. Maybe we could
declare a new "everything" special case instead? It's confusing naming,
but that would at least preserve the existing behavior for those CI
systems.

> > 
> > > --b.
> > > 
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > > nfs4.1/testmod.py | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > > 
> > > > If this is unacceptable, then an alternative could be to add a new
> > > > (similarly special-cased) "everything" flag.
> > > > 
> > > > diff --git a/nfs4.1/testmod.py b/nfs4.1/testmod.py index
> > > > 11e759d673fd..7b3bac543084 100644
> > > > --- a/nfs4.1/testmod.py
> > > > +++ b/nfs4.1/testmod.py
> > > > @@ -386,6 +386,8 @@ def createtests(testdir):
> > > >     for t in tests:
> > > > ##         if not t.flags_list:
> > > > ##             raise RuntimeError("%s has no flags" % t.fullname)
> > > > +        if "all" not in t.flags_list:
> > > > +            t.flags_list.append("all")
> > > >         for f in t.flags_list:
> > > >             if f not in flag_dict:
> > > >                 flag_dict[f] = bit
> > > > --
> > > > 2.39.2
> > 
> > --
> > Chuck Lever
> > 
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [pynfs RFC PATCH] testserver.py: special-case the "all" flag

[Mkrtchyan, Tigran]

[-- Attachment #1: Type: text/plain, Size: 6347 bytes --]



----- Original Message -----
> From: "Jeff Layton" <jlayton@kernel.org>
> To: "Frank Filz" <ffilzlnx@mindspring.com>, "Chuck Lever III" <chuck.lever@oracle.com>, "J. Bruce Fields"
> <bfields@fieldses.org>
> Cc: "Dai Ngo" <dai.ngo@oracle.com>, "Linux NFS Mailing List" <linux-nfs@vger.kernel.org>, "Calum Mackay"
> <calum.mackay@oracle.com>
> Sent: Thursday, 23 February, 2023 18:10:47
> Subject: Re: [pynfs RFC PATCH] testserver.py: special-case the "all" flag

> On Thu, 2023-02-23 at 08:26 -0800, Frank Filz wrote:
>> 
>> > -----Original Message-----
>> > From: Chuck Lever III [mailto:chuck.lever@oracle.com]
>> > Sent: Thursday, February 23, 2023 8:22 AM
>> > To: Bruce Fields <bfields@fieldses.org>
>> > Cc: Jeff Layton <jlayton@kernel.org>; Dai Ngo <dai.ngo@oracle.com>; Linux
>> > NFS Mailing List <linux-nfs@vger.kernel.org>; Calum Mackay
>> > <calum.mackay@oracle.com>; Frank Filz <ffilzlnx@mindspring.com>
>> > Subject: Re: [pynfs RFC PATCH] testserver.py: special-case the "all" flag
>> > 
>> > 
>> > 
>> > > On Feb 23, 2023, at 10:19 AM, J. Bruce Fields <bfields@fieldses.org>
>> wrote:
>> > > 
>> > > On Wed, Feb 22, 2023 at 01:20:43PM -0500, Jeff Layton wrote:
>> > > > The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0, the
>> "all"
>> > > > flag is supposed to run all of the "standard" tests. For v4.1 "all"
>> > > > is documented to run all of the tests, but it actually doesn't since
>> > > > not every tests has "all" in its FLAGS: field.
>> > > > 
>> > > > I move that we change this. If I say that I want to run "all", then I
>> > > > really do want to run _all_ of the tests. Ensure that every test has
>> > > > the "all" flag set.
>> > > 
>> > > In some (all?) cases where the "all" flag was left off, it was
>> > > intentional.
>> > > 
>> > > We try not to flag spec-compliant servers as failing, because people
>> > > are sometimes a little careless about "fixing" failures that in their
>> > > particular case really shouldn't be fixed.  But sometimes it's still
>> > > useful to have a test that goes somewhat beyond the spec.
>> > > 
>> > > There might be other ways to handle that kind of test, but it would
>> > > need some more thought.
>> > 
>> > We could use a different name for "all" since it doesn't actually run
>> /all/ tests.
>> > Jeff suggested "standard", which seems sensible.
>> 
>> The challenge with changing this is all the CI scripts and other testing
>> scripts out there that use all. If all suddenly changed, server maintainers
>> might get a deluge of bug reports for failing odd test cases no one
>> necessarily expected to work...
>> 
> 
> Are those CI systems pulling down the upstream tree to run? How do they
> contend with new tests that might suddenly show up as part of "all"?

We are actually explicitly disabling some tests that are part of the `all`

```
./testserver.py -v --noinit --xml=/scratch/jenkins/root/workspace/pynfs-test/xunit-report-v40.xml nfs-lab:/data/nfs all noACC2a noACC2b noACC2c noACC2d noACC2f noACC2r noACC2s noCID1 noCID2 noCID4a noCID4b noCID4c noCID4d noCID4e noCLOSE10 noCLOSE12 noCLOSE5 noCLOSE6 noCLOSE8 noCLOSE9 noCMT1aa noCMT1b noCMT1c noCMT1d noCMT1e noCMT1f noCMT2a noCMT2b noCMT2c noCMT2d noCMT2f noCMT2s noCMT3 noCMT4 noCR12 noLKT1 noLKT2a noLKT2b noLKT2c noLKT2d noLKT2f noLKT2s noLKT3 noLKT4 noLKT6 noLKT7 noLKT8 noLKT9 noLKU10 noLKU3 noLKU4 noLKU5 noLKU6 noLKU6b noLKU7 noLKU8 noLKU9 noLKUNONE noLOCK12a noLOCK12b noLOCK13 noLOCK24 noLOCKRNG noLOCKCHGU noLOCKCHGD noOPCF1 noOPCF6 noOPDG2 noOPDG3 noOPDG6 noOPDG7 noOPEN15 noOPEN18 noOPEN2 noOPEN20 noOPEN22 noOPEN23 noOPEN24 noOPEN26 noOPEN27 noOPEN28 noOPEN3 noOPEN30 noOPEN4 noRENEW3 noRD1 noRD10 noRD2 noRD3 noRD5 noRD6 noRD7a noRD7b noRD7c noRD7d noRD7f noRD7s noRPLY1 noRPLY10 noRPLY12 noRPLY14 noRPLY2 noRPLY3 noRPLY5 noRPLY6 noRPLY7 noRPLY8 noRPLY9 noSATT3d noSATT4 noSATT6d noSATT6r noSATT18 noSEC7 noWRT1 noWRT11 noWRT13 noWRT14 noWRT15 noWRT18 noWRT19 noWRT1b noWRT2 noWRT3 noWRT6a noWRT6b noWRT6c noWRT6d noWRT6f noWRT6s noWRT8 noWRT9
```

However, I don't think it will be a big issue to adjust if suddenly `all` has more tests.
BTW, we have the containerized version as well, which is publicly available. I will write some docu...

podman run -ti --rm -v `pwd`/out:/out dcache/pynfs:0.1  /bin/bash -c "cd /pynfs/nfs4.0; python3 -u ./testserver.py --xml=/out/xunit-report-v40.xml --noinit nfs-lab:/data all; exit 0"

I am happy to share Dockerfile and make it more user friendly :)

Best regards,
   Tigran.



> 
>> > Also, we could add test categories specifically for particular server
>> > implementations, if that's interesting to folks.
>> 
>> I have already done so with a ganesha tag...
>> 
>> Literally all anyone has to do is start using a new tag so it's a trivial
>> thing for developers to add.
>> 
> 
> The problem is that you have to add the tag to hundreds of tests. It's
> scriptable, sure, but if you want to be at all selective, it's not
> trivial.
> 
> That said, I'm open to doing something different here. Maybe we could
> declare a new "everything" special case instead? It's confusing naming,
> but that would at least preserve the existing behavior for those CI
> systems.
> 
>> > 
>> > > --b.
>> > > 
>> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
>> > > > ---
>> > > > nfs4.1/testmod.py | 2 ++
>> > > > 1 file changed, 2 insertions(+)
>> > > > 
>> > > > If this is unacceptable, then an alternative could be to add a new
>> > > > (similarly special-cased) "everything" flag.
>> > > > 
>> > > > diff --git a/nfs4.1/testmod.py b/nfs4.1/testmod.py index
>> > > > 11e759d673fd..7b3bac543084 100644
>> > > > --- a/nfs4.1/testmod.py
>> > > > +++ b/nfs4.1/testmod.py
>> > > > @@ -386,6 +386,8 @@ def createtests(testdir):
>> > > >     for t in tests:
>> > > > ##         if not t.flags_list:
>> > > > ##             raise RuntimeError("%s has no flags" % t.fullname)
>> > > > +        if "all" not in t.flags_list:
>> > > > +            t.flags_list.append("all")
>> > > >         for f in t.flags_list:
>> > > >             if f not in flag_dict:
>> > > >                 flag_dict[f] = bit
>> > > > --
>> > > > 2.39.2
>> > 
>> > --
>> > Chuck Lever
>> > 
>> 
>> 
> 
> --
> Jeff Layton <jlayton@kernel.org>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2208 bytes --]

RE: [pynfs RFC PATCH] testserver.py: special-case the "all" flag

[Frank Filz]

> From: Jeff Layton [mailto:jlayton@kernel.org]
 
> On Thu, 2023-02-23 at 08:26 -0800, Frank Filz wrote:
> >
> > > -----Original Message-----
> > > From: Chuck Lever III [mailto:chuck.lever@oracle.com]
> > > Sent: Thursday, February 23, 2023 8:22 AM
> > > To: Bruce Fields <bfields@fieldses.org>
> > > Cc: Jeff Layton <jlayton@kernel.org>; Dai Ngo <dai.ngo@oracle.com>;
> > > Linux NFS Mailing List <linux-nfs@vger.kernel.org>; Calum Mackay
> > > <calum.mackay@oracle.com>; Frank Filz <ffilzlnx@mindspring.com>
> > > Subject: Re: [pynfs RFC PATCH] testserver.py: special-case the "all"
> > > flag
> > >
> > >
> > >
> > > > On Feb 23, 2023, at 10:19 AM, J. Bruce Fields
> > > > <bfields@fieldses.org>
> > wrote:
> > > >
> > > > On Wed, Feb 22, 2023 at 01:20:43PM -0500, Jeff Layton wrote:
> > > > > The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0,
> > > > > the
> > "all"
> > > > > flag is supposed to run all of the "standard" tests. For v4.1
"all"
> > > > > is documented to run all of the tests, but it actually doesn't
> > > > > since not every tests has "all" in its FLAGS: field.
> > > > >
> > > > > I move that we change this. If I say that I want to run "all",
> > > > > then I really do want to run _all_ of the tests. Ensure that
> > > > > every test has the "all" flag set.
> > > >
> > > > In some (all?) cases where the "all" flag was left off, it was
> > > > intentional.
> > > >
> > > > We try not to flag spec-compliant servers as failing, because
> > > > people are sometimes a little careless about "fixing" failures
> > > > that in their particular case really shouldn't be fixed.  But
> > > > sometimes it's still useful to have a test that goes somewhat beyond
the
> spec.
> > > >
> > > > There might be other ways to handle that kind of test, but it
> > > > would need some more thought.
> > >
> > > We could use a different name for "all" since it doesn't actually
> > > run
> > /all/ tests.
> > > Jeff suggested "standard", which seems sensible.
> >
> > The challenge with changing this is all the CI scripts and other
> > testing scripts out there that use all. If all suddenly changed,
> > server maintainers might get a deluge of bug reports for failing odd
> > test cases no one necessarily expected to work...
> >
> 
> Are those CI systems pulling down the upstream tree to run? How do they
> contend with new tests that might suddenly show up as part of "all"?

Honestly I'm not sure, the Ganesha CI may pull from a non-upstream repo. Or
it may be that anything that has been added to "all" happens to be OK. I
don't know...

> > > Also, we could add test categories specifically for particular
> > > server implementations, if that's interesting to folks.
> >
> > I have already done so with a ganesha tag...
> >
> > Literally all anyone has to do is start using a new tag so it's a
> > trivial thing for developers to add.
> >
> 
> The problem is that you have to add the tag to hundreds of tests. It's
scriptable,
> sure, but if you want to be at all selective, it's not trivial.
> 
> That said, I'm open to doing something different here. Maybe we could
declare
> a new "everything" special case instead? It's confusing naming, but that
would
> at least preserve the existing behavior for those CI systems.

That might be the best thing to do. That way existing scripts aren't broken,
yet someone who genuinely wants to try and run everything can.

The next problem will be how to exclude tests that have specific conditions
that can't be met, especially tests for a specific server implementation.
Specifying everything -ganesha won't work because that will cut out tests
that aren't Ganesha specific. But we could add a new ganeshaonly tag for
those test cases that is added in addition to the ganesha tag. Then
everthing -ganeshaonly excludes those while all ganesha still runs
everything Ganesha is expected to pass.

Frank

> > >
> > > > --b.
> > > >
> > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > > ---
> > > > > nfs4.1/testmod.py | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > If this is unacceptable, then an alternative could be to add a
> > > > > new (similarly special-cased) "everything" flag.
> > > > >
> > > > > diff --git a/nfs4.1/testmod.py b/nfs4.1/testmod.py index
> > > > > 11e759d673fd..7b3bac543084 100644
> > > > > --- a/nfs4.1/testmod.py
> > > > > +++ b/nfs4.1/testmod.py
> > > > > @@ -386,6 +386,8 @@ def createtests(testdir):
> > > > >     for t in tests:
> > > > > ##         if not t.flags_list:
> > > > > ##             raise RuntimeError("%s has no flags" % t.fullname)
> > > > > +        if "all" not in t.flags_list:
> > > > > +            t.flags_list.append("all")
> > > > >         for f in t.flags_list:
> > > > >             if f not in flag_dict:
> > > > >                 flag_dict[f] = bit
> > > > > --
> > > > > 2.39.2
> > >
> > > --
> > > Chuck Lever
> > >
> >
> >
> 
> --
> Jeff Layton <jlayton@kernel.org>