| Message ID | 5060ce3d64419369b2912c395a880fb49a0a3137.1579263809.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] t7410: rename to t2405-worktree-submodule.sh | expand |
On Fri, Jan 17, 2020 at 7:24 AM Philippe Blain via GitGitGadget <gitgitgadget@gmail.com> wrote: > The subshells used in the setup phase of this test are unnecessary. > Remove them by using 'git -C' and 'test_commit -C'. The subshells may not be necessary, but the code feels cleaner before this patch is applied since all the added "-C foo/bar" noise hurts readability. So, I'm "meh" on this patch and wouldn't complain if it was dropped (though I don't insist upon it). > Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> > --- > diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh > @@ -6,32 +6,16 @@ test_description='Combination of submodules and multiple worktrees' > - git commit -m "file1 updated" > + git -C origin/main commit -m "add sub" && > + test_commit -C origin/sub "file1-updated" file1 file1updated && > @@ -49,7 +33,7 @@ test_expect_success 'checkout main' ' > - grep "file1 updated" out > + grep "file1-updated" out Why this change? Is it because test_commit() mishandles the whitespace in the commit message? If so, it might deserve mention in the commit message of this patch. (Even better would be to fix test_commit(), if that is the case.)
Eric Sunshine <sunshine@sunshineco.com> writes: > On Fri, Jan 17, 2020 at 7:24 AM Philippe Blain via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> The subshells used in the setup phase of this test are unnecessary. >> Remove them by using 'git -C' and 'test_commit -C'. > > The subshells may not be necessary, but the code feels cleaner before > this patch is applied since all the added "-C foo/bar" noise hurts > readability. So, I'm "meh" on this patch and wouldn't complain if it > was dropped (though I don't insist upon it). I dunno. Each of these subshells did not do much after going into its subdirectory, so repetition of "-C foo/bar" did not bother me that much. >> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> >> --- >> diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh >> @@ -6,32 +6,16 @@ test_description='Combination of submodules and multiple worktrees' >> - git commit -m "file1 updated" >> + git -C origin/main commit -m "add sub" && >> + test_commit -C origin/sub "file1-updated" file1 file1updated && >> @@ -49,7 +33,7 @@ test_expect_success 'checkout main' ' >> - grep "file1 updated" out >> + grep "file1-updated" out > > Why this change? Is it because test_commit() mishandles the whitespace > in the commit message? If so, it might deserve mention in the commit > message of this patch. (Even better would be to fix test_commit(), if > that is the case.) FWIW I had the same reaction on that dash in "file1 updated".
Hi Eric, >> - grep "file1 updated" out >> + grep "file1-updated" out > > Why this change? Is it because test_commit() mishandles the whitespace > in the commit message? If so, it might deserve mention in the commit > message of this patch. (Even better would be to fix test_commit(), if > that is the case.) The only reason is that I didn’t notice that test_commit accepts a <tag> argument, which defaults to <message> if unset. So when I changed the test to use test_commit and ran it I got errors from ‘git tag’ saying "file1 updated" is not a valid tag name, so I added a dash. I’ll correct that in v2. Thanks, Philippe.
diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh index f2eee328cc..b9040c30d4 100755 --- a/t/t2405-worktree-submodule.sh +++ b/t/t2405-worktree-submodule.sh @@ -6,32 +6,16 @@ test_description='Combination of submodules and multiple worktrees' base_path=$(pwd -P) -test_expect_success 'setup: make origin' ' - mkdir -p origin/sub && - ( - cd origin/sub && git init && - echo file1 >file1 && - git add file1 && - git commit -m file1 - ) && - mkdir -p origin/main && - ( - cd origin/main && git init && - git submodule add ../sub && - git commit -m "add sub" - ) && - ( - cd origin/sub && - echo file1updated >file1 && - git add file1 && - git commit -m "file1 updated" - ) && +test_expect_success 'setup: create origin repos' ' + git init origin/sub && + test_commit -C origin/sub file1 && + git init origin/main && + git -C origin/main submodule add ../sub && + git -C origin/main commit -m "add sub" && + test_commit -C origin/sub "file1-updated" file1 file1updated && git -C origin/main/sub pull && - ( - cd origin/main && - git add sub && - git commit -m "sub updated" - ) + git -C origin/main add sub && + git -C origin/main commit -m "sub updated" ' test_expect_success 'setup: clone' ' @@ -49,7 +33,7 @@ test_expect_success 'checkout main' ' test_expect_failure 'can see submodule diffs just after checkout' ' git -C default_checkout/main diff --submodule master"^!" >out && - grep "file1 updated" out + grep "file1-updated" out ' test_expect_success 'checkout main and initialize independent clones' ' @@ -60,7 +44,7 @@ test_expect_success 'checkout main and initialize independent clones' ' test_expect_success 'can see submodule diffs after independent cloning' ' git -C fully_cloned_submodule/main diff --submodule master"^!" >out && - grep "file1 updated" out + grep "file1-updated" out ' test_expect_success 'checkout sub manually' ' @@ -71,7 +55,7 @@ test_expect_success 'checkout sub manually' ' test_expect_success 'can see submodule diffs after manual checkout of linked submodule' ' git -C linked_submodule/main diff --submodule master"^!" >out && - grep "file1 updated" out + grep "file1-updated" out ' test_done