From: Jeff Layton [mailto:jlayton@kernel.org]

On Thu, 2023-02-23 at 08:26 -0800, Frank Filz wrote:

quoted

quoted

-----Original Message-----
From: Chuck Lever III [mailto:chuck.lever@oracle.com]
Sent: Thursday, February 23, 2023 8:22 AM
To: Bruce Fields <redacted>
Cc: Jeff Layton <jlayton@kernel.org>; Dai Ngo <dai.ngo@oracle.com>;
Linux NFS Mailing List [off-list ref]; Calum Mackay
[off-list ref]; Frank Filz [off-list ref]
Subject: Re: [pynfs RFC PATCH] testserver.py: special-case the "all"
flag

quoted

On Feb 23, 2023, at 10:19 AM, J. Bruce Fields
[off-list ref]

wrote:

quoted

quoted

On Wed, Feb 22, 2023 at 01:20:43PM -0500, Jeff Layton wrote:

quoted

The READMEs for v4.0 and v4.1 are inconsistent here. For v4.0,
the

"all"

quoted

quoted

quoted

flag is supposed to run all of the "standard" tests. For v4.1

"all"

quoted

quoted

quoted

quoted

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.

quoted

quoted

quoted

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.

quoted

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...

quoted

quoted

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

quoted

quoted

quoted

--b.

quoted

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 [off-list ref]