| Message ID | 20220506051017.GF1949718@dread.disaster.area (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] fstests: faster group file creation | expand |
On Fri, May 06, 2022 at 03:10:17PM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > 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@redhat.com> > --- Looks good to me. It's much faster than before, especially when rebuild for small changes. Reviewed-by: Zorro Lang <zlang@kernel.org> > 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 >
On Fri, May 06, 2022 at 03:13:11PM +0800, Zorro Lang wrote: > On Fri, May 06, 2022 at 03:10:17PM +1000, Dave Chinner wrote: > > From: Dave Chinner <dchinner@redhat.com> > > > > 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@redhat.com> > > --- > > Looks good to me. It's much faster than before, especially when rebuild for > small changes. Yup, my run scripts always run make before starting a run, so when I'm iterating single tests for failure analysis, this knocks 20-30s out of the cycle time. check starting up is now the thing that is really slow - it's still taking around 10s to get to the first test these days.... > Reviewed-by: Zorro Lang <zlang@kernel.org> Thanks! -Dave.
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