{"thread":{"id":"54594","subject":"[PATCH] tests: consolidate the `file_size` function into `test-lib-functions.sh`","startedAt":"2020-11-07T01:13:02Z","lastAt":"2020-11-07T01:23:55Z","messageCount":2,"participants":["Johannes Schindelin via GitGitGadget","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"409312","messageId":"pull.780.git.1604711577662.gitgitgadget@gmail.com","threadId":"54594","inReplyTo":null,"subject":"[PATCH] tests: consolidate the `file_size` function into `test-lib-functions.sh`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-07T01:12:57Z","receivedAt":"2020-11-07T01:13:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn 8de7eeb54b6 (compression: unify pack.compression configuration\nparsing, 2016-11-15), we introduced identical copies of the `file_size`\nhelper into three test scripts, with the plan to eventually consolidate\nthem into a single copy.\n\nLet's do that, and adjust the function name to adhere to the `test_*`\nnaming convention.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    tests: consolidate the file_size function into test-lib-functions.sh\n    \n    My ulterior motive with this patch is not even to address that old\n    concern, but to avoid having to exclude the code comments from the\n    upcoming master -> main rename (because those code comments talk about\n    git/git's main branch, which is called master).\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-780%2Fdscho%2Ffile_size-test-function-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-780/dscho/file_size-test-function-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/780\n\n t/t0021-conversion.sh               | 36 +++++++++++++----------------\n t/t1050-large.sh                    |  8 +------\n t/t5315-pack-objects-compression.sh |  8 +------\n t/t9303-fast-import-compression.sh  | 10 ++------\n t/test-lib-functions.sh             |  4 ++++\n 5 files changed, 24 insertions(+), 42 deletions(-)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 4bfffa9c31..f6deaf498b 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -23,10 +23,6 @@ generate_random_characters () {\n \t\tperl -pe \"s/./chr((ord($&) % 26) + ord('a'))/sge\" >\"$TEST_ROOT/$NAME\"\n }\n \n-file_size () {\n-\ttest-tool path-utils file-size \"$1\"\n-}\n-\n filter_git () {\n \trm -f *.log &&\n \tgit \"$@\"\n@@ -361,9 +357,9 @@ test_expect_success PERL 'required process filter should filter data' '\n \t\tcp \"$TEST_ROOT/test3 '\\''sq'\\'',\\$x=.o\" \"testsubdir/test3 '\\''sq'\\'',\\$x=.r\" &&\n \t\t>test4-empty.r &&\n \n-\t\tS=$(file_size test.r) &&\n-\t\tS2=$(file_size test2.r) &&\n-\t\tS3=$(file_size \"testsubdir/test3 '\\''sq'\\'',\\$x=.r\") &&\n+\t\tS=$(test_file_size test.r) &&\n+\t\tS2=$(test_file_size test2.r) &&\n+\t\tS3=$(test_file_size \"testsubdir/test3 '\\''sq'\\'',\\$x=.r\") &&\n \t\tM=$(git hash-object test.r) &&\n \t\tM2=$(git hash-object test2.r) &&\n \t\tM3=$(git hash-object \"testsubdir/test3 '\\''sq'\\'',\\$x=.r\") &&\n@@ -432,9 +428,9 @@ test_expect_success PERL 'required process filter should filter data for various\n \t(\n \t\tcd repo &&\n \n-\t\tS=$(file_size test.r) &&\n-\t\tS2=$(file_size test2.r) &&\n-\t\tS3=$(file_size \"testsubdir/test3 '\\''sq'\\'',\\$x=.r\") &&\n+\t\tS=$(test_file_size test.r) &&\n+\t\tS2=$(test_file_size test2.r) &&\n+\t\tS3=$(test_file_size \"testsubdir/test3 '\\''sq'\\'',\\$x=.r\") &&\n \t\tM=$(git hash-object test.r) &&\n \t\tM2=$(git hash-object test2.r) &&\n \t\tM3=$(git hash-object \"testsubdir/test3 '\\''sq'\\'',\\$x=.r\") &&\n@@ -549,7 +545,7 @@ test_expect_success PERL 'required process filter takes precedence' '\n \n \t\techo \"*.r filter=protocol\" >.gitattributes &&\n \t\tcp \"$TEST_ROOT/test.o\" test.r &&\n-\t\tS=$(file_size test.r) &&\n+\t\tS=$(test_file_size test.r) &&\n \n \t\t# Check that the process filter is invoked here\n \t\tfilter_git add . &&\n@@ -573,7 +569,7 @@ test_expect_success PERL 'required process filter should be used only for \"clean\n \n \t\techo \"*.r filter=protocol\" >.gitattributes &&\n \t\tcp \"$TEST_ROOT/test.o\" test.r &&\n-\t\tS=$(file_size test.r) &&\n+\t\tS=$(test_file_size test.r) &&\n \n \t\tfilter_git add . &&\n \t\tcat >expected.log <<-EOF &&\n@@ -697,9 +693,9 @@ test_expect_success PERL 'process filter should restart after unexpected write f\n \t\techo \"this is going to fail\" >smudge-write-fail.o &&\n \t\tcp smudge-write-fail.o smudge-write-fail.r &&\n \n-\t\tS=$(file_size test.r) &&\n-\t\tS2=$(file_size test2.r) &&\n-\t\tSF=$(file_size smudge-write-fail.r) &&\n+\t\tS=$(test_file_size test.r) &&\n+\t\tS2=$(test_file_size test2.r) &&\n+\t\tSF=$(test_file_size smudge-write-fail.r) &&\n \t\tM=$(git hash-object test.r) &&\n \t\tM2=$(git hash-object test2.r) &&\n \t\tMF=$(git hash-object smudge-write-fail.r) &&\n@@ -752,9 +748,9 @@ test_expect_success PERL 'process filter should not be restarted if it signals a\n \t\techo \"this will cause an error\" >error.o &&\n \t\tcp error.o error.r &&\n \n-\t\tS=$(file_size test.r) &&\n-\t\tS2=$(file_size test2.r) &&\n-\t\tSE=$(file_size error.r) &&\n+\t\tS=$(test_file_size test.r) &&\n+\t\tS2=$(test_file_size test2.r) &&\n+\t\tSE=$(test_file_size error.r) &&\n \t\tM=$(git hash-object test.r) &&\n \t\tM2=$(git hash-object test2.r) &&\n \t\tME=$(git hash-object error.r) &&\n@@ -797,7 +793,7 @@ test_expect_success PERL 'process filter abort stops processing of all further f\n \n \t\tM=\"blob=$(git hash-object abort.r)\" &&\n \t\trm -f debug.log &&\n-\t\tSA=$(file_size abort.r) &&\n+\t\tSA=$(test_file_size abort.r) &&\n \n \t\tgit add . &&\n \t\trm -f *.r &&\n@@ -859,7 +855,7 @@ test_expect_success PERL 'delayed checkout in process filter' '\n \t\tgit commit -m \"test commit\"\n \t) &&\n \n-\tS=$(file_size \"$TEST_ROOT/test.o\") &&\n+\tS=$(test_file_size \"$TEST_ROOT/test.o\") &&\n \tPM=\"ref=refs/heads/master treeish=$(git -C repo rev-parse --verify master) \" &&\n \tM=\"${PM}blob=$(git -C repo rev-parse --verify master:test.a)\" &&\n \tcat >a.exp <<-EOF &&\ndiff --git a/t/t1050-large.sh b/t/t1050-large.sh\nindex 61e89a8071..4bab6a513c 100755\n--- a/t/t1050-large.sh\n+++ b/t/t1050-large.sh\n@@ -5,12 +5,6 @@ test_description='adding and checking out large blobs'\n \n . ./test-lib.sh\n \n-# This should be moved to test-lib.sh together with the\n-# copy in t0021 after both topics have graduated to 'master'.\n-file_size () {\n-\ttest-tool path-utils file-size \"$1\"\n-}\n-\n test_expect_success setup '\n \t# clone does not allow us to pass core.bigfilethreshold to\n \t# new repos, so set core.bigfilethreshold globally\n@@ -29,7 +23,7 @@ do\n \ttest_expect_success \"add with $config\" '\n \t\ttest_when_finished \"rm -f .git/objects/pack/pack-*.* .git/index\" &&\n \t\tgit $config add large1 &&\n-\t\tsz=$(file_size .git/objects/pack/pack-*.pack) &&\n+\t\tsz=$(test_file_size .git/objects/pack/pack-*.pack) &&\n \t\tcase \"$expect\" in\n \t\tsmall) test \"$sz\" -le 100000 ;;\n \t\tlarge) test \"$sz\" -ge 100000 ;;\ndiff --git a/t/t5315-pack-objects-compression.sh b/t/t5315-pack-objects-compression.sh\nindex df970d7584..8bacd96275 100755\n--- a/t/t5315-pack-objects-compression.sh\n+++ b/t/t5315-pack-objects-compression.sh\n@@ -4,12 +4,6 @@ test_description='pack-object compression configuration'\n \n . ./test-lib.sh\n \n-# This should be moved to test-lib.sh together with the\n-# copy in t0021 after both topics have graduated to 'master'.\n-file_size () {\n-\ttest-tool path-utils file-size \"$1\"\n-}\n-\n test_expect_success setup '\n \tprintf \"%2000000s\" X |\n \tgit hash-object -w --stdin >object-name &&\n@@ -24,7 +18,7 @@ do\n \ttest_expect_success \"pack-objects with $config\" '\n \t\ttest_when_finished \"rm -f pack-*.*\" &&\n \t\tgit $config pack-objects pack <object-name &&\n-\t\tsz=$(file_size pack-*.pack) &&\n+\t\tsz=$(test_file_size pack-*.pack) &&\n \t\tcase \"$expect\" in\n \t\tsmall) test \"$sz\" -le 100000 ;;\n \t\tlarge) test \"$sz\" -ge 100000 ;;\ndiff --git a/t/t9303-fast-import-compression.sh b/t/t9303-fast-import-compression.sh\nindex 5045f02a53..57d916524e 100755\n--- a/t/t9303-fast-import-compression.sh\n+++ b/t/t9303-fast-import-compression.sh\n@@ -3,12 +3,6 @@\n test_description='compression setting of fast-import utility'\n . ./test-lib.sh\n \n-# This should be moved to test-lib.sh together with the\n-# copy in t0021 after both topics have graduated to 'master'.\n-file_size () {\n-\ttest-tool path-utils file-size \"$1\"\n-}\n-\n import_large () {\n \t(\n \t\techo blob\n@@ -24,7 +18,7 @@ do\n \t\ttest_when_finished \"rm -f .git/objects/pack/pack-*.*\" &&\n \t\ttest_when_finished \"rm -rf .git/objects/??\" &&\n \t\timport_large -c fastimport.unpacklimit=0 $config &&\n-\t\tsz=$(file_size .git/objects/pack/pack-*.pack) &&\n+\t\tsz=$(test_file_size .git/objects/pack/pack-*.pack) &&\n \t\tcase \"$expect\" in\n \t\tsmall) test \"$sz\" -le 100000 ;;\n \t\tlarge) test \"$sz\" -ge 100000 ;;\n@@ -47,7 +41,7 @@ do\n \t\ttest_when_finished \"rm -f .git/objects/pack/pack-*.*\" &&\n \t\ttest_when_finished \"rm -rf .git/objects/??\" &&\n \t\timport_large -c fastimport.unpacklimit=9 $config &&\n-\t\tsz=$(file_size .git/objects/??/????*) &&\n+\t\tsz=$(test_file_size .git/objects/??/????*) &&\n \t\tcase \"$expect\" in\n \t\tsmall) test \"$sz\" -le 100000 ;;\n \t\tlarge) test \"$sz\" -ge 100000 ;;\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 4a35bde145..59bbf75e83 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -783,6 +783,10 @@ test_line_count () {\n \tfi\n }\n \n+test_file_size () {\n+\ttest-tool path-utils file-size \"$1\"\n+}\n+\n # Returns success if a comma separated string of keywords ($1) contains a\n # given keyword ($2).\n # Examples:\n\nbase-commit: 7f7ebe054af6d831b999d6c2241b9227c4e4e08d\n-- \ngitgitgadget\n"},{"id":"409314","messageId":"20201107012345.GD6252@camp.crustytoothpaste.net","threadId":"54594","inReplyTo":"pull.780.git.1604711577662.gitgitgadget@gmail.com","subject":"Re: [PATCH] tests: consolidate the `file_size` function into `test-lib-functions.sh`","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-11-07T01:23:45Z","receivedAt":"2020-11-07T01:23:55Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-11-07 at 01:12:57, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> \n> In 8de7eeb54b6 (compression: unify pack.compression configuration\n> parsing, 2016-11-15), we introduced identical copies of the `file_size`\n> helper into three test scripts, with the plan to eventually consolidate\n> them into a single copy.\n> \n> Let's do that, and adjust the function name to adhere to the `test_*`\n> naming convention.\n> \n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis seems reasonable.  For a moment, I was going to ask why we didn't\njust use test -s, but then I remembered that test -s doesn't have that\nbehavior, Perl's -s does, and you replaced that with the helper for\nefficiency on Windows.\n\nI looked at the patch and it seemed pretty straightforward.\n\n>     tests: consolidate the file_size function into test-lib-functions.sh\n> \n>     My ulterior motive with this patch is not even to address that old\n>     concern, but to avoid having to exclude the code comments from the\n>     upcoming master -> main rename (because those code comments talk about\n>     git/git's main branch, which is called master).\n\nAlso a good reason.  Why modify code when you can just delete it?\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"}]}