[3/4] t2405: clarify test descriptions and simplify test
diff mbox series

Message ID 6e0e664026757315a337dead8079167f4fb29c4b.1579263809.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • [1/4] t7410: rename to t2405-worktree-submodule.sh
Related show

Commit Message

Han-Wen Nienhuys via GitGitGadget Jan. 17, 2020, 12:23 p.m. UTC
From: Philippe Blain <levraiphilippeblain@gmail.com>

When 'checkout --to' functionality was moved to 'worktree add', tests were adapted
in f194b1ef6e (tests: worktree: retrofit "checkout --to" tests for "worktree add",
2015-07-06).

The calls were changed to 'worktree add' in this test (then t7410), but the test
descriptions were not updated, keeping 'checkout' instead of using the new
terminology (linked worktrees).

Clarify the tests by using the right terminology. While at it, remove some unnecessary
leading directories such that all superproject worktrees are directly next to one
another in the trash directory.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 t/t2405-worktree-submodule.sh | 36 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

Comments

Eric Sunshine Jan. 17, 2020, 1:56 p.m. UTC | #1
On Fri, Jan 17, 2020 at 7:25 AM Philippe Blain via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> When 'checkout --to' functionality was moved to 'worktree add', tests were adapted
> in f194b1ef6e (tests: worktree: retrofit "checkout --to" tests for "worktree add",
> 2015-07-06).
>
> The calls were changed to 'worktree add' in this test (then t7410), but the test
> descriptions were not updated, keeping 'checkout' instead of using the new
> terminology (linked worktrees).
>
> Clarify the tests by using the right terminology. While at it, remove some unnecessary
> leading directories such that all superproject worktrees are directly next to one
> another in the trash directory.

The unanswered questions which popped into my head when reading the
"While at it..." include:

    Why is it desirable for the worktrees to live in this new location
    rather than their original locations?

    Is it safe to relocate the worktrees like this without losing some
    important aspect of the test?

    Why were the worktrees located as they were originally? Was there
    some hidden or not-so-obvious reason that we're overlooking? (I
    guess this is really the same as question #2.)
Philippe Blain Jan. 19, 2020, 12:21 a.m. UTC | #2
Hi Eric,
> Le 17 janv. 2020 à 08:56, Eric Sunshine <sunshine@sunshineco.com> a écrit :
> 
>> While at it, remove some unnecessary
>> leading directories such that all superproject worktrees are directly next to one
>> another in the trash directory.
> 
> The unanswered questions which popped into my head when reading the
> "While at it..." include:
> 
>    Why is it desirable for the worktrees to live in this new location
>    rather than their original locations?
I thought it was desirable because the leading directories don’t serve any purpose apart from carrying information about what they are testing, which the worktree directory itself can do instead of all of them being called "main". 
> 
>    Is it safe to relocate the worktrees like this without losing some
>    important aspect of the test?
After analyzing the test to see what was being tested, making the change and making sure the test behaved the same way, I concluded that it was.
> 
>    Why were the worktrees located as they were originally? Was there
>    some hidden or not-so-obvious reason that we're overlooking? (I
>    guess this is really the same as question #2.)
I don’t think there was a reason. The worktrees directories were structured that way since the test was introduced in df56607dff (git-common-dir: make "modules/" per-working-directory directory, 2014-11-30). Maybe one reason was for every superproject worktree to be in a directory called "main", just as every submodule is in a directory called "sub"…

I can explain more this reasoning in the commit message if necessary.

Philippe.
Eric Sunshine Jan. 19, 2020, 1:41 a.m. UTC | #3
On Sat, Jan 18, 2020 at 7:21 PM Philippe Blain
<levraiphilippeblain@gmail.com> wrote:
> > Le 17 janv. 2020 à 08:56, Eric Sunshine <sunshine@sunshineco.com> a écrit :
> > The unanswered questions which popped into my head when reading the
> > "While at it..." include:
> > [...]
> I can explain more this reasoning in the commit message if necessary.

Yes, please.

Patch
diff mbox series

diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh
index b9040c30d4..f1952c70dd 100755
--- a/t/t2405-worktree-submodule.sh
+++ b/t/t2405-worktree-submodule.sh
@@ -18,43 +18,39 @@  test_expect_success 'setup: create origin repos'  '
 	git -C origin/main commit -m "sub updated"
 '
 
-test_expect_success 'setup: clone' '
-	mkdir clone &&
-	git -C clone clone --recursive "$base_path/origin/main"
+test_expect_success 'setup: clone superproject to create main worktree' '
+	git clone --recursive "$base_path/origin/main" main
 '
 
 rev1_hash_main=$(git --git-dir=origin/main/.git show --pretty=format:%h -q "HEAD~1")
 rev1_hash_sub=$(git --git-dir=origin/sub/.git show --pretty=format:%h -q "HEAD~1")
 
-test_expect_success 'checkout main' '
-	mkdir default_checkout &&
-	git -C clone/main worktree add "$base_path/default_checkout/main" "$rev1_hash_main"
+test_expect_success 'add superproject worktree' '
+	git -C main worktree add "$base_path/worktree" "$rev1_hash_main"
 '
 
-test_expect_failure 'can see submodule diffs just after checkout' '
-	git -C default_checkout/main diff --submodule master"^!" >out &&
+test_expect_failure 'submodule is checked out just after worktree add' '
+	git -C worktree diff --submodule master"^!" >out &&
 	grep "file1-updated" out
 '
 
-test_expect_success 'checkout main and initialize independent clones' '
-	mkdir fully_cloned_submodule &&
-	git -C clone/main worktree add "$base_path/fully_cloned_submodule/main" "$rev1_hash_main" &&
-	git -C fully_cloned_submodule/main submodule update
+test_expect_success 'add superproject worktree and initialize submodules' '
+	git -C main worktree add "$base_path/worktree-submodule-update" "$rev1_hash_main" &&
+	git -C worktree-submodule-update submodule update
 '
 
-test_expect_success 'can see submodule diffs after independent cloning' '
-	git -C fully_cloned_submodule/main diff --submodule master"^!" >out &&
+test_expect_success 'submodule is checked out just after submodule update in linked worktree' '
+	git -C worktree-submodule-update diff --submodule master"^!" >out &&
 	grep "file1-updated" out
 '
 
-test_expect_success 'checkout sub manually' '
-	mkdir linked_submodule &&
-	git -C clone/main worktree add "$base_path/linked_submodule/main" "$rev1_hash_main" &&
-	git -C clone/main/sub worktree add "$base_path/linked_submodule/main/sub" "$rev1_hash_sub"
+test_expect_success 'add superproject worktree and manually add submodule worktree' '
+	git -C main worktree add "$base_path/linked_submodule" "$rev1_hash_main" &&
+	git -C main/sub worktree add "$base_path/linked_submodule/sub" "$rev1_hash_sub"
 '
 
-test_expect_success 'can see submodule diffs after manual checkout of linked submodule' '
-	git -C linked_submodule/main diff --submodule master"^!" >out &&
+test_expect_success 'submodule is checked out after manually adding submodule worktree' '
+	git -C linked_submodule diff --submodule master"^!" >out &&
 	grep "file1-updated" out
 '