git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] tests: consolidate the `file_size` function into `test-lib-functions.sh`
@ 2020-11-07  1:12 Johannes Schindelin via GitGitGadget
  2020-11-07  1:23 ` brian m. carlson
  0 siblings, 1 reply; 2+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2020-11-07  1:12 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Schindelin

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In 8de7eeb54b6 (compression: unify pack.compression configuration
parsing, 2016-11-15), we introduced identical copies of the `file_size`
helper into three test scripts, with the plan to eventually consolidate
them into a single copy.

Let's do that, and adjust the function name to adhere to the `test_*`
naming convention.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    tests: consolidate the file_size function into test-lib-functions.sh
    
    My ulterior motive with this patch is not even to address that old
    concern, but to avoid having to exclude the code comments from the
    upcoming master -> main rename (because those code comments talk about
    git/git's main branch, which is called master).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-780%2Fdscho%2Ffile_size-test-function-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-780/dscho/file_size-test-function-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/780

 t/t0021-conversion.sh               | 36 +++++++++++++----------------
 t/t1050-large.sh                    |  8 +------
 t/t5315-pack-objects-compression.sh |  8 +------
 t/t9303-fast-import-compression.sh  | 10 ++------
 t/test-lib-functions.sh             |  4 ++++
 5 files changed, 24 insertions(+), 42 deletions(-)

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 4bfffa9c31..f6deaf498b 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -23,10 +23,6 @@ generate_random_characters () {
 		perl -pe "s/./chr((ord($&) % 26) + ord('a'))/sge" >"$TEST_ROOT/$NAME"
 }
 
-file_size () {
-	test-tool path-utils file-size "$1"
-}
-
 filter_git () {
 	rm -f *.log &&
 	git "$@"
@@ -361,9 +357,9 @@ test_expect_success PERL 'required process filter should filter data' '
 		cp "$TEST_ROOT/test3 '\''sq'\'',\$x=.o" "testsubdir/test3 '\''sq'\'',\$x=.r" &&
 		>test4-empty.r &&
 
-		S=$(file_size test.r) &&
-		S2=$(file_size test2.r) &&
-		S3=$(file_size "testsubdir/test3 '\''sq'\'',\$x=.r") &&
+		S=$(test_file_size test.r) &&
+		S2=$(test_file_size test2.r) &&
+		S3=$(test_file_size "testsubdir/test3 '\''sq'\'',\$x=.r") &&
 		M=$(git hash-object test.r) &&
 		M2=$(git hash-object test2.r) &&
 		M3=$(git hash-object "testsubdir/test3 '\''sq'\'',\$x=.r") &&
@@ -432,9 +428,9 @@ test_expect_success PERL 'required process filter should filter data for various
 	(
 		cd repo &&
 
-		S=$(file_size test.r) &&
-		S2=$(file_size test2.r) &&
-		S3=$(file_size "testsubdir/test3 '\''sq'\'',\$x=.r") &&
+		S=$(test_file_size test.r) &&
+		S2=$(test_file_size test2.r) &&
+		S3=$(test_file_size "testsubdir/test3 '\''sq'\'',\$x=.r") &&
 		M=$(git hash-object test.r) &&
 		M2=$(git hash-object test2.r) &&
 		M3=$(git hash-object "testsubdir/test3 '\''sq'\'',\$x=.r") &&
@@ -549,7 +545,7 @@ test_expect_success PERL 'required process filter takes precedence' '
 
 		echo "*.r filter=protocol" >.gitattributes &&
 		cp "$TEST_ROOT/test.o" test.r &&
-		S=$(file_size test.r) &&
+		S=$(test_file_size test.r) &&
 
 		# Check that the process filter is invoked here
 		filter_git add . &&
@@ -573,7 +569,7 @@ test_expect_success PERL 'required process filter should be used only for "clean
 
 		echo "*.r filter=protocol" >.gitattributes &&
 		cp "$TEST_ROOT/test.o" test.r &&
-		S=$(file_size test.r) &&
+		S=$(test_file_size test.r) &&
 
 		filter_git add . &&
 		cat >expected.log <<-EOF &&
@@ -697,9 +693,9 @@ test_expect_success PERL 'process filter should restart after unexpected write f
 		echo "this is going to fail" >smudge-write-fail.o &&
 		cp smudge-write-fail.o smudge-write-fail.r &&
 
-		S=$(file_size test.r) &&
-		S2=$(file_size test2.r) &&
-		SF=$(file_size smudge-write-fail.r) &&
+		S=$(test_file_size test.r) &&
+		S2=$(test_file_size test2.r) &&
+		SF=$(test_file_size smudge-write-fail.r) &&
 		M=$(git hash-object test.r) &&
 		M2=$(git hash-object test2.r) &&
 		MF=$(git hash-object smudge-write-fail.r) &&
@@ -752,9 +748,9 @@ test_expect_success PERL 'process filter should not be restarted if it signals a
 		echo "this will cause an error" >error.o &&
 		cp error.o error.r &&
 
-		S=$(file_size test.r) &&
-		S2=$(file_size test2.r) &&
-		SE=$(file_size error.r) &&
+		S=$(test_file_size test.r) &&
+		S2=$(test_file_size test2.r) &&
+		SE=$(test_file_size error.r) &&
 		M=$(git hash-object test.r) &&
 		M2=$(git hash-object test2.r) &&
 		ME=$(git hash-object error.r) &&
@@ -797,7 +793,7 @@ test_expect_success PERL 'process filter abort stops processing of all further f
 
 		M="blob=$(git hash-object abort.r)" &&
 		rm -f debug.log &&
-		SA=$(file_size abort.r) &&
+		SA=$(test_file_size abort.r) &&
 
 		git add . &&
 		rm -f *.r &&
@@ -859,7 +855,7 @@ test_expect_success PERL 'delayed checkout in process filter' '
 		git commit -m "test commit"
 	) &&
 
-	S=$(file_size "$TEST_ROOT/test.o") &&
+	S=$(test_file_size "$TEST_ROOT/test.o") &&
 	PM="ref=refs/heads/master treeish=$(git -C repo rev-parse --verify master) " &&
 	M="${PM}blob=$(git -C repo rev-parse --verify master:test.a)" &&
 	cat >a.exp <<-EOF &&
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index 61e89a8071..4bab6a513c 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -5,12 +5,6 @@ test_description='adding and checking out large blobs'
 
 . ./test-lib.sh
 
-# This should be moved to test-lib.sh together with the
-# copy in t0021 after both topics have graduated to 'master'.
-file_size () {
-	test-tool path-utils file-size "$1"
-}
-
 test_expect_success setup '
 	# clone does not allow us to pass core.bigfilethreshold to
 	# new repos, so set core.bigfilethreshold globally
@@ -29,7 +23,7 @@ do
 	test_expect_success "add with $config" '
 		test_when_finished "rm -f .git/objects/pack/pack-*.* .git/index" &&
 		git $config add large1 &&
-		sz=$(file_size .git/objects/pack/pack-*.pack) &&
+		sz=$(test_file_size .git/objects/pack/pack-*.pack) &&
 		case "$expect" in
 		small) test "$sz" -le 100000 ;;
 		large) test "$sz" -ge 100000 ;;
diff --git a/t/t5315-pack-objects-compression.sh b/t/t5315-pack-objects-compression.sh
index df970d7584..8bacd96275 100755
--- a/t/t5315-pack-objects-compression.sh
+++ b/t/t5315-pack-objects-compression.sh
@@ -4,12 +4,6 @@ test_description='pack-object compression configuration'
 
 . ./test-lib.sh
 
-# This should be moved to test-lib.sh together with the
-# copy in t0021 after both topics have graduated to 'master'.
-file_size () {
-	test-tool path-utils file-size "$1"
-}
-
 test_expect_success setup '
 	printf "%2000000s" X |
 	git hash-object -w --stdin >object-name &&
@@ -24,7 +18,7 @@ do
 	test_expect_success "pack-objects with $config" '
 		test_when_finished "rm -f pack-*.*" &&
 		git $config pack-objects pack <object-name &&
-		sz=$(file_size pack-*.pack) &&
+		sz=$(test_file_size pack-*.pack) &&
 		case "$expect" in
 		small) test "$sz" -le 100000 ;;
 		large) test "$sz" -ge 100000 ;;
diff --git a/t/t9303-fast-import-compression.sh b/t/t9303-fast-import-compression.sh
index 5045f02a53..57d916524e 100755
--- a/t/t9303-fast-import-compression.sh
+++ b/t/t9303-fast-import-compression.sh
@@ -3,12 +3,6 @@
 test_description='compression setting of fast-import utility'
 . ./test-lib.sh
 
-# This should be moved to test-lib.sh together with the
-# copy in t0021 after both topics have graduated to 'master'.
-file_size () {
-	test-tool path-utils file-size "$1"
-}
-
 import_large () {
 	(
 		echo blob
@@ -24,7 +18,7 @@ do
 		test_when_finished "rm -f .git/objects/pack/pack-*.*" &&
 		test_when_finished "rm -rf .git/objects/??" &&
 		import_large -c fastimport.unpacklimit=0 $config &&
-		sz=$(file_size .git/objects/pack/pack-*.pack) &&
+		sz=$(test_file_size .git/objects/pack/pack-*.pack) &&
 		case "$expect" in
 		small) test "$sz" -le 100000 ;;
 		large) test "$sz" -ge 100000 ;;
@@ -47,7 +41,7 @@ do
 		test_when_finished "rm -f .git/objects/pack/pack-*.*" &&
 		test_when_finished "rm -rf .git/objects/??" &&
 		import_large -c fastimport.unpacklimit=9 $config &&
-		sz=$(file_size .git/objects/??/????*) &&
+		sz=$(test_file_size .git/objects/??/????*) &&
 		case "$expect" in
 		small) test "$sz" -le 100000 ;;
 		large) test "$sz" -ge 100000 ;;
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4a35bde145..59bbf75e83 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -783,6 +783,10 @@ test_line_count () {
 	fi
 }
 
+test_file_size () {
+	test-tool path-utils file-size "$1"
+}
+
 # Returns success if a comma separated string of keywords ($1) contains a
 # given keyword ($2).
 # Examples:

base-commit: 7f7ebe054af6d831b999d6c2241b9227c4e4e08d
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] tests: consolidate the `file_size` function into `test-lib-functions.sh`
  2020-11-07  1:12 [PATCH] tests: consolidate the `file_size` function into `test-lib-functions.sh` Johannes Schindelin via GitGitGadget
@ 2020-11-07  1:23 ` brian m. carlson
  0 siblings, 0 replies; 2+ messages in thread
From: brian m. carlson @ 2020-11-07  1:23 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

On 2020-11-07 at 01:12:57, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> In 8de7eeb54b6 (compression: unify pack.compression configuration
> parsing, 2016-11-15), we introduced identical copies of the `file_size`
> helper into three test scripts, with the plan to eventually consolidate
> them into a single copy.
> 
> Let's do that, and adjust the function name to adhere to the `test_*`
> naming convention.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This seems reasonable.  For a moment, I was going to ask why we didn't
just use test -s, but then I remembered that test -s doesn't have that
behavior, Perl's -s does, and you replaced that with the helper for
efficiency on Windows.

I looked at the patch and it seemed pretty straightforward.

>     tests: consolidate the file_size function into test-lib-functions.sh
> 
>     My ulterior motive with this patch is not even to address that old
>     concern, but to avoid having to exclude the code comments from the
>     upcoming master -> main rename (because those code comments talk about
>     git/git's main branch, which is called master).

Also a good reason.  Why modify code when you can just delete it?
-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-11-07  1:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-07  1:12 [PATCH] tests: consolidate the `file_size` function into `test-lib-functions.sh` Johannes Schindelin via GitGitGadget
2020-11-07  1:23 ` brian m. carlson

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).