Linux NFS development
 help / color / mirror / Atom feed
From: "Frank Filz" <ffilzlnx@mindspring.com>
To: "'Chuck Lever III'" <chuck.lever@oracle.com>,
	"'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>
Subject: RE: [pynfs RFC PATCH] testserver.py: special-case the "all" flag
Date: Thu, 23 Feb 2023 08:26:55 -0800	[thread overview]
Message-ID: <029a01d947a3$a51b4750$ef51d5f0$@mindspring.com> (raw)
In-Reply-To: <3B034712-F376-4D71-8A72-703B030140F9@oracle.com>



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



  reply	other threads:[~2023-02-23 16:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-22 18:20 [pynfs RFC PATCH] testserver.py: special-case the "all" flag Jeff Layton
2023-02-23 15:19 ` J. Bruce Fields
2023-02-23 16:20   ` Frank Filz
2023-02-23 16:21   ` Chuck Lever III
2023-02-23 16:26     ` Frank Filz [this message]
2023-02-23 17:10       ` Jeff Layton
2023-02-23 17:59         ` Mkrtchyan, Tigran
2023-02-23 22:41         ` Frank Filz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='029a01d947a3$a51b4750$ef51d5f0$@mindspring.com' \
    --to=ffilzlnx@mindspring.com \
    --cc=bfields@fieldses.org \
    --cc=calum.mackay@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=dai.ngo@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox