[PATCH] xfstests: btrfs: fix up 001.out

[PATCH] xfstests: btrfs: fix up 001.out

[Anand Jain]

The subvol delete output has changed with btrfs-progs
    -Delete subvolume 'SCRATCH_MNT/snap'
    +Delete subvolume (no-commit): 'SCRATCH_MNT/snap'

make the matching changes in the xfstests btrfs 001.out

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/btrfs/001.out | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/btrfs/001.out b/tests/btrfs/001.out
index c782bde..8dc6eac 100644
--- a/tests/btrfs/001.out
+++ b/tests/btrfs/001.out
@@ -33,7 +33,7 @@ subvol
 Listing subvolumes
 snap
 subvol
-Delete subvolume 'SCRATCH_MNT/snap'
+Delete subvolume (no-commit): 'SCRATCH_MNT/snap'
 List root dir
 subvol
 List root dir
-- 
2.0.0.153.g79dcccc

Re: [PATCH] xfstests: btrfs: fix up 001.out

[Filipe David Manana]

On Wed, Dec 31, 2014 at 7:48 PM, Anand Jain <anand.jain@oracle.com> wrote:
> The subvol delete output has changed with btrfs-progs
>     -Delete subvolume 'SCRATCH_MNT/snap'
>     +Delete subvolume (no-commit): 'SCRATCH_MNT/snap'
>
> make the matching changes in the xfstests btrfs 001.out

Hi Anand,

This is a wrong approach to fix it.
With this change it means the test will now fail with a btrfs-progs
release older than v3.18...

The test should just ignore the output and check if the snapshot
creation command succeeds.
See how more recent tests do it - they are calling
_run_btrfs_util_prog (which calls run_check).

thanks

>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  tests/btrfs/001.out | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/btrfs/001.out b/tests/btrfs/001.out
> index c782bde..8dc6eac 100644
> --- a/tests/btrfs/001.out
> +++ b/tests/btrfs/001.out
> @@ -33,7 +33,7 @@ subvol
>  Listing subvolumes
>  snap
>  subvol
> -Delete subvolume 'SCRATCH_MNT/snap'
> +Delete subvolume (no-commit): 'SCRATCH_MNT/snap'
>  List root dir
>  subvol
>  List root dir
> --
> 2.0.0.153.g79dcccc
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

[PATCH v2] xfstests: btrfs: fix up 001.out

[Anand Jain]

The subvol delete output has changed with btrfs-progs
    -Delete subvolume 'SCRATCH_MNT/snap'
    +Delete subvolume (no-commit): 'SCRATCH_MNT/snap'

so fix 001 failing.

Signed-off-by: Anand Jain <anand.jain@oracle.com>

v2: Thanks Filipe for mentioning now we have _run_btrfs_util_prog. and
    commit update
---
 tests/btrfs/001     | 2 +-
 tests/btrfs/001.out | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/btrfs/001 b/tests/btrfs/001
index 8258d06..a7747c8 100755
--- a/tests/btrfs/001
+++ b/tests/btrfs/001
@@ -99,7 +99,7 @@ echo "Listing subvolumes"
 $BTRFS_UTIL_PROG subvolume list $SCRATCH_MNT | awk '{ print $NF }'
 
 # Delete the snapshot
-$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/snap | _filter_btrfs_subvol_delete
+_run_btrfs_util_prog subvolume delete $SCRATCH_MNT/snap
 echo "List root dir"
 ls $SCRATCH_MNT
 _scratch_remount
diff --git a/tests/btrfs/001.out b/tests/btrfs/001.out
index c782bde..43e8c56 100644
--- a/tests/btrfs/001.out
+++ b/tests/btrfs/001.out
@@ -33,7 +33,6 @@ subvol
 Listing subvolumes
 snap
 subvol
-Delete subvolume 'SCRATCH_MNT/snap'
 List root dir
 subvol
 List root dir
-- 
2.0.0.153.g79dcccc

Re: [PATCH] xfstests: btrfs: fix up 001.out

[Anand Jain]

>
> The test should just ignore the output and check if the snapshot
> creation command succeeds.
> See how more recent tests do it - they are calling
> _run_btrfs_util_prog (which calls run_check).

How nice we have _run_btrfs_util_prog. it was needed for a long time Thanks,
v2 is out.
Anand

Re: [PATCH v2] xfstests: btrfs: fix up 001.out

[Eryu Guan]

On Fri, Jan 02, 2015 at 09:04:29PM +0800, Anand Jain wrote:
> The subvol delete output has changed with btrfs-progs

Better to point out that since which btrfs-progs version the output
changed.

>     -Delete subvolume 'SCRATCH_MNT/snap'
>     +Delete subvolume (no-commit): 'SCRATCH_MNT/snap'
> 
> so fix 001 failing.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> v2: Thanks Filipe for mentioning now we have _run_btrfs_util_prog. and
>     commit update

I think a better way to fix this is to update the
_filter_btrfs_subvol_delete filter

Right now the filter does delete message about transaction commit:

	sed -e "/Transaction commit: none (default)/d"

Just adding another -e to sed to delete the "(no-commit):" part is fine.

Thanks,
Eryu
> ---
>  tests/btrfs/001     | 2 +-
>  tests/btrfs/001.out | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/001 b/tests/btrfs/001
> index 8258d06..a7747c8 100755
> --- a/tests/btrfs/001
> +++ b/tests/btrfs/001
> @@ -99,7 +99,7 @@ echo "Listing subvolumes"
>  $BTRFS_UTIL_PROG subvolume list $SCRATCH_MNT | awk '{ print $NF }'
>  
>  # Delete the snapshot
> -$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/snap | _filter_btrfs_subvol_delete
> +_run_btrfs_util_prog subvolume delete $SCRATCH_MNT/snap
>  echo "List root dir"
>  ls $SCRATCH_MNT
>  _scratch_remount
> diff --git a/tests/btrfs/001.out b/tests/btrfs/001.out
> index c782bde..43e8c56 100644
> --- a/tests/btrfs/001.out
> +++ b/tests/btrfs/001.out
> @@ -33,7 +33,6 @@ subvol
>  Listing subvolumes
>  snap
>  subvol
> -Delete subvolume 'SCRATCH_MNT/snap'
>  List root dir
>  subvol
>  List root dir
> -- 
> 2.0.0.153.g79dcccc
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] xfstests: btrfs: fix up 001.out

[Anand Jain]

On 01/05/2015 11:25 AM, Eryu Guan wrote:
> On Fri, Jan 02, 2015 at 09:04:29PM +0800, Anand Jain wrote:
>> The subvol delete output has changed with btrfs-progs
>
> Better to point out that since which btrfs-progs version the output
> changed.

  The fix here is output string change neutral, so it does not matter.

>>      -Delete subvolume 'SCRATCH_MNT/snap'
>>      +Delete subvolume (no-commit): 'SCRATCH_MNT/snap'
>>
>> so fix 001 failing.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>
>> v2: Thanks Filipe for mentioning now we have _run_btrfs_util_prog. and
>>      commit update
>
> I think a better way to fix this is to update the
> _filter_btrfs_subvol_delete filter
>
> Right now the filter does delete message about transaction commit:
>
> 	sed -e "/Transaction commit: none (default)/d"
>
> Just adding another -e to sed to delete the "(no-commit):" part is fine.

  in this case checking for the output string was fundamentally wrong 
for a long.

Thanks, Anand

> Thanks,
> Eryu
>> ---
>>   tests/btrfs/001     | 2 +-
>>   tests/btrfs/001.out | 1 -
>>   2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/tests/btrfs/001 b/tests/btrfs/001
>> index 8258d06..a7747c8 100755
>> --- a/tests/btrfs/001
>> +++ b/tests/btrfs/001
>> @@ -99,7 +99,7 @@ echo "Listing subvolumes"
>>   $BTRFS_UTIL_PROG subvolume list $SCRATCH_MNT | awk '{ print $NF }'
>>
>>   # Delete the snapshot
>> -$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/snap | _filter_btrfs_subvol_delete
>> +_run_btrfs_util_prog subvolume delete $SCRATCH_MNT/snap
>>   echo "List root dir"
>>   ls $SCRATCH_MNT
>>   _scratch_remount
>> diff --git a/tests/btrfs/001.out b/tests/btrfs/001.out
>> index c782bde..43e8c56 100644
>> --- a/tests/btrfs/001.out
>> +++ b/tests/btrfs/001.out
>> @@ -33,7 +33,6 @@ subvol
>>   Listing subvolumes
>>   snap
>>   subvol
>> -Delete subvolume 'SCRATCH_MNT/snap'
>>   List root dir
>>   subvol
>>   List root dir
>> --
>> 2.0.0.153.g79dcccc
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v2] xfstests: btrfs: fix up 001.out

[Dave Chinner]

On Fri, Jan 02, 2015 at 09:04:29PM +0800, Anand Jain wrote:
> The subvol delete output has changed with btrfs-progs
>     -Delete subvolume 'SCRATCH_MNT/snap'
>     +Delete subvolume (no-commit): 'SCRATCH_MNT/snap'
> 
> so fix 001 failing.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> v2: Thanks Filipe for mentioning now we have _run_btrfs_util_prog. and
>     commit update
> ---
>  tests/btrfs/001     | 2 +-
>  tests/btrfs/001.out | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/001 b/tests/btrfs/001
> index 8258d06..a7747c8 100755
> --- a/tests/btrfs/001
> +++ b/tests/btrfs/001
> @@ -99,7 +99,7 @@ echo "Listing subvolumes"
>  $BTRFS_UTIL_PROG subvolume list $SCRATCH_MNT | awk '{ print $NF }'
>  
>  # Delete the snapshot
> -$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/snap | _filter_btrfs_subvol_delete
> +_run_btrfs_util_prog subvolume delete $SCRATCH_MNT/snap

This is also the wrong way to fix the problem.

We have output filters for a reason, people:

_filter_btrfs_subvol_delete()
{
	_filter_scratch | _filter_transcation_commit_default
}

Simply becomes:

_filter_btrfs_subvol_delete()
{
	_filter_scratch | _filter_transcation_commit_default | \
		sed -e 's/^Delete subvolume.*:/Delete subvolume/'
}

The golden output does not change - the filter simply removes the
part of the message that changed between versions.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] xfstests: btrfs: fix up 001.out

[Anand Jain]

Dave,

On 01/21/2015 12:26 PM, Dave Chinner wrote:
> On Fri, Jan 02, 2015 at 09:04:29PM +0800, Anand Jain wrote:
>> The subvol delete output has changed with btrfs-progs
>>      -Delete subvolume 'SCRATCH_MNT/snap'
>>      +Delete subvolume (no-commit): 'SCRATCH_MNT/snap'
>>
>> so fix 001 failing.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>
>> v2: Thanks Filipe for mentioning now we have _run_btrfs_util_prog. and
>>      commit update
>> ---
>>   tests/btrfs/001     | 2 +-
>>   tests/btrfs/001.out | 1 -
>>   2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/tests/btrfs/001 b/tests/btrfs/001
>> index 8258d06..a7747c8 100755
>> --- a/tests/btrfs/001
>> +++ b/tests/btrfs/001
>> @@ -99,7 +99,7 @@ echo "Listing subvolumes"
>>   $BTRFS_UTIL_PROG subvolume list $SCRATCH_MNT | awk '{ print $NF }'
>>
>>   # Delete the snapshot
>> -$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/snap | _filter_btrfs_subvol_delete
>> +_run_btrfs_util_prog subvolume delete $SCRATCH_MNT/snap
>
> This is also the wrong way to fix the problem.
>
> We have output filters for a reason, people:

  actually I purposely discouraged using the output filters (instead use
  command exit code), mainly because output filters are
  unnecessary hindrance to the good changes. And UIs using btrfs-progs
  anyway has to depend on the exit code to check the command status,
  so we should rightfully check that in our test scripts, this may
  apply lightly for commands like show, but would fit well for commands
  like delete as in here. just my point of view.

Thanks,

> _filter_btrfs_subvol_delete()
> {
> 	_filter_scratch | _filter_transcation_commit_default
> }
>
> Simply becomes:
>
> _filter_btrfs_subvol_delete()
> {
> 	_filter_scratch | _filter_transcation_commit_default | \
> 		sed -e 's/^Delete subvolume.*:/Delete subvolume/'
> }
>
> The golden output does not change - the filter simply removes the
> part of the message that changed between versions.
>
> Cheers,
>
> Dave.
>