Skip to contentSkip to conversation

Fix test bug with spaces in file owner

PATCH In review Derrick Stolee via GitGitGadget 3 participants 4 messages Jul 1, 2019 – Jul 2, 2019

lore
Share & tools
lore
#0 lore raw

cover Fix test bug with spaces in file owner

Thanks to Johannes Sixt for reporting this bug and Dscho for presenting the correct test-tool to use.

This applies on ds/midx-expire-repack. If we instead want to squash this into that branch, the two hunks will need to be squashed into these commits:

THIRD_SMALLEST_SIZE applies to ce1e4a105b4 (midx: implement midx_repack(), 2019-06-10).

MINSIZE applies to 2af890bb280 (multi-pack-index: prepare 'repack' subcommand, 2019-06-10).

Thanks, -Stolee

Derrick Stolee (1):
t5319: use 'test-tool path-utils' instead of 'ls -l'

 t/t5319-multi-pack-index.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

base-commit: b526d8cbbb8740faa10411caa757c6586395fcab
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-280%2Fderrickstolee%2Fmidx-test-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-280/derrickstolee/midx-test-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/280

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/1 t5319: use 'test-tool path-utils' instead of 'ls -l'

From: Derrick Stolee <[email protected]>

Using 'ls -l' and parsing the columns to find file sizes is problematic when the platform could report the owner as a name with spaces. Instead, use the 'test-tool path-utils file-size' command to list only the sizes.

---
 t/t5319-multi-pack-index.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
t/t5319-multi-pack-index.sh +2 -2
index 79bfaeafa9..c72ca04399 100755
@@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '
 		touch -m -t 201901010002 .git/objects/pack/pack-B* &&
 		touch -m -t 201901010003 .git/objects/pack/pack-A* &&
 		ls .git/objects/pack >expect &&
-		MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) &&
+		MINSIZE=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 1) &&
 		git multi-pack-index repack --batch-size=$MINSIZE &&
 		ls .git/objects/pack >actual &&
 		test_cmp expect actual
@@ -455,7 +455,7 @@ test_expect_success 'repack creates a new pack' '
 		cd dup &&
 		ls .git/objects/pack/*idx >idx-list &&
 		test_line_count = 5 idx-list &&
-		THIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 3 | tail -n 1) &&
+		THIRD_SMALLEST_SIZE=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 3 | tail -n 1) &&
 		BATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) &&
 		git multi-pack-index repack --batch-size=$BATCH_SIZE &&
 		ls .git/objects/pack/*idx >idx-list &&
Signature · 1 line
gitgitgadget
#2 lore raw

Re: [PATCH 1/1] t5319: use 'test-tool path-utils' instead of 'ls -l'

Hi Stolee,

Quoted 6 lines · Derrick Stolee via GitGitGadget

From: Derrick Stolee <[email protected]>

Using 'ls -l' and parsing the columns to find file sizes is problematic when the platform could report the owner as a name with spaces. Instead, use the 'test-tool path-utils file-size' command to list only the sizes.

Thank you!

t/t5319-multi-pack-index.sh test_expect_success 'repack with minimum size does not alter existing packs' ' · line 446

Quoted 10 lines
t/t5319-multi-pack-index.sh +1 -1
index 79bfaeafa9..c72ca04399 100755
@@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' '
 		touch -m -t 201901010002 .git/objects/pack/pack-B* &&
 		touch -m -t 201901010003 .git/objects/pack/pack-A* &&
 		ls .git/objects/pack >expect &&
-		MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) &&
+		MINSIZE=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 1) &&

You know, we could also add a --show-minimum option...

t/t5319-multi-pack-index.sh test_expect_success 'repack creates a new pack' ' · line 458

Quoted 6 lines
t/t5319-multi-pack-index.sh +1 -1
@@ -455,7 +455,7 @@ test_expect_success 'repack creates a new pack' '
 		cd dup &&
 		ls .git/objects/pack/*idx >idx-list &&
 		test_line_count = 5 idx-list &&
-		THIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 3 | tail -n 1) &&
+		THIRD_SMALLEST_SIZE=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 3 | tail -n 1) &&

I guess the --show-minimum option is no longer such a smart idea ;-)

Patch looks good, thanks for working on this,
Dscho

Quoted 6 lines
t/t5319-multi-pack-index.sh +0 -1
 		BATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) &&
 		git multi-pack-index repack --batch-size=$BATCH_SIZE &&
 		ls .git/objects/pack/*idx >idx-list &&
--

gitgitgadget

#3 lore raw

Re: [PATCH 1/1] t5319: use 'test-tool path-utils' instead of 'ls -l'

Quoted 4 lines · Johannes Schindelin

Hi Stolee,

On Mon, 1 Jul 2019, Derrick Stolee via GitGitGadget wrote:

Quoted 8 lines
Quoted 6 lines

From: Derrick Stolee <[email protected]>

Using 'ls -l' and parsing the columns to find file sizes is problematic when the platform could report the owner as a name with spaces. Instead, use the 'test-tool path-utils file-size' command to list only the sizes.

Thank you!

Yup. Sounds good.