On Fri, May 06, 2022 at 03:10:17PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> We don't need to execute every test just to check it's groups are
> valid. Just grab all the groups with grep, pull out the unique ones,
> then check them.
>
> This also avoids the problem of editor swap files being present in
> the test directory and breaking the build because they are not
> executable.
>
> Building on a clean, already built tree so it only builds the
> group lists:
>
> $ time make
> ....
> Building udf
> [GROUP] /home/dave/src/xfstests-dev/tests/udf/group.list
> Building xfs
> [GROUP] /home/dave/src/xfstests-dev/tests/xfs/group.list
>
> real 0m36.917s
> user 0m15.032s
> sys 0m26.219s
> $
>
> Patched:
>
> $ time make
> ....
> Building udf
> [GROUP] /home/dave/src/xfstests-dev/tests/udf/group.list
> Building xfs
> [GROUP] /home/dave/src/xfstests-dev/tests/xfs/group.list
> groups "frobnozzle" not mentioned in documentation.
> gmake[3]: *** [../../include/buildgrouplist:8: group.list] Error 1
> gmake[2]: *** [../include/buildrules:31: xfs] Error 2
> gmake[1]: *** [include/buildrules:31: tests] Error 2
> make: *** [Makefile:51: default] Error 2
>
> real 0m1.751s
> user 0m0.863s
> sys 0m1.067s
>
> $
>
> Just a little bit faster, and as you can see that it still detects
> groups that are not documented. There was also an open .001.swp file
> in the XFS directory and that doesn't throw a failure anymore,
> either.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
Looks good to me. It's much faster than before, especially when rebuild for
small changes.
Reviewed-by: Zorro Lang <zlang@xxxxxxxxxx>
> V2: don't optimise away the per-test group listing in the output
> file that check relies on for '-g <group>' matching.
>
> common/preamble | 28 -----------------------
> tools/mkgroupfile | 66 ++++++++++++++++++++++++++++++++++++++-----------------
> 2 files changed, 46 insertions(+), 48 deletions(-)
>
> diff --git a/common/preamble b/common/preamble
> index 64d79385..f4a405ac 100644
> --- a/common/preamble
> +++ b/common/preamble
> @@ -23,26 +23,6 @@ _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.
> @@ -59,14 +39,6 @@ _begin_fstest()
>
> seq=`basename $0`
>
> - # 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
> -
> seqres=$RESULT_DIR/$seq
> echo "QA output created by $seq"
>
> diff --git a/tools/mkgroupfile b/tools/mkgroupfile
> index 3844e57d..24435898 100755
> --- a/tools/mkgroupfile
> +++ b/tools/mkgroupfile
> @@ -11,40 +11,64 @@ fi
>
> test_dir="$PWD"
> groupfile="$1"
> +new_groups="/tmp/groups.$$"
> GROUPNAME_DOC_FILE="$(readlink -m ../../doc/group-names.txt)"
> -export GROUPNAME_DOC_FILE
>
> if [ ! -x ../../check ]; then
> echo "$0: Run this from tests/XXX/."
> exit 1
> fi
>
> -cleanup() {
> - test -z "$groupfile" && return
> - test -z "$ngroupfile" && return
> -
> - if [ $ret -eq 0 ]; then
> - mv -f "$ngroupfile" "$groupfile"
> - else
> - rm -f "$ngroupfile"
> - fi
> +cleanup()
> +{
> + rm -f $new_groups.check
> + rm -f $new_groups
> }
>
> ret=1 # trigger cleanup of temporary files unless we succeed
> trap 'cleanup; exit $ret' EXIT INT TERM QUIT
>
> +# Make sure each group is in the documentation file.
> +_check_groups() {
> + test -n "$GROUPNAME_DOC_FILE" || return 0
> +
> + local groups="$1"
> + declare -a missing=()
> +
> + for group in `grep -v '#' $groups`; 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 "group$suffix ${missing[@]} not mentioned in documentation." 1>&2
> + ret=1
> + exit 1
> +}
> +
> generate_groupfile() {
> - cat << ENDL
> + cat << ENDL > $new_groups
> # QA groups control file, automatically generated.
> # See _begin_fstest in each test for details.
>
> ENDL
> +
> cd ../../
> - export GENERATE_GROUPS=yes
> - grep -R -l "^_begin_fstest" "$test_dir/" 2>/dev/null | while read testfile; do
> - test -x "$testfile" && "$testfile" || return 1
> - done | sort -g
> - ret="${PIPESTATUS[1]}"
> +
> + # Aggregate the groups each test belongs to for the group file
> + grep -I -R "^_begin_fstest" $test_dir/ | \
> + sed -e 's/^.*\/\([0-9]*\):_begin_fstest/\1/' >> $new_groups
> +
> + # Create the list of unique groups for existence checking
> + grep -I -R "^_begin_fstest" $test_dir/ | \
> + sed -e 's/^.*_begin_fstest //' -e 's/ /\n/g' | \
> + sort -u > $new_groups.check
> +
> + _check_groups $new_groups.check
> +
> cd "$test_dir"
> }
>
> @@ -52,10 +76,12 @@ if [ -z "$groupfile" ] || [ "$groupfile" = "-" ]; then
> # Dump the group file to stdout and exit
> unset groupfile
> generate_groupfile
> + cat $new_groups
> else
> # Otherwise, write the group file to disk somewhere.
> - ngroupfile="${groupfile}.new"
> - rm -f "$ngroupfile"
> - generate_groupfile >> "$ngroupfile"
> - # let cleanup rename or delete ngroupfile
> + generate_groupfile
> + mv -f "$new_groups" "$groupfile"
> fi
> +
> +# Success!
> +ret=0
>
