2019-10-09 08:23:30

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH xfstests] generic/192: Move 'cd /' to the place where the program exits

Running generic/192 with overlayfs(Let ubifs as base fs) yields the
following output:

generic/192 - output mismatch
QA output created by 192
sleep for 5 seconds
test
+./common/rc: line 316: src/t_dir_type: No such file or directory
delta1 is in range
delta2 is in range
...

When the use case fails, the call stack in generic/192 is:

local unknowns=$(src/t_dir_type $dir u | wc -l) common/rc:316
_supports_filetype common/rc:299
_overlay_mount common/overlay:52
_overlay_test_mount common/overlay:93
_test_mount common/rc:407
_test_cycle_mount generic/192:50

Before _test_cycle_mount() being invoked, generic/192 executed 'cd /'
to change work dir from 'xfstests-dev' to '/', so src/t_dir_type was not
found.

Signed-off-by: Zhihao Cheng <[email protected]>
---
tests/generic/192 | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/generic/192 b/tests/generic/192
index 50b3d6fd..5550f39e 100755
--- a/tests/generic/192
+++ b/tests/generic/192
@@ -15,7 +15,12 @@ echo "QA output created by $seq"
here=`pwd`
tmp=/tmp/$$
status=1 # failure is the default!
-trap "exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+}

_access_time()
{
@@ -46,7 +51,6 @@ sleep $delay # sleep to allow time to move on for access
cat $testfile
time2=`_access_time $testfile | tee -a $seqres.full`

-cd /
_test_cycle_mount
time3=`_access_time $testfile | tee -a $seqres.full`

--
2.13.6


2019-10-13 12:47:46

by Eryu Guan

[permalink] [raw]
Subject: Re: [PATCH xfstests] generic/192: Move 'cd /' to the place where the program exits

On Wed, Oct 09, 2019 at 04:27:57PM +0800, Zhihao Cheng wrote:
> Running generic/192 with overlayfs(Let ubifs as base fs) yields the
> following output:
>
> generic/192 - output mismatch
> QA output created by 192
> sleep for 5 seconds
> test
> +./common/rc: line 316: src/t_dir_type: No such file or directory
> delta1 is in range
> delta2 is in range
> ...
>
> When the use case fails, the call stack in generic/192 is:
>
> local unknowns=$(src/t_dir_type $dir u | wc -l) common/rc:316
> _supports_filetype common/rc:299
> _overlay_mount common/overlay:52
> _overlay_test_mount common/overlay:93
> _test_mount common/rc:407
> _test_cycle_mount generic/192:50
>
> Before _test_cycle_mount() being invoked, generic/192 executed 'cd /'
> to change work dir from 'xfstests-dev' to '/', so src/t_dir_type was not
> found.
>
> Signed-off-by: Zhihao Cheng <[email protected]>

Thanks for the debug! But I think the right fix is to call t_dir_type
via "$here", i.e.

local unknowns=$($here/src/t_dir_type $dir u | wc -l)

'here', which points to the top level dir of xfstests source code, is
defined in every test in test setup, and is guaranteed not to be empty.

Thanks,
Eryu

> ---
> tests/generic/192 | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tests/generic/192 b/tests/generic/192
> index 50b3d6fd..5550f39e 100755
> --- a/tests/generic/192
> +++ b/tests/generic/192
> @@ -15,7 +15,12 @@ echo "QA output created by $seq"
> here=`pwd`
> tmp=/tmp/$$
> status=1 # failure is the default!
> -trap "exit \$status" 0 1 2 3 15
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> +}
>
> _access_time()
> {
> @@ -46,7 +51,6 @@ sleep $delay # sleep to allow time to move on for access
> cat $testfile
> time2=`_access_time $testfile | tee -a $seqres.full`
>
> -cd /
> _test_cycle_mount
> time3=`_access_time $testfile | tee -a $seqres.full`
>
> --
> 2.13.6
>

2019-10-14 02:30:31

by Zhihao Cheng

[permalink] [raw]
Subject: Re: [PATCH xfstests] generic/192: Move 'cd /' to the place where the program exits

I agree with your proposal. Indeed, '$here' is referenced in other places where executable files under src are used, including '$here/src/feature', '$here/src/seek_sanity_test', etc.
I have a question about why many test cases execute 'cd /' before the end. For example, generic/124, generic/122, generic/003. I wonder the intention of operation 'cd /'.

?? 2019/10/13 20:46, Eryu Guan ะด??:
> On Wed, Oct 09, 2019 at 04:27:57PM +0800, Zhihao Cheng wrote:
>> Running generic/192 with overlayfs(Let ubifs as base fs) yields the
>> following output:
>>
>> generic/192 - output mismatch
>> QA output created by 192
>> sleep for 5 seconds
>> test
>> +./common/rc: line 316: src/t_dir_type: No such file or directory
>> delta1 is in range
>> delta2 is in range
>> ...
>>
>> When the use case fails, the call stack in generic/192 is:
>>
>> local unknowns=$(src/t_dir_type $dir u | wc -l) common/rc:316
>> _supports_filetype common/rc:299
>> _overlay_mount common/overlay:52
>> _overlay_test_mount common/overlay:93
>> _test_mount common/rc:407
>> _test_cycle_mount generic/192:50
>>
>> Before _test_cycle_mount() being invoked, generic/192 executed 'cd /'
>> to change work dir from 'xfstests-dev' to '/', so src/t_dir_type was not
>> found.
>>
>> Signed-off-by: Zhihao Cheng <[email protected]>
>
> Thanks for the debug! But I think the right fix is to call t_dir_type
> via "$here", i.e.
>
> local unknowns=$($here/src/t_dir_type $dir u | wc -l)
>
> 'here', which points to the top level dir of xfstests source code, is
> defined in every test in test setup, and is guaranteed not to be empty.
>
> Thanks,
> Eryu
>
>> ---
>> tests/generic/192 | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/generic/192 b/tests/generic/192
>> index 50b3d6fd..5550f39e 100755
>> --- a/tests/generic/192
>> +++ b/tests/generic/192
>> @@ -15,7 +15,12 @@ echo "QA output created by $seq"
>> here=`pwd`
>> tmp=/tmp/$$
>> status=1 # failure is the default!
>> -trap "exit \$status" 0 1 2 3 15
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> + cd /
>> +}
>>
>> _access_time()
>> {
>> @@ -46,7 +51,6 @@ sleep $delay # sleep to allow time to move on for access
>> cat $testfile
>> time2=`_access_time $testfile | tee -a $seqres.full`
>>
>> -cd /
>> _test_cycle_mount
>> time3=`_access_time $testfile | tee -a $seqres.full`
>>
>> --
>> 2.13.6
>>
>
> .
>