Re: [PATCH v2.1 6/8] tools: make sure that test groups are described in the documentation

Darrick J. Wong
2021-09-13T19:03:54Z

On Sat, Sep 04, 2021 at 11:52:32AM +0300, Amir Goldstein wrote:
> On Sat, Sep 4, 2021 at 6:06 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 change mkgroupfile to check that every
> > group mentioned in the tests is also mentioned in the documentation.
> >
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > common/preamble | 21 ++++++++
> > doc/group-names.txt | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > tools/mkgroupfile | 15 +++++-
> > 3 files changed, 168 insertions(+), 3 deletions(-)
> > create mode 100644 doc/group-names.txt
> >
> > diff --git a/common/preamble b/common/preamble
> > index 66b0ed05..64d79385 100644
> > --- a/common/preamble
> > +++ b/common/preamble
> > @@ -23,6 +23,26 @@ _register_cleanup()
> > trap "${cleanup}exit \$status" EXIT HUP INT QUIT TERM $*
> > }
> >
> > +# Make sure each group is in the documentation file.
> > +_check_groups() {
> > + test -n "$GROUPNAME_DOC_FILE" || return 0
> > +
> > + local testname="$(echo "$0" | sed -e 's/^.*tests\///g')"
> > + declare -a missing=()
> > +
> > + for group in "$@"; do
> > + if ! grep -q "^${group}[[:space:]]" "$GROUPNAME_DOC_FILE"; then
> > + missing+=("\"${group}\"")
> > + fi
> > + done
> > + test "${#missing}" -eq 0 && return 0
> > +
> > + local suffix=
> > + test "${#missing}" -gt 1 && suffix="s"
> > + echo "$testname: group$suffix ${missing[@]} not mentioned in documentation." 1>&2
> > + return 1
> > +}
> > +
> > # Prepare to run a fstest by initializing the required global variables to
> > # their defaults, sourcing common functions, registering a cleanup function,
> > # and removing the $seqres.full file.
> > @@ -42,6 +62,7 @@ _begin_fstest()
> > # If we're only running the test to generate a group.list file,
> > # spit out the group data and exit.
> > if [ -n "$GENERATE_GROUPS" ]; then
> > + _check_groups "$@" || exit 1
> > echo "$seq $@"
> > exit 0
> > fi
> > diff --git a/doc/group-names.txt b/doc/group-names.txt
> > new file mode 100644
> > index 00000000..e8e3477e
> > --- /dev/null
> > +++ b/doc/group-names.txt
> > @@ -0,0 +1,135 @@
> > +======================= =======================================================
> > +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 FALLOC_FL_COLLAPSE_RANGE
> > +compress file compression
> > +convert btrfs ext[34] conversion tool
> > +copy xfs_copy functionality
> > +copy_range copy_file_range syscall
> > +copyup overlayfs copyup
> > +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 FALLOC_FL_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
> > +pattern specific IO pattern tests
> > +perms access control and permission checking
> > +pipe pipe functionality
> > +pnfs PNFS
> > +posix POSIX behavior conformance
> > +prealloc fallocate for preallocating unwritten space
> > +preallocrw fallocate, then read and write
> > +punch fallocate FALLOC_FL_PUNCH_HOLE
> > +qgroup btrfs qgroup feature
> > +quota filesystem usage quotas
> > +raid btrfs RAID
> > +realtime XFS realtime volumes
> > +recoveryloop crash recovery loops
> > +redirect overlayfs redirect_dir feature
> > +remote dump and restore with a remote tape
> > +remount remounting filesystems
> > +rename rename system call
> > +repair xfs_repair functional tests
> > +replace btrfs device replace
> > +replay dm-logwrites replays
> > +resize resize2fs functionality tests
> > +richacl rich ACL feature
> > +rmap XFS reverse mapping exercisers
> > +rotate overlayfs upper layer rotate tests from the unionmount
> > + test suite
> > +rw read/write IO tests
> > +samefs overlayfs when all layers are on the same fs
> > +scrub filesystem metadata scrubbers
> > +seed btrfs seeded filesystems
> > +seek llseek functionality
> > +send btrfs send/receive
> > +shrinkfs decreasing the size of a filesystem
> > +shutdown FS_IOC_SHUTDOWN ioctl
> > +snapshot btrfs snapshots
> > +soak long running soak tests of any kind
> > +spaceman xfs_spaceman functional tests
> > +splice splice system call
> > +stress fsstress filesystem exerciser
> > +subvol btrfs subvolumes
> > +swap swap files
> > +symlink symbolic links
> > +tape dump and restore with a tape
> > +thin thin provisioning
> > +trim FITRIM ioctl
> > +udf UDF functionality tests
> > +union tests from the unionmount test suite
> > +unlink O_TMPFILE unlinked files
> > +unshare fallocate FALLOC_FL_UNSHARE_RANGE
> > +v2log XFS v2 log format tests
> > +verity fsverity
> > +volume btrfs volume management
> > +whiteout overlayfs whiteout functionality
> > +xino overlayfs xino feature
> > +zero fallocate FALLOC_FL_ZERO_RANGE
> > +zone zoned (SMR) device support
> > +======================= =======================================================
> > diff --git a/tools/mkgroupfile b/tools/mkgroupfile
> > index 0681e5d2..767bac90 100755
> > --- a/tools/mkgroupfile
> > +++ b/tools/mkgroupfile
> > @@ -9,6 +9,8 @@ fi
> >
> > test_dir="$PWD"
> > groupfile="$1"
> > +GROUPNAME_DOC_FILE="$(readlink -m ../../doc/group-names.txt)"
> > +export GROUPNAME_DOC_FILE
> >
> > if [ ! -x ../../check ]; then
> > echo "$0: Run this from tests/XXX/."
> > @@ -24,19 +26,26 @@ ENDL
> > cd ../../
> > export GENERATE_GROUPS=yes
> > grep -R -l "^_begin_fstest" "$test_dir/" 2>/dev/null | while read testfile; do
> > - test -x "$testfile" && "$testfile"
> > + test -x "$testfile" && "$testfile" || return 1
> > done | sort -g
> > + local ret="${PIPESTATUS[1]}"
> > cd "$test_dir"
> > + return $ret
> > }
> >
> > if [ -z "$groupfile" ] || [ "$groupfile" = "-" ]; then
> > # Dump the group file to stdout and exit
> > generate_groupfile
> > - exit 0
>
> Now the comment above is incorrect, but I think you
> did want to exit $ret?

"exit" by itself will cause the script to exit with the last accumulated
return value (which is the 'return $ret' in the last line of
generat_groupfile), but yes, thanks for pointing out that I'd deleted
the whole line by mistake. Fixed.

>
> > fi
> >
> > # Otherwise, write the group file to disk somewhere.
> > ngroupfile="${groupfile}.new"
> > rm -f "$ngroupfile"
> > generate_groupfile >> "$ngroupfile"
> > -mv "$ngroupfile" "$groupfile"
> > +ret=$?
> > +if [ $ret -eq 0 ]; then
> > + mv "$ngroupfile" "$groupfile"
> > +else
> > + rm -f "$ngroupfile"
> > +fi
> > +exit $ret
>
> I think that a cleanup() trap would have made this script a lot cleaner,
> not having to deal with ${PIPESTATUS[1]} and whatnot, but if there is
> a reason not to use cleanup() trap, I'm fine with this as well.

No real reason. I'll reimplement the prologue as a cleanup trap so that
we don't leave junk everywhere if someone ^Cs during build.

--D

>
> Thanks,
> Amir.