Re: [PATCH 3/5] t5318: avoid top-level directory changes

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Jul 21, 2023 at 1:33 PM Taylor Blau <me@ttaylorr.com> wrote:
> Avoid changing the current working directory from outside of a sub-shell
> during the tests in t5318.
>
> Each test has mostly straightforward changes, either:
>
>   - Removing the top-level `cd "$TRASH_DIRECTORY/full"`, which is
>     unnecessary after ensuring that other tests don't change their
>     working directory outside of a sub-shell.
>
>   - Changing any Git invocations which want to be in a sub-directory by
>     either (a) adding a "-C $DIR" argument, or (b) moving the whole test
>     into a sub-shell.
>
> While we're here, remove any explicit "git config core.commitGraph true"
> invocations which were designed to enable use of the commit-graph. These
> are unnecessary following 31b1de6a09b (commit-graph: turn on
> commit-graph by default, 2019-08-13).
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> @@ -69,117 +64,106 @@ test_expect_success 'create commits and repack' '
>  test_expect_success 'Add more commits' '
>  ...
> -       git reset --hard commits/2 &&
> -       git merge commits/4 &&
> -       git branch merge/1 &&
> -       git reset --hard commits/4 &&
> -       git merge commits/6 &&
> -       git branch merge/2 &&
> -       git reset --hard commits/3 &&
> -       git merge commits/5 commits/7 &&
> -       git branch merge/3 &&
> -       git repack
> +       git -C full reset --hard commits/2 &&
> +       git -C full merge commits/4 &&
> +       git -C full branch merge/1 &&
> +       git -C full reset --hard commits/4 &&
> +       git -C full merge commits/6 &&
> +       git -C full branch merge/2 &&
> +       git -C full reset --hard commits/3 &&
> +       git -C full merge commits/5 commits/7 &&
> +       git -C full branch merge/3 &&
> +       git -C full repack
>  '

This is one of those cases in which "cd full" while in subshell,
rather than using "-C full" repeatedly, would perhaps make the code a
bit less noisy, but it is, of course, subjective and what you have
here is good enough.

> @@ -229,114 +211,101 @@ graph_git_behavior 'mixed mode, commit 8 vs merge 1' full commits/8 merge/1
>  test_expect_success 'build graph from latest pack with closure' '
> -       cd "$TRASH_DIRECTORY/full" &&
> -       cat new-idx | git commit-graph write --stdin-packs &&
> -       test_path_is_file $objdir/info/commit-graph &&
> -       graph_read_expect "9" "generation_data extra_edges"
> +       git -C full commit-graph write --stdin-packs <new-idx &&
> +       test_path_is_file full/$objdir/info/commit-graph &&
> +       graph_read_expect -C full 9 "generation_data extra_edges"
>  '

This works because an earlier test created "new-idx", and it created
it directly in the trash-directory rather than in the "full"
subdirectory where it used to reside before this patch. Okay.

> @@ -495,25 +464,24 @@ GRAPH_BYTE_OCTOPUS=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4))
>  corrupt_graph_setup() {
> -       cd "$TRASH_DIRECTORY/full" &&
> -       test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
> -       cp $objdir/info/commit-graph commit-graph-backup &&
> -       chmod u+w $objdir/info/commit-graph
> +       test_when_finished mv commit-graph-backup full/$objdir/info/commit-graph &&
> +       cp full/$objdir/info/commit-graph commit-graph-backup &&
> +       chmod u+w full/$objdir/info/commit-graph
>  }

Prior to this patch, "commit-graph-backup" was placed in the "full"
subdirectory and test_when_finished() removed it from that location.
As of this patch, "commit-graph-backup" is placed in the top-level
trash-directory and cleaned up from there. Okay.