From: Eryu Guan <eguan@redhat.com>
To: Dmitry Monakhov <dmongw@gmail.com>
Cc: fstests@vger.kernel.org, Dmitry Monakhov <dmonakhov@openvz.org>
Subject: Re: [PATCH 1/2] check: prepare test report generator infrastructure
Date: Thu, 16 Feb 2017 17:18:48 +0800 [thread overview]
Message-ID: <20170216091847.GW24562@eguan.usersys.redhat.com> (raw)
In-Reply-To: <1485884601-21651-2-git-send-email-dmonakhov@openvz.org>
On Tue, Jan 31, 2017 at 09:43:20PM +0400, Dmitry Monakhov wrote:
> Save testcase data which later may be used by report generators
> - Save failure reason to $err_msg variable
> - Save number of notrun tests to $n_notrun counter, similar to $n_try,$n_bad
Sorry for the late review.
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> check | 32 ++++++++++++++++++++++----------
> common/btrfs | 12 ++++++------
> common/rc | 20 +++++++++++++-------
> common/xfs | 33 +++++++++++++++++----------------
> 4 files changed, 58 insertions(+), 39 deletions(-)
>
> diff --git a/check b/check
> index 5a93c94..edc0899 100755
> --- a/check
> +++ b/check
> @@ -28,6 +28,7 @@ try=""
> n_bad=0
> sum_bad=0
> bad=""
> +n_notrun=0
> notrun=""
> interrupt=true
> diff="diff -u"
> @@ -37,6 +38,7 @@ randomize=false
> export here=`pwd`
> xfile=""
> brief_test_summary=false
> +err_msg=""
>
> DUMP_OUTPUT=false
>
> @@ -368,6 +370,7 @@ _wipe_counters()
> {
> n_try="0"
> n_bad="0"
> + n_notrun="0"
> unset try notrun bad
> }
>
> @@ -596,6 +599,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
> for seq in $list
> do
> err=false
> + err_msg=""
> if [ ! -f $seq ]; then
> # Try to get full name in case the user supplied only seq id
> # and the test has a name. A bit of hassle to find really
> @@ -629,14 +633,17 @@ for section in $HOST_OPTIONS_SECTIONS; do
>
> echo -n "$seqnum"
>
> - if $showme; then
> - echo
> - continue
> - fi
> + if $showme; then
> + echo
> + start=0
> + stop=0
> + n_notrun=`expr $n_notrun + 1`
> + continue
> + fi
>
> - if [ ! -f $seq ]; then
> - echo " - no such test?"
> - else
> + if [ ! -f $seq ]; then
> + echo " - no such test?"
> + else
> # really going to try and run this one
> #
> rm -f $seqres.out.bad
> @@ -684,7 +691,8 @@ for section in $HOST_OPTIONS_SECTIONS; do
>
> if [ -f core ]
> then
> - echo -n " [dumped core]"
> + err_msg="[dumped core]"
> + echo -n " $err_msg"
> mv core $RESULT_BASE/$seqnum.core
> err=true
> fi
> @@ -695,15 +703,18 @@ for section in $HOST_OPTIONS_SECTIONS; do
> $timestamp && echo " [not run]" && echo -n " $seqnum -- "
> cat $seqres.notrun
> notrun="$notrun $seqnum"
> + n_notrun=`expr $n_notrun + 1`
> else
> if [ $sts -ne 0 ]
> then
> - echo -n " [failed, exit status $sts]"
> + err_msg="[failed, exit status $sts]"
> + echo -n " $err_msg"
> err=true
> fi
> if [ ! -f $seq.out ]
> then
> - echo " - no qualified output"
> + err_msg="no qualified output"
> + echo "- $err_msg"
> err=true
> else
>
> @@ -733,6 +744,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
> " to see the entire diff)"
> fi; } | \
> sed -e 's/^\(.\)/ \1/'
> + err_msg="output mismatch (see $diff $seq.out $seqres.out.bad)"
> err=true
> fi
> fi
> diff --git a/common/btrfs b/common/btrfs
> index 96c3635..64c40bd 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -105,24 +105,24 @@ _check_btrfs_filesystem()
> if [ -f ${RESULT_DIR}/require_scratch.require_qgroup_report ]; then
> $BTRFS_UTIL_PROG check $device --qgroup-report > $tmp.qgroup_report 2>&1
> if grep -qE "Counts for qgroup.*are different" $tmp.qgroup_report ; then
> - echo "_check_btrfs_filesystem: filesystem on $device has wrong qgroup numbers (see $seqres.full)"
> - echo "_check_btrfs_filesystem: filesystem on $device has wrong qgroup numbers" \
> - >> $seqres.full
> + msg="_check_btrfs_filesystem: filesystem on $device has wrong qgroup numbers"
> + echo "$msg" >> $seqres.full
> echo "*** qgroup_report.$FSTYP output ***" >>$seqres.full
> cat $tmp.qgroup_report >>$seqres.full
> echo "*** qgroup_report.$FSTYP output ***" >>$seqres.full
> + err_msg="$msg (see $seqres.full)"
This only assigns err_msg but doesn't echo it to stdout like what the
original code does. The same issue applies to other fsck error report
path.
And I noticed that similar pattern repeats time and time again, maybe
they can be abstracted into a helper?
Thanks,
Eryu
> fi
> rm -f $tmp.qgroup_report
> fi
>
> $BTRFS_UTIL_PROG check $device >$tmp.fsck 2>&1
> if [ $? -ne 0 ]; then
> - echo "_check_btrfs_filesystem: filesystem on $device is inconsistent (see $seqres.full)"
> -
> - echo "_check_btrfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
> + msg="_check_btrfs_filesystem: filesystem on $device is inconsistent"
> + echo "$msg" >>$seqres.full
> echo "*** fsck.$FSTYP output ***" >>$seqres.full
> cat $tmp.fsck >>$seqres.full
> echo "*** end fsck.$FSTYP output" >>$seqres.full
> + err_msg="$msg (see $seqres.full)"
>
> ok=0
> fi
> diff --git a/common/rc b/common/rc
> index 7e2eaee..f97b4d2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1017,7 +1017,10 @@ _repair_scratch_fs()
> _scratch_xfs_repair "$@" 2>&1
> res=$?
> fi
> - test $res -ne 0 && >&2 echo "xfs_repair failed, err=$res"
> + if [ test $res -ne 0]; then
> + err_msg="xfs_repair failed, err=$res"
> + >&2 echo "$err_msg"
> + fi
> return $res
> ;;
> *)
> @@ -1029,7 +1032,8 @@ _repair_scratch_fs()
> res=0
> ;;
> *)
> - >&2 echo "fsck.$FSTYP failed, err=$res"
> + err_msg="fsck.$FSTYP failed, err=$res"
> + >&2 echo "$err_msg"
> ;;
> esac
> return $res
> @@ -2130,7 +2134,8 @@ _mount_or_remount_rw()
> _overlay_mount $device $mountpoint
> fi
> if [ $? -ne 0 ]; then
> - echo "!!! failed to remount $device on $mountpoint"
> + err_msg="!!! failed to remount $device on $mountpoint"
> + echo "$err_msg"
> return 0 # ok=0
> fi
> else
> @@ -2164,12 +2169,12 @@ _check_generic_filesystem()
> fsck -t $FSTYP $FSCK_OPTIONS $device >$tmp.fsck 2>&1
> if [ $? -ne 0 ]
> then
> - echo "_check_generic_filesystem: filesystem on $device is inconsistent (see $seqres.full)"
> -
> - echo "_check_generic filesystem: filesystem on $device is inconsistent" >>$seqres.full
> + msg="_check_generic_filesystem: filesystem on $device is inconsistent"
> + echo "$msg" >>$seqres.full
> echo "*** fsck.$FSTYP output ***" >>$seqres.full
> cat $tmp.fsck >>$seqres.full
> echo "*** end fsck.$FSTYP output" >>$seqres.full
> + err_msg="$msg (see $seqres.full)"
>
> ok=0
> fi
> @@ -3023,7 +3028,8 @@ _check_dmesg()
> -e "general protection fault:" \
> $seqres.dmesg
> if [ $? -eq 0 ]; then
> - echo "_check_dmesg: something found in dmesg (see $seqres.dmesg)"
> + err_msg="_check_dmesg: something found in dmesg (see $seqres.dmesg)"
> + echo "$err_msg"
> return 1
> else
> rm -f $seqres.dmesg
> diff --git a/common/xfs b/common/xfs
> index 767a481..ad4c505 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -333,7 +333,8 @@ _check_xfs_filesystem()
> if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
> "$XFS_SCRUB_PROG" $scrubflag -vd $device >>$seqres.full
> if [ $? -ne 0 ]; then
> - echo "filesystem on $device failed scrub (see $seqres.full)"
> + err_msg="filesystem on $device failed scrub (see $seqres.full)"
> + echo "$err_msg"
> ok=0
> fi
> fi
> @@ -344,12 +345,12 @@ _check_xfs_filesystem()
> $XFS_LOGPRINT_PROG -t $extra_log_options $device 2>&1 \
> | tee $tmp.logprint | grep -q "<CLEAN>"
> if [ $? -ne 0 -a "$HOSTOS" = "Linux" ]; then
> - echo "_check_xfs_filesystem: filesystem on $device has dirty log (see $seqres.full)"
> -
> - echo "_check_xfs_filesystem: filesystem on $device has dirty log" >>$seqres.full
> + msg="_check_xfs_filesystem: filesystem on $device has dirty log"
> + echo "$msg" >>$seqres.full
> echo "*** xfs_logprint -t output ***" >>$seqres.full
> cat $tmp.logprint >>$seqres.full
> echo "*** end xfs_logprint output" >>$seqres.full
> + err_msg="$msg (see $seqres.full)"
>
> ok=0
> fi
> @@ -362,24 +363,24 @@ _check_xfs_filesystem()
> _fix_malloc >$tmp.fs_check
> fi
> if [ -s $tmp.fs_check ]; then
> - echo "_check_xfs_filesystem: filesystem on $device is inconsistent (c) (see $seqres.full)"
> -
> - echo "_check_xfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
> + msg="_check_xfs_filesystem: filesystem on $device is inconsistent (c)"
> + echo "$msg" >>$seqres.full
> echo "*** xfs_check output ***" >>$seqres.full
> cat $tmp.fs_check >>$seqres.full
> echo "*** end xfs_check output" >>$seqres.full
> + err_msg="$msg (see $seqres.full)"
>
> ok=0
> fi
>
> $XFS_REPAIR_PROG -n $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
> if [ $? -ne 0 ]; then
> - echo "_check_xfs_filesystem: filesystem on $device is inconsistent (r) (see $seqres.full)"
> -
> - echo "_check_xfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
> + msg="_check_xfs_filesystem: filesystem on $device is inconsistent (r)"
> + echo "$msg" >>$seqres.full
> echo "*** xfs_repair -n output ***" >>$seqres.full
> cat $tmp.repair | _fix_malloc >>$seqres.full
> echo "*** end xfs_repair output" >>$seqres.full
> + err_msg="$msg (see $seqres.full)"
>
> ok=0
> fi
> @@ -389,12 +390,12 @@ _check_xfs_filesystem()
> if [ -n "$TEST_XFS_REPAIR_REBUILD" ]; then
> $XFS_REPAIR_PROG $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
> if [ $? -ne 0 ]; then
> - echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild) (see $seqres.full)"
> -
> - echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild)" >>$seqres.full
> + msg="_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild)"
> + echo "$msg" >>$seqres.full
> echo "*** xfs_repair output ***" >>$seqres.full
> cat $tmp.repair | _fix_malloc >>$seqres.full
> echo "*** end xfs_repair output" >>$seqres.full
> + err_msg="$msg (see $seqres.full)"
>
> ok=0
> fi
> @@ -402,12 +403,12 @@ _check_xfs_filesystem()
>
> $XFS_REPAIR_PROG -n $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
> if [ $? -ne 0 ]; then
> - echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild-reverify) (see $seqres.full)"
> -
> - echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild-reverify)" >>$seqres.full
> + err_msg="_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild-reverify)"
> + echo "$msg" >>$seqres.full
> echo "*** xfs_repair -n output ***" >>$seqres.full
> cat $tmp.repair | _fix_malloc >>$seqres.full
> echo "*** end xfs_repair output" >>$seqres.full
> + err_msg="$msg (see $seqres.full)"
>
> ok=0
> fi
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2017-02-16 9:18 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-31 17:43 [PATCH 0/2][RESEND] Add report generators support Dmitry Monakhov
2017-01-31 17:43 ` [PATCH 1/2] check: prepare test report generator infrastructure Dmitry Monakhov
2017-02-16 9:18 ` Eryu Guan [this message]
2017-02-16 9:38 ` Dmitry Monakhov
2017-01-31 17:43 ` [PATCH 2/2] report: Add xunit format report generator Dmitry Monakhov
2017-02-16 10:08 ` Eryu Guan
-- strict thread matches above, loose matches on Subject: below --
2017-03-03 8:26 [PATCH 0/2] Add report generators support v3 Dmitry Monakhov
2017-03-03 8:26 ` [PATCH 1/2] check: prepare test report generator infrastructure Dmitry Monakhov
2017-02-21 12:44 [PATCH 0/2] Add report generators support v2 Dmitry Monakhov
2017-02-21 12:44 ` [PATCH 1/2] check: prepare test report generator infrastructure Dmitry Monakhov
2017-03-02 8:05 ` Eryu Guan
2017-01-31 17:16 [PATCH 0/2] Add report generators support Dmitry Monakhov
2017-01-31 17:16 ` [PATCH 1/2] check: prepare test report generator infrastructure Dmitry Monakhov
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=20170216091847.GW24562@eguan.usersys.redhat.com \
--to=eguan@redhat.com \
--cc=dmonakhov@openvz.org \
--cc=dmongw@gmail.com \
--cc=fstests@vger.kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox