[PATCH] test: use test_must_be_empty F instead of test_cmp empty F

[PATCH] test: use test_must_be_empty F instead of test_cmp empty F

[René Scharfe]

Use test_must_be_empty instead of comparing it to an empty file.  That's
more efficient, as the function only needs built-in meta-data only check
in the usual case, and provides nicer debug output otherwise.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 t/t1011-read-tree-sparse-checkout.sh | 3 +--
 t/t9010-svn-fe.sh                    | 6 ++----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index ba71b159ba..eb44bafb59 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -215,7 +215,6 @@ test_expect_success 'read-tree adds to worktree, dirty case' '
 '

 test_expect_success 'index removal and worktree narrowing at the same time' '
-	>empty &&
 	echo init.t >.git/info/sparse-checkout &&
 	echo sub/added >>.git/info/sparse-checkout &&
 	git checkout -f top &&
@@ -223,7 +222,7 @@ test_expect_success 'index removal and worktree narrowing at the same time' '
 	git checkout removed &&
 	git ls-files sub/added >result &&
 	test ! -f sub/added &&
-	test_cmp empty result
+	test_must_be_empty result
 '

 test_expect_success 'read-tree --reset removes outside worktree' '
diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index 0b20b07e68..3a3d8d3bd5 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -53,9 +53,7 @@ text_no_props () {
 	printf "%s\n" "$text"
 }

->empty
-
-test_expect_success 'empty dump' '
+test_expect_success PIPE 'empty dump' '
 	reinit_git &&
 	echo "SVN-fs-dump-format-version: 2" >input &&
 	try_dump input
@@ -208,7 +206,7 @@ test_expect_failure 'timestamp and empty file' '
 	test_cmp expect.date actual.date &&
 	test_cmp expect.files actual.files &&
 	git checkout HEAD empty-file &&
-	test_cmp empty file
+	test_must_be_empty file
 '

 test_expect_success 'directory with files' '
--
2.24.0

Re: [PATCH] test: use test_must_be_empty F instead of test_cmp empty F

[Denton Liu]

Hi René,

On Tue, Nov 26, 2019 at 08:46:34PM +0100, René Scharfe wrote:
> Use test_must_be_empty instead of comparing it to an empty file.  That's
> more efficient, as the function only needs built-in meta-data only check
> in the usual case, and provides nicer debug output otherwise.
> 
> Signed-off-by: René Scharfe <l.s.r@web.de>

Nice, I like test cleanup :)

> ---

[...]

> diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
> index 0b20b07e68..3a3d8d3bd5 100755
> --- a/t/t9010-svn-fe.sh
> +++ b/t/t9010-svn-fe.sh
> @@ -53,9 +53,7 @@ text_no_props () {
>  	printf "%s\n" "$text"
>  }
> 
> ->empty
> -
> -test_expect_success 'empty dump' '
> +test_expect_success PIPE 'empty dump' '

How come we need the PIPE prereq here? At the top of this file, we have

	if test_have_prereq !PIPE
	then
		skip_all="svn dumpfile importer testing requires the PIPE prerequisite"
		test_done
	fi

which should guarantee PIPE at this point already.

Thanks,

Denton

>  	reinit_git &&
>  	echo "SVN-fs-dump-format-version: 2" >input &&
>  	try_dump input
> @@ -208,7 +206,7 @@ test_expect_failure 'timestamp and empty file' '
>  	test_cmp expect.date actual.date &&
>  	test_cmp expect.files actual.files &&
>  	git checkout HEAD empty-file &&
> -	test_cmp empty file
> +	test_must_be_empty file
>  '
> 
>  test_expect_success 'directory with files' '
> --
> 2.24.0
> 

Re: [PATCH] test: use test_must_be_empty F instead of test_cmp empty F

[René Scharfe]

Am 26.11.19 um 21:02 schrieb Denton Liu:
>> diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
>> index 0b20b07e68..3a3d8d3bd5 100755
>> --- a/t/t9010-svn-fe.sh
>> +++ b/t/t9010-svn-fe.sh
>> @@ -53,9 +53,7 @@ text_no_props () {
>>  	printf "%s\n" "$text"
>>  }
>>
>> ->empty
>> -
>> -test_expect_success 'empty dump' '
>> +test_expect_success PIPE 'empty dump' '
>
> How come we need the PIPE prereq here? At the top of this file, we have
>
> 	if test_have_prereq !PIPE
> 	then
> 		skip_all="svn dumpfile importer testing requires the PIPE prerequisite"
> 		test_done
> 	fi
>
> which should guarantee PIPE at this point already.

Right, we haven't needed it since 7236a34c98 (t9010-*.sh: skip all tests
if the PIPE prereq is missing, 2017-09-17).  Thanks for catching this!

My patch had been gathering dust here already before that time, and
sometime along the way I did a sloppy rebase.  Didn't see it before
sending because I was focusing on "can we remove the empty file?".  O_o

René

[PATCH v2] test: use test_must_be_empty F instead of test_cmp empty F

[René Scharfe]

Use test_must_be_empty instead of comparing it to an empty file.  That's
more efficient, as the function only needs built-in meta-data only check
in the usual case, and provides nicer debug output otherwise.

Helped-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Change since v1: Don't re-add PIPE prerequisite.

 t/t1011-read-tree-sparse-checkout.sh | 3 +--
 t/t9010-svn-fe.sh                    | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index ba71b159ba..eb44bafb59 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -215,7 +215,6 @@ test_expect_success 'read-tree adds to worktree, dirty case' '
 '

 test_expect_success 'index removal and worktree narrowing at the same time' '
-	>empty &&
 	echo init.t >.git/info/sparse-checkout &&
 	echo sub/added >>.git/info/sparse-checkout &&
 	git checkout -f top &&
@@ -223,7 +222,7 @@ test_expect_success 'index removal and worktree narrowing at the same time' '
 	git checkout removed &&
 	git ls-files sub/added >result &&
 	test ! -f sub/added &&
-	test_cmp empty result
+	test_must_be_empty result
 '

 test_expect_success 'read-tree --reset removes outside worktree' '
diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index 0b20b07e68..c90fdc5c89 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -53,8 +53,6 @@ text_no_props () {
 	printf "%s\n" "$text"
 }

->empty
-
 test_expect_success 'empty dump' '
 	reinit_git &&
 	echo "SVN-fs-dump-format-version: 2" >input &&
@@ -208,7 +206,7 @@ test_expect_failure 'timestamp and empty file' '
 	test_cmp expect.date actual.date &&
 	test_cmp expect.files actual.files &&
 	git checkout HEAD empty-file &&
-	test_cmp empty file
+	test_must_be_empty file
 '

 test_expect_success 'directory with files' '
--
2.24.0