diff mbox series

[v3,03/10] cache-tree tests: use a sub-shell with less indirection

Message ID 20210123130046.21975-4-avarab@gmail.com (mailing list archive)
State Accepted
Commit fa6edee7765028daa8c69051b404c8374d0c2a04
Headers show
Series [v3,01/10] cache-tree tests: refactor for modern test style | expand

Commit Message

Ævar Arnfjörð Bjarmason Jan. 23, 2021, 1 p.m. UTC
Change a "cd xyz && work && cd .." pattern introduced in
9c4d6c0297 (cache-tree: Write updated cache-tree after commit,
2014-07-13) to use a sub-shell instead with less indirection.

We did actually recover correctly if we failed in this function since
we were wrapped in a subshell one function call up. Let's just use the
sub-shell at the point where we want to change the directory
instead.

It's important that the "|| return 1" is outside the
subshell. Normally, we `exit 1` from within subshells[1], but that
wouldn't help us exit this loop early[1][2].

Since we can get rid of the wrapper function let's rename the main
function to drop the "rec" (for "recursion") suffix[3].

1. https://lore.kernel.org/git/CAPig+cToj8nQmyBCqC1k7DXF2vXaonCEA-fCJ4x7JBZG2ixYBw@mail.gmail.com/
2. https://lore.kernel.org/git/20150325052952.GE31924@peff.net/
3. https://lore.kernel.org/git/YARsCsgXuiXr4uFX@coredump.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0090-cache-tree.sh | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

Comments

Junio C Hamano Jan. 23, 2021, 9:35 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Change a "cd xyz && work && cd .." pattern introduced in
> 9c4d6c0297 (cache-tree: Write updated cache-tree after commit,
> 2014-07-13) to use a sub-shell instead with less indirection.

Much nicer.  Pleased.
diff mbox series

Patch

diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 7ff7f04719..5bb4f75443 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -17,7 +17,7 @@  cmp_cache_tree () {
 # We don't bother with actually checking the SHA1:
 # test-tool dump-cache-tree already verifies that all existing data is
 # correct.
-generate_expected_cache_tree_rec () {
+generate_expected_cache_tree () {
 	dir="$1${1:+/}" &&
 	# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
 	# We want to count only foo because it's the only direct child
@@ -28,18 +28,13 @@  generate_expected_cache_tree_rec () {
 	printf "SHA $dir (%d entries, %d subtrees)\n" "$entries" "$subtree_count" &&
 	for subtree in $subtrees
 	do
-		cd "$subtree"
-		generate_expected_cache_tree_rec "$dir$subtree" || return 1
-		cd ..
+		(
+			cd "$subtree" &&
+			generate_expected_cache_tree "$dir$subtree"
+		) || return 1
 	done
 }
 
-generate_expected_cache_tree () {
-	(
-		generate_expected_cache_tree_rec
-	)
-}
-
 test_cache_tree () {
 	generate_expected_cache_tree >expect &&
 	cmp_cache_tree expect