| Message ID | 546A0BFE.6080608@plexistor.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 17, 2014 at 04:53:50PM +0200, Omer Zilberberg wrote: > > Another FYI: we can actually test any filesystem without a scratch > > device configured: > > > > $ sudo TEST_DEV=/dev/vda TEST_DIR=/mnt/test ./check generic/120 > > FSTYP -- xfs (non-debug) > > PLATFORM -- Linux/x86_64 test2 3.18.0-rc2-dgc+ > > > > generic/120 16s ... [not run] this test requires a valid $SCRATCH_DEV > > Not run: generic/120 > > Passed all 0 tests > > $ > > Please note that since commit 83ef157d, that is no longer true, because _require_test calls _is_block_dev with empty parameter, which prints usage: > $ sudo TEST_DEV=/dev/sda TEST_DIR=/mnt/dev0 ./check generic/001 > FSTYP -- ext4 > PLATFORM -- Linux/x86_64 testvm 3.17.0 > > generic/001 7s ... - output mismatch (see /opt/xfstests/results//generic/001.out.bad) > --- tests/generic/001.out 2014-09-10 11:04:44.249185592 +0300 > +++ /opt/xfstests/results//generic/001.out.bad 2014-11-17 15:14:12.380061760 +0200 > @@ -1,4 +1,5 @@ > QA output created by 001 > +Usage: _is_block_dev dev > cleanup > setup .................................... > iter 1 chain ... check .................................... > ... > (Run 'diff -u tests/generic/001.out /opt/xfstests/results//generic/001.out.bad' to see the entire diff) > Ran: generic/001 > Failures: generic/001 > Failed 1 of 1 tests > > Here's a patch to fix that: > ---- > Subject: [PATCH] _required_test: removed unneeded test for scratch_dev > > testing for scratch_dev in _required_test is unnecessary, since it is > not required for these tests. Furthermore, if a scratch_dev is not given, > all tests which are supposed to pass on test_dev fail, because > _is_block_dev is given an empty string as scratch_dev, and prints usage. I'm beginning to sound like a broken record. Why isn't this *invalid config check* being done in the config parsing rather than on every test that requires a scratch or test device? i.e. removing the check is not the correct fix - checking it during config parsing means none of these checks need to be done in _require_test, nor in _require_scratch_nocheck. i.e. in get_next_config() we already know the FSTYP, so we should be checking that: a) TEST_DEV is valid for fstyp b) SCRATCH_DEV (if configured) is valid for fstyp c) TEST_DEV != SCRATCH_DEV if they both exist and fstyp indicates they should be block devices And then we can pretty much assume that they are valid everywhere else, and _require_scratch_nocheck() simply needs to check for a non-empty string... > Signed-off-by: Omer Zilberberg <omzg@plexistor.com> > --- > common/rc | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/common/rc b/common/rc > index d5e3aff..b863808 100644 > --- a/common/rc > +++ b/common/rc > @@ -1104,7 +1104,7 @@ _require_scratch() > } > > > -# this test needs a test partition - check we're ok & unmount it > +# this test needs a test partition - check we're ok & mount if necessary > # > _require_test() > { > @@ -1138,10 +1138,6 @@ _require_test() > then > _notrun "this test requires a valid \$TEST_DEV" > fi > - if [ "`_is_block_dev $SCRATCH_DEV`" = "`_is_block_dev $TEST_DEV`" ] > - then > - _notrun "this test requires a valid \$TEST_DEV" > - fi > if [ ! -d "$TEST_DIR" ] > then > _notrun "this test requires a valid \$TEST_DIR" The patch is whitespace damaged. Please read: https://www.kernel.org/doc/Documentation/email-clients.txt and resend. Cheers, Dave.
diff --git a/common/rc b/common/rc index d5e3aff..b863808 100644 --- a/common/rc +++ b/common/rc @@ -1104,7 +1104,7 @@ _require_scratch() } -# this test needs a test partition - check we're ok & unmount it +# this test needs a test partition - check we're ok & mount if necessary # _require_test() { @@ -1138,10 +1138,6 @@ _require_test() then _notrun "this test requires a valid \$TEST_DEV" fi - if [ "`_is_block_dev $SCRATCH_DEV`" = "`_is_block_dev $TEST_DEV`" ] - then - _notrun "this test requires a valid \$TEST_DEV" - fi if [ ! -d "$TEST_DIR" ] then _notrun "this test requires a valid \$TEST_DIR"