Re: [PATCH 11/10] xfstests: rework large filesystem testing - add golden output

[Dave Chinner <david@fromorbit.com>]

On Thu, Sep 06, 2012 at 07:57:18AM -0500, Rich Johnston wrote:
> Thanks for the comments.
> 
> On 09/05/2012 05:26 PM, Dave Chinner wrote:
> >On Fri, Aug 31, 2012 at 02:43:27PM -0500, rjohnston@sgi.com wrote:
> >>Patch "rework large filesystem testing" introduces a new option --large-fs
> >>which creates a new file $SCRATCH_MNT/.use_space.  If this 10 part patchset is
> >>applied, the following tests will fail:
> >>	019 026 027 028 046 047 050 056 059 060 062 063 064 065 066
> >
> >That's a lot more tests than I see failing.
> 
> It is very repeatable for me.
> 
> >
> >>This patch accounts for the following new output when testing xfs filesystems with
> >>the --large-fs option by creating new output file to compare against
> >>($seq.largefs.out):
> >
> >Creating new output files is the absolute last resort.  Indeed, what
> >happens when you get different output for tests that already select
> >an output file based on, say, platform or some other criteria? We
> >get a combinatorial explosion of golden output files, and that is
> >simply not manageable.
> >
> >The usual thing to do is update the necessary filters or change the
> >way the tests run to avoid trivial output file differences e.g. use
> >a subdir rather than SCRATCH_MNT directly. Or, for example the
> >filters that munge different standard error messages from different
> >platforms to be the same...
> >
> 
> OK good to know.
> 
> >>1. The following four lines appear in test 019.
> >>	 File: "./.use_space"
> >>	 Size: 6312890368 Filetype: Regular File
> >>	Mode: (0600/-rw-------) Uid: (0) Gid: (0)
> >>	Device: <DEVICE> Inode: <INODE> Links: 1
> >
> >This test doesn't really need to be run for large filesystems -
> >running it on large filesystems doesn't improve the coverage of or
> >our confidence in the code it is testing, so I'd just add a
> >_require_no_large_scratch_dev to it.
> >
> 
> Works for me.
> 
> >>2. When the nodump attribute is set, the xfsdump -e option will cause the
> >>    following additional lines to appear.
> >>	xfsdump: NOTE: pruned 1 files: skip attribute set
> >>	Only in SCRATCH_MNT: .use_space
> >>	SCRATCH_MNT/.use_space
> >
> >Ok, those are the errors I haven't seen - not sure why. I'll have to
> >look into that.
> >
> >However, this is definitely a case of updating the dump output
> >filter to remove these messages from the output stream.  The
> >alternative is to change the common dump code to use a subdirectory
> >rather than the root directory so it doesn't see these files at all.
> >
> 
> Good suggestion
> 
> >>3. Number of files off by one.
> >>	xfsrestore: # directories and (off by 1) entries processed
> >
> >That would be fixed by using a subdir for the dump tests. I don't
> >recommend that the number should be filtered, as having dump report
> >the correct number of files scanned is important.
> 
> I agree.
> 
> >
> >>	[ROOT] 0 0 0 00 [--------] (off by 1) 0 0 00 [--------] 0 0 0 00 [--------]
> >
> >Perhaps the usre/group of the use_space file needs to be changed so
> >it doesn't impact on the test results. Alternatively, a filter could
> >be written/modified to fix the number appropriately.
> 
> Sounds reasonable.
> 
> >
> >>This patch also modifies check and common.quota to use the new output file
> >>$seq.largefs.out when the --large-fs option is used (LARGE_SCRATCH_DEV = yes)
> >>or  $seq.out when the --large-fs option is NOT used (LARGE_SCRATCH_DEV != yes).
> >>
> >>Signed-off-by: Rich Johnston <rjohnston@sgi.com>
> >>
> >>---
> >>  019.largefs.out |    5 +++
> >>  026.largefs.out |    4 ++-
> >>  027.largefs.out |    2 -
> >>  028.largefs.out |    5 +++
> >>  046.largefs.out |    3 +-
> >>  047.largefs.out |    5 +++
> >>  050.largefs.out |   72 ++++++++++++++++++++++++++++----------------------------
> >>  056.largefs.out |    3 +-
> >>  059.largefs.out |    2 +
> >>  060.largefs.out |    4 ++-
> >>  062.largefs.out |    2 +
> >>  063.largefs.out |    3 +-
> >>  064.largefs.out |   41 ++++++++++++++++---------------
> >>  065.largefs.out |   29 +++++++++++-----------
> >>  066.largefs.out |    3 +-
> >>  check           |   12 +++++++--
> >>  common.quota    |   20 ++++++++++-----
> >>  17 files changed, 128 insertions(+), 87 deletions(-)
> >
> >FWIW, this patch is supposed to add these *.largefs.out files, right? The
> >patch, however:
> >
> >>Index: b/019.largefs.out
> >>===================================================================
> >>--- a/019.largefs.out
> >>+++ b/019.largefs.out
> >>@@ -9,6 +9,11 @@ Wrote 2048.00Kb (value 0x2c)
> >>   Mode: (0777/drwxrwxrwx) Uid: (3) Gid: (1)
> >>  Device: <DEVICE> Inode: <INODE> Links: 3
> >>
> >>+ File: "./.use_space"
> >>+ Size: 6312890368 Filetype: Regular File
> >>+ Mode: (0600/-rw-------) Uid: (0) Gid: (0)
> >>+Device: <DEVICE> Inode: <INODE> Links: 1
> >>+
> >>   File: "./bigfile"
> >>   Size: 2097152 Filetype: Regular File
> >>   Mode: (0666/-rw-rw-rw-) Uid: (3) Gid: (0)
> >
> >... assumes they already exist...
> >
> 
> Yup my bad, I only posted the differences from the original *.out files.
> 
> May I make the suggested changes, or as this is your patchset do you
> want to make them?

I've got a couple of other fixes to make to the series, so I'll work
these in. Thanks for reporting them and prototyping fixes for them.
:)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs