linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Eryu Guan <guaneryu@gmail.com>,
	linux-xfs <linux-xfs@vger.kernel.org>,
	fstests <fstests@vger.kernel.org>, Eryu Guan <guan@eryu.me>
Subject: Re: [PATCH 4/5] tools: make sure that test groups are described in the documentation
Date: Thu, 2 Sep 2021 07:49:51 +0300	[thread overview]
Message-ID: <CAOQ4uxgJz6OBmV=SD1fp9tkCAfiAhxjdCr+fxGd4ko4Y6NUscA@mail.gmail.com> (raw)
In-Reply-To: <20210901164311.GB9911@magnolia>

On Wed, Sep 1, 2021 at 7:43 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Wed, Sep 01, 2021 at 07:46:01AM +0300, Amir Goldstein wrote:
> > On Wed, Sep 1, 2021 at 3:37 AM Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > From: Darrick J. Wong <djwong@kernel.org>
> > >
> > > Create a file to document the purpose of each test group that is
> > > currently defined in fstests, and add a build script to check that every
> > > group mentioned in the tests is also mentioned in the documentation.
> > >
> >
> > This is awesome and long due.
> > Thanks for doing that!
> >
> > Minor nits about overlayfs groups below...
>
> Heh, yeah, thanks for making corrections. :)
>
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > >  doc/group-names.txt    |  136 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  include/buildgrouplist |    1
> > >  tools/check-groups     |   33 ++++++++++++
> > >  3 files changed, 170 insertions(+)
> > >  create mode 100644 doc/group-names.txt
> > >  create mode 100755 tools/check-groups
> > >
> > >
> > > diff --git a/doc/group-names.txt b/doc/group-names.txt
> > > new file mode 100644
> > > index 00000000..ae517328
> > > --- /dev/null
> > > +++ b/doc/group-names.txt
> > > @@ -0,0 +1,136 @@
> > > +======================= =======================================================
> > > +Group Name:            Description:
> > > +======================= =======================================================
> > > +all                    All known tests, automatically generated by ./check at
> > > +                       runtime
> > > +auto                   Tests that should be run automatically.  These should
> > > +                       not require more than ~5 minutes to run.
> > > +quick                  Tests that should run in under 30 seconds.
> > > +deprecated             Old tests that should not be run.
> > > +
> > > +acl                    Access Control Lists
> > > +admin                  xfs_admin functionality
> > > +aio                    general libaio async io tests
> > > +atime                  file access time
> > > +attr                   extended attributes
> > > +attr2                  xfs v2 extended aributes
> > > +balance                        btrfs tree rebalance
> > > +bigtime                        timestamps beyond the year 2038
> > > +blockdev               block device functionality
> > > +broken                 broken tests
> > > +cap                    Linux capabilities
> > > +casefold               directory name casefolding
> > > +ci                     ASCII case-insensitive directory name lookups
> > > +clone                  FICLONE/FICLONERANGE ioctls
> > > +clone_stress           stress testing FICLONE/FICLONERANGE
> > > +collapse               fallocate collapse_range
> > > +compress               file compression
> > > +convert                        btrfs ext[34] conversion tool
> > > +copy                   xfs_copy functionality
> > > +copy_range             copy_file_range syscall
> > > +copyup                 overlayfs copyup support
> >
> > The tests in this group exercise copy up.
> > There is no such thing as overlayfs without "copyup support",
> > so guess my point is - remove the word "support"
>
> OK.
>
> > > +dangerous              dangerous test that can crash the system
> > > +dangerous_bothrepair   fuzzers to evaluate xfs_scrub + xfs_repair repair
> > > +dangerous_fuzzers      fuzzers that can crash your computer
> > > +dangerous_norepair     fuzzers to evaluate kernel metadata verifiers
> > > +dangerous_online_repair        fuzzers to evaluate xfs_scrub online repair
> > > +dangerous_repair       fuzzers to evaluate xfs_repair offline repair
> > > +dangerous_scrub                fuzzers to evaluate xfs_scrub checking
> > > +data                   data loss checkers
> > > +dax                    direct access mode for persistent memory files
> > > +db                     xfs_db functional tests
> > > +dedupe                 FIEDEDUPERANGE ioctl
> > > +defrag                 filesystem defragmenters
> > > +dir                    directory test functions
> > > +dump                   dump and restore utilities
> > > +eio                    IO error reporting
> > > +encrypt                        encrypted file contents
> > > +enospc                 ENOSPC error reporting
> > > +exportfs               file handles
> > > +filestreams            XFS filestreams allocator
> > > +freeze                 filesystem freeze tests
> > > +fsck                   general fsck tests
> > > +fsmap                  FS_IOC_GETFSMAP ioctl
> > > +fsr                    XFS free space reorganizer
> > > +fuzzers                        filesystem fuzz tests
> > > +growfs                 increasing the size of a filesystem
> > > +hardlink               hardlinks
> > > +health                 XFS health reporting
> > > +idmapped               idmapped mount functionality
> > > +inobtcount             XFS inode btree count tests
> > > +insert                 fallocate insert_range
> > > +ioctl                  general ioctl tests
> > > +io_uring               general io_uring async io tests
> > > +label                  filesystem labelling
> > > +limit                  resource limits
> > > +locks                  file locking
> > > +log                    metadata logging
> > > +logprint               xfs_logprint functional tests
> > > +long_rw                        long-soak read write IO path exercisers
> > > +metacopy               overlayfs metadata-only copy-up
> > > +metadata               filesystem metadata update exercisers
> > > +metadump               xfs_metadump/xfs_mdrestore functionality
> > > +mkfs                   filesystem formatting tools
> > > +mount                  mount option and functionality checks
> > > +nested                 nested overlayfs instances
> > > +nfs4_acl               NFSv4 access control lists
> > > +nonsamefs              overlayfs layers on different filesystems
> > > +online_repair          online repair functionality tests
> > > +other                  dumping ground, do not add more tests to this group
> > > +overlay                        using overlayfs on top of FSTYP
> >
> > This description is a bit confusing, because the recommended
> > way to run overlayfs tests as described in README.overlay is
> > to set FSTYP=xfs and run ./check -overlay
> >
> > I'm struggling for a better description but perhaps:
> > "using overlayfs regardless of ./check -overlay flag"?
>
> Hmm.  Since I'm the author of the only test that uses this tag, I guess
> I'm the authority (ha!) on what the name actually means.
>
> That test (generic/631) is a regression test for a XFS whiteout handling
> bug that can only be reproduced by layering overlayfs atop xfs.
> Overlayfs is incidental to reproducing the XFS bug, but AFAIK overlayfs
> is the only in-kernel user of whiteout, which is why it's critical here.
>
> It's not right to make it "_supported_fs overlay" because we're not
> testing overlayfs functionality; we're merely using it as a stick to
> poke another filesystem.

Yes. I know.
Note that while this is the only case of _require_extra_fs overaly
there is another case of _require_extra_fs ext2 (xfs/049)

>
> How about: "regression tests that require the use of overlayfs in a
> targetted configuration" ?
>

TBH, I do not think it is wise to tag the test by the test method
rather than the tested functionality.

What is more likely?
that a tester wants to run all tests that use overlay over FSTYP?
Or that a tester wants to run all tests to verify whiteout related
behavior after changing whiteout related code?

I suggest that you re-tag this test as 'whiteout', which is documented
already.

If you want to be more specific, you can create a group
rename_whiteout, because RENAME_WHITEOUT is the vfs
interface that this test is actually exercising.

Thanks,
Amir.

  reply	other threads:[~2021-09-02  4:50 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01  0:12 [PATCHSET 0/5] fstests: document all test groups Darrick J. Wong
2021-09-01  0:12 ` [PATCH 1/5] ceph: re-tag copy_file_range as being in the copy_range group Darrick J. Wong
2021-09-01  8:44   ` Christoph Hellwig
2021-09-01  0:12 ` [PATCH 2/5] xfs: move reflink tests into the clone group Darrick J. Wong
2021-09-01  8:46   ` Christoph Hellwig
2021-09-01  0:12 ` [PATCH 3/5] xfs: fix incorrect fuzz test group name Darrick J. Wong
2021-09-01  8:48   ` Christoph Hellwig
2021-09-01  0:12 ` [PATCH 4/5] tools: make sure that test groups are described in the documentation Darrick J. Wong
2021-09-01  4:46   ` Amir Goldstein
2021-09-01 16:43     ` Darrick J. Wong
2021-09-02  4:49       ` Amir Goldstein [this message]
2021-09-02 15:03         ` Darrick J. Wong
2021-09-01  8:50   ` Christoph Hellwig
2021-09-01  0:12 ` [PATCH 5/5] new: only allow documented test group names Darrick J. Wong
2021-09-01  8:53   ` Christoph Hellwig
2021-09-01 21:29   ` Dave Chinner

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='CAOQ4uxgJz6OBmV=SD1fp9tkCAfiAhxjdCr+fxGd4ko4Y6NUscA@mail.gmail.com' \
    --to=amir73il@gmail.com \
    --cc=djwong@kernel.org \
    --cc=fstests@vger.kernel.org \
    --cc=guan@eryu.me \
    --cc=guaneryu@gmail.com \
    --cc=linux-xfs@vger.kernel.org \
    --subject='Re: [PATCH 4/5] tools: make sure that test groups are described in the documentation' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).