diff mbox

fstests: link .out to correct output when we set USE_ATTR_SECURE=yes

Message ID CAHpGcMLZT5xYx5EUBLTPyLQd0f+jQ-g+c_f-oErsHP33m7-ROg@mail.gmail.com
State Not Applicable
Headers show

Commit Message

Andreas Grünbacher Aug. 20, 2015, 6:18 a.m. UTC
Dongsheng,

2015-08-20 3:01 GMT+02:00 Dongsheng Yang <yangds.fnst@cn.fujitsu.com>:
>
> When we set USE_ATTR_SECURE to yes or no, the expected outputs of generic/062
> would be different. So we need to link the .out to different file.

Can the tests for the different namespaces please be separated from
each other here (see below)?

Next, can the security namespace tests be put in their own test case
so that we don't need to switch between two different .out files? This
could be done by setting ATTR_MODES and then invoking
tests/generic/062 to avoid duplicating the entire test script.

Thanks,
Andreas

Comments

Dave Chinner Aug. 20, 2015, 9:08 p.m. UTC | #1
On Thu, Aug 20, 2015 at 08:18:28AM +0200, Andreas Grünbacher wrote:
> Dongsheng,
> 
> 2015-08-20 3:01 GMT+02:00 Dongsheng Yang <yangds.fnst@cn.fujitsu.com>:
> >
> > When we set USE_ATTR_SECURE to yes or no, the expected outputs of generic/062
> > would be different. So we need to link the .out to different file.
> 
> Can the tests for the different namespaces please be separated from
> each other here (see below)?

I certainly agree with this approach ;)

> 
> diff --git a/tests/generic/062 b/tests/generic/062
> index 194b638..9dd4498 100755
> --- a/tests/generic/062
> +++ b/tests/generic/062
> @@ -142,6 +142,7 @@ for nsp in $ATTR_MODES; do
>                 echo "*** final list (strings, type=$inode, nsp=$nsp)"
>                 getfattr -m '.' -e hex $SCRATCH_MNT/$inode
> 
> +               # FIXME: Remove all remaining xattrs
>         done
>  done
> 
> Next, can the security namespace tests be put in their own test case
> so that we don't need to switch between two different .out files? This
> could be done by setting ATTR_MODES and then invoking
> tests/generic/062 to avoid duplicating the entire test script.

Just not the suggested mechanism :)

We can't execute one test from another - that will lead to all sorts
of bad juju occurring with results and timing and so on because they
are all based on $0.  The correct way to do this is to factor the
test internals into a common file, then wrap the common functions
with a new test file.

e.g. see _test_generic_punch(), which started off as corner case
testing the XFS_IOC_ZERORANGE ioctl in a single test, Then got
extended to testing hole punching in a new test, then got factored
into common functions in common/punch, and it is now called by 14
different tests....

Cheers,

Dave.
Andreas Grünbacher Aug. 20, 2015, 9:17 p.m. UTC | #2
2015-08-20 23:08 GMT+02:00 Dave Chinner <david@fromorbit.com>:
> The correct way to do this [...]

Yes, that's better.

Thanks,
Andreas
Dongsheng Yang Aug. 21, 2015, 12:36 a.m. UTC | #3
On 08/21/2015 05:08 AM, Dave Chinner wrote:
> On Thu, Aug 20, 2015 at 08:18:28AM +0200, Andreas Grünbacher wrote:
>> Dongsheng,
>>
>> 2015-08-20 3:01 GMT+02:00 Dongsheng Yang <yangds.fnst@cn.fujitsu.com>:
>>>
>>> When we set USE_ATTR_SECURE to yes or no, the expected outputs of generic/062
>>> would be different. So we need to link the .out to different file.
>>
>> Can the tests for the different namespaces please be separated from
>> each other here (see below)?
>
> I certainly agree with this approach ;)
>
>>
>> diff --git a/tests/generic/062 b/tests/generic/062
>> index 194b638..9dd4498 100755
>> --- a/tests/generic/062
>> +++ b/tests/generic/062
>> @@ -142,6 +142,7 @@ for nsp in $ATTR_MODES; do
>>                  echo "*** final list (strings, type=$inode, nsp=$nsp)"
>>                  getfattr -m '.' -e hex $SCRATCH_MNT/$inode
>>
>> +               # FIXME: Remove all remaining xattrs
>>          done
>>   done
>>
>> Next, can the security namespace tests be put in their own test case
>> so that we don't need to switch between two different .out files? This
>> could be done by setting ATTR_MODES and then invoking
>> tests/generic/062 to avoid duplicating the entire test script.
>
> Just not the suggested mechanism :)
>
> We can't execute one test from another - that will lead to all sorts
> of bad juju occurring with results and timing and so on because they
> are all based on $0.  The correct way to do this is to factor the
> test internals into a common file, then wrap the common functions
> with a new test file.

Sounds good, thanx Dave and Andreas.

Yang
>
> e.g. see _test_generic_punch(), which started off as corner case
> testing the XFS_IOC_ZERORANGE ioctl in a single test, Then got
> extended to testing hole punching in a new test, then got factored
> into common functions in common/punch, and it is now called by 14
> different tests....
>
> Cheers,
>
> Dave.
>
diff mbox

Patch

diff --git a/tests/generic/062 b/tests/generic/062
index 194b638..9dd4498 100755
--- a/tests/generic/062
+++ b/tests/generic/062
@@ -142,6 +142,7 @@  for nsp in $ATTR_MODES; do
                echo "*** final list (strings, type=$inode, nsp=$nsp)"
                getfattr -m '.' -e hex $SCRATCH_MNT/$inode

+               # FIXME: Remove all remaining xattrs
        done
 done