diff mbox

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

Message ID 1420203869-26085-1-git-send-email-anand.jain@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anand Jain Jan. 2, 2015, 1:04 p.m. UTC
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(-)

Comments

Eryu Guan Jan. 5, 2015, 3:25 a.m. UTC | #1
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
--
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
Anand Jain Jan. 5, 2015, 4:40 a.m. UTC | #2
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
>
--
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
Dave Chinner Jan. 21, 2015, 4:26 a.m. UTC | #3
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.
Anand Jain Jan. 21, 2015, 5:30 a.m. UTC | #4
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.
>
--
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
diff mbox

Patch

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