On Thu, Apr 12, 2018 at 05:38:38PM -0400, jeffm@xxxxxxxx wrote:
> From: Jeff Mahoney <jeffm@xxxxxxxx>
[Sorry for the late review..]
>
> Currently, we only create results files when a test has failed or was
> supposed to run but some dependency wasn't met causing it not to run.
> Short of saving the summary at the end of the run, there's no way to tell
> which tests passed or which tests weren't run due to being excluded.
>
> This patch moves successful test results to $seqres.out.good to annotate
> good results. It also adds tests excluded by group to the $tmp.xlist file
> and adds annotations for every test excluded. When a test is expunged
I can see that annotating tests excluded by group is helpful, but I
don't think we need $seqres.out.good, passed tests are already shown by
check, and $seqres.out.good is same as the golden image, it looks
redundant.
> during execution, an expunged message will be issued and a
> $seqres.expunged file will be created, both containing the reason for
> the test being expunged. Reasons can be "command line", "file $filename",
> or "group <group> [group...]".
>
> This makes the output more noisy in the expunged case and makes startup
> take slightly longer, but ends up with results that can be more easily
> parsed by automated tools.
>
> Signed-off-by: Jeff Mahoney <jeffm@xxxxxxxx>
> ---
> check | 57 +++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/check b/check
> index 546683c5..9a1f4e7e 100755
> --- a/check
> +++ b/check
> @@ -173,28 +173,18 @@ get_all_tests()
> done
> }
>
> -# takes the list of tests to run in $tmp.list, and removes the tests passed to
> -# the function from that list.
> +# takes the list of tests to run in $tmp.list and adds them to the excluded
> +# test list, annotated with the group that excluded each one
The comments don't seem quite right, the new behavior doesn't take tests
from $tmp.list, it just edits the $tmp.xlist file for later use.
> trim_test_list()
> {
> + group=$1
Please declare local variable as 'local' when possible.
> + shift
> test_list="$*"
>
> - rm -f $tmp.grep
> - numsed=0
> - for t in $test_list
> - do
> - if [ $numsed -gt 100 ]; then
> - grep -v -f $tmp.grep <$tmp.list >$tmp.tmp
> - mv $tmp.tmp $tmp.list
> - numsed=0
> - rm -f $tmp.grep
> - fi
> - echo "^$t\$" >>$tmp.grep
> - numsed=`expr $numsed + 1`
> + for test in $test_list; do
> + test=${test##tests/}
> + echo "$test group $group" >> $tmp.xlist
> done
> - grep -v -f $tmp.grep <$tmp.list >$tmp.tmp
> - mv $tmp.tmp $tmp.list
> - rm -f $tmp.grep
> }
>
>
> @@ -246,7 +236,7 @@ _prepare_test_list()
> exit 1
> fi
>
> - trim_test_list $list
> + trim_test_list $xgroup $list
> done
>
> # sort the list of tests into numeric order
> @@ -285,13 +275,15 @@ while [ $# -gt 0 ]; do
> for d in $SRC_GROUPS $FSTYP; do
> [ -f $SRC_DIR/$d/$xfile ] || continue
> for f in `sed "s/#.*$//" $SRC_DIR/$d/$xfile`; do
> - echo $d/$f >> $tmp.xlist
> + echo "$d/$f command line" >> $tmp.xlist
> done
> done
> ;;
> -E) xfile=$2; shift ;
> if [ -f $xfile ]; then
> - sed "s/#.*$//" "$xfile" >> $tmp.xlist
> + sed -e "s/#.*$//" \
> + -e "s;$; file $xfile;" "$xfile" \
> + >> $tmp.xlist
When using config section, e.g. ./check -s <sec_name>, and FSTYP is
different in new section, check will do _prepare_test_list again, so
tests selected by "-x <xgroup>" will be listed twice in $tmp.xlist file,
as a result, I see duplicated group names in the annotate message, e.g.
generic/458 [expunged] group metadata metadata
I think we should move above "-X/-E" handlings to _prepare_test_list,
only assign variable "xfile" here (use a new variable for -E option,
perhaps). So that we could simply remove $tmp.xlist before calling
_prepare_test_list again, and it could exclude all specified tests only
once. Maybe as patch 1/2 in a patchset.
> fi
> ;;
> -s) RUN_SECTION="$RUN_SECTION $2"; shift ;;
> @@ -491,11 +483,17 @@ _check_filesystems()
> _expunge_test()
> {
> local TEST_ID="$1"
> + local OUTPUT="$2"
OUTPUT is not used.
> if [ -s $tmp.xlist ]; then
> - if grep -q $TEST_ID $tmp.xlist; then
> - echo " [expunged]"
> + grep $TEST_ID $tmp.xlist > $tmp._expunge_test
> + if [ $? -eq 0 ]; then
> + sed -e "s;$TEST_ID ;;" $tmp._expunge_test | \
> + tr '\n' ' ' | sed -e 's; group;;g'
> + echo
> + rm -f $tmp._expunge_test
> return 1
> fi
> + rm -f $tmp._expunge_test
> fi
> return 0
> }
> @@ -670,8 +668,10 @@ for section in $HOST_OPTIONS_SECTIONS; do
> echo -n "$seqnum"
>
> if $showme; then
> - _expunge_test $seqnum
> + _expunge_test $seqnum > $tmp.xreason
> if [ $? -eq 1 ]; then
> + echo -n " [expunged] "
> + cat $tmp.xreason
> continue
> fi
> echo
> @@ -689,11 +689,15 @@ for section in $HOST_OPTIONS_SECTIONS; do
> else
> # really going to try and run this one
> #
> - rm -f $seqres.out.bad
> + rm -f $seqres.out.bad $seqres.out.good $seqres.expunged
> + rm -f $seqres.notrun
>
> # check if we really should run it
> - _expunge_test $seqnum
> + _expunge_test $seqnum > $tmp.xreason
> if [ $? -eq 1 ]; then
> + mv $tmp.xreason $seqres.expunged
> + echo -n " [expunged] "
> + cat $seqres.expunged
> continue
> fi
>
> @@ -704,7 +708,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
> else
> echo -n " " # prettier output with timestamps.
> fi
> - rm -f core $seqres.notrun
> + rm -f core
>
> start=`_wallclock`
> $timestamp && echo -n " ["`date "+%T"`"]"
> @@ -770,6 +774,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
> else
> echo "$seqnum `expr $stop - $start`" >>$tmp.time
> echo -n " `expr $stop - $start`s"
> + mv $tmp.out $seqres.out.good
As said above, I don't think it's that useful.
Thanks,
Eryu
> fi
> echo ""
> else
> --
> 2.15.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
