xfs
[Top] [All Lists]

Re: [PATCH] xfstests: make length of diff output configurable

To: David Sterba <dsterba@xxxxxxx>
Subject: Re: [PATCH] xfstests: make length of diff output configurable
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 23 Jan 2013 08:44:35 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1358863307-16489-1-git-send-email-dsterba@xxxxxxx>
References: <1358863307-16489-1-git-send-email-dsterba@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Jan 22, 2013 at 03:01:46PM +0100, David Sterba wrote:
> In commit 11c1d79414e2571 "xfstests: Change the diff output of failed
> tests", the diff output of a failed test was hardcoded to 10 lines to
> avoid overly long output and user can get the full output by manually
> running the diff. However this is not always possible and convenient,
> eg. in repeated automated tests where the required information is lost
> after the test round finished. Then the caputred logs do not contain
> enough informatin for analysis.

Ah, the problem of pulling in year old patches from the list without
first posting them for review again has struck again. I noticed this
same change of behaviour a couple of days again when I updated and
it lead to a few WTF moments for me, too.

Maintainers, we've talked about this before. Please don't just pull
in old patches that have been sitting on the list ignored for months
- post them for review again first so people at least have a heads
up that behaviour is going to change soon.

> Introduce the DIFF_LENGTH env variable to tune the diff size, keeping it
> 10 as deafult and 0 to disable the limit.

Seems reasonable. Implementation comment below.

> Signed-off-by: David Sterba <dsterba@xxxxxxx>
> ---
>  README        | 2 ++
>  check         | 7 ++++++-
>  common.config | 2 ++
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/README b/README
> index d81ede9..b70f75d 100644
> --- a/README
> +++ b/README
> @@ -63,6 +63,8 @@ Preparing system for tests (IRIX and Linux):
>               - if TEST_LOGDEV and/or TEST_RTDEV, these will always be used.
>               - if SCRATCH_LOGDEV and/or SCRATCH_RTDEV, the USE_EXTERNAL
>                 environment variable set to "yes" will enable their use.
> +             - setenv DIFF_LENGTH "number of diff lines to print from a 
> failed test",
> +               by default 10, set to 0 to print the full diff
>          - or add a case to the switch in common.config assigning
>            these variables based on the hostname of your test
>            machine
> diff --git a/check b/check
> index 75addb5..8b79678 100755
> --- a/check
> +++ b/check
> @@ -287,7 +287,12 @@ do
>               else
>                   echo " - output mismatch (see $seq.out.bad)"
>                   mv $tmp.out $seq.out.bad
> -                 $diff $seq.out $seq.out.bad | head -n 10 | \
> +                 $diff $seq.out $seq.out.bad | {
> +                     if [ "$DIFF_LENGTH" -le 0 ]; then
> +                             cat
> +                     else
> +                             head -n "$DIFF_LENGTH"
> +                     fi; } | \
>                       sed -e 's/^\(.\)/    \1/'
>                   echo "     ..."
>                   echo "     (Run '$diff $seq.out $seq.out.bad' to see the" \
> diff --git a/common.config b/common.config
> index 57f505d..8dfe574 100644
> --- a/common.config
> +++ b/common.config
> @@ -63,6 +63,8 @@ HOSTOS=`uname -s`
>  MODULAR=0               # using XFS as a module or not
>  BOOT="/boot"            # install target for kernels
>  export EXTRA=${EXTRA:=xfs-qa}
> +# number of diff lines from a failed test, 0 for whole output
> +export DIFF_LENGTH=${DIFF_LENGTH:=10}

I'd actually set that in the check executable, as the diff output is
a function of the check program, not the generic test harness
config...

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>