From: Dmitry Monakhov <dmonakhov@openvz.org>
To: Eryu Guan <eguan@redhat.com>, Dmitry Monakhov <dmongw@gmail.com>
Cc: fstests@vger.kernel.org
Subject: Re: [PATCH 1/2] check: prepare test report generator infrastructure
Date: Thu, 16 Feb 2017 12:38:24 +0300 [thread overview]
Message-ID: <877f4qh3vz.fsf@dmlp.sw.ru> (raw)
In-Reply-To: <20170216091847.GW24562@eguan.usersys.redhat.com>
Eryu Guan <eguan@redhat.com> writes:
> 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?
Definitely. This is reasonable idea, I'll resend updated version.
>
> 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:38 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
2017-02-16 9:38 ` Dmitry Monakhov [this message]
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=877f4qh3vz.fsf@dmlp.sw.ru \
--to=dmonakhov@openvz.org \
--cc=dmongw@gmail.com \
--cc=eguan@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.