Re: [PATCH v2 02/11] cache-tree tests: use a sub-shell with less indirection
From: Jeff King <hidden>
Date: 2021-01-17 16:56:24
On Sat, Jan 16, 2021 at 04:35:45PM +0100, Ævar Arnfjörð Bjarmason wrote:
quoted hunk ↗ jump to hunk
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh index 354b7f15f7..2e3efeb80e 100755 --- a/t/t0090-cache-tree.sh +++ b/t/t0090-cache-tree.sh@@ -27,20 +27,15 @@ 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_rec "$dir$subtree" || return 1 + ) done
We don't check that "cd" worked either before or after your patch. Should we? After your patch, we "return" from inside a subshell. Is that portable? ISTR issues around that before, but it just have been when we are not in a function at all. Still, I wonder if: for ... do ( cd "$subtree" && generate_expected_cache_tree_rec "$dir$subtree" ) || return 1 done might be more obvious.
-generate_expected_cache_tree () {
- (
- generate_expected_cache_tree_rec
- )
-}I wondered what the "rec" was for, but I guess it is "recurse". Not a problem to keep it, but I wonder if it could be dropped in the name of shortness/simplicity (not worth a re-roll for sure, but maybe worth doing so if you re-roll for the above issues). -Peff