Re: [PATCH v2 4/6] tests: use `git submodule add` and fix expected status
- From
Glen Choo <chooglen@google.com>
- Date
- Mar 7, 2023, 00:15 UTC
- Message-ID
- <kl6lpm9lifhz.fsf@chooglen-macbookpro.roam.corp.google.com>
- In-Reply-To
- <20230228185642.2357806-4-calvinwan@google.com>
Calvin Wan <calvinwan@google.com> writes:
Show 38 quoted lines
> @@ -122,25 +123,30 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)' > ' > > test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.gitmodules]' ' > + git branch pristine-gitmodules && > git config diff.ignoreSubmodules dirty && > git diff HEAD >actual && > test_must_be_empty actual && > git config --add -f .gitmodules submodule.subname.ignore none && > git config --add -f .gitmodules submodule.subname.path sub && > + git commit -m "Update .gitmodules" .gitmodules && > git diff HEAD >actual && > sed -e "1,/^@@/d" actual >actual.body && > expect_from_to >expect.body $subprev $subprev-dirty && > test_cmp expect.body actual.body && > git config -f .gitmodules submodule.subname.ignore all && > git config -f .gitmodules submodule.subname.path sub && > + git commit -m "Update .gitmodules" .gitmodules && > git diff HEAD >actual2 && > test_must_be_empty actual2 && > git config -f .gitmodules submodule.subname.ignore untracked && > + git commit -m "Update .gitmodules" .gitmodules && > git diff HEAD >actual3 && > sed -e "1,/^@@/d" actual3 >actual3.body && > expect_from_to >expect.body $subprev $subprev-dirty && > test_cmp expect.body actual3.body && > git config -f .gitmodules submodule.subname.ignore dirty && > + git commit -m "Update .gitmodules" .gitmodules && > git diff HEAD >actual4 && > test_must_be_empty actual4 && > git config submodule.subname.ignore none && > @@ -152,7 +158,7 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) > git config --remove-section submodule.subname && > git config --remove-section -f .gitmodules submodule.subname && > git config --unset diff.ignoreSubmodules && > - rm .gitmodules > + git reset --hard pristine-gitmodules > '
This looks like the perfect use case for test_when_finished :)
Show 16 quoted lines
> @@ -190,12 +196,15 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)' > test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) [.gitmodules]' ' > git config --add -f .gitmodules submodule.subname.ignore all && > git config --add -f .gitmodules submodule.subname.path sub && > + git commit -m "Update .gitmodules" .gitmodules && > git diff HEAD >actual2 && > test_must_be_empty actual2 && > git config -f .gitmodules submodule.subname.ignore untracked && > + git commit -m "Update .gitmodules" .gitmodules && > git diff HEAD >actual3 && > test_must_be_empty actual3 && > git config -f .gitmodules submodule.subname.ignore dirty && > + git commit -m "Update .gitmodules" .gitmodules && > git diff HEAD >actual4 && > test_must_be_empty actual4 && > git config submodule.subname.ignore none &&
Like the previous patch, I wonder a little whether we should be diffing with :!.gitmodules, but at least here we are focused on diffing with submodules in general (and not the specific "git diff --submodule=" behavior), so I thnk this is okay to keep.
Show 6 quoted lines
> @@ -206,7 +215,7 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) > test_cmp expect.body actual.body && > git config --remove-section submodule.subname && > git config --remove-section -f .gitmodules submodule.subname && > - rm .gitmodules > + git reset --hard pristine-gitmodules
Ditto about test_when_finished.
Show 11 quoted lines
> ' > > test_expect_success 'git diff between submodule commits' ' > @@ -243,7 +252,7 @@ test_expect_success 'git diff between submodule commits [.gitmodules]' ' > expect_from_to >expect.body $subtip $subprev && > git config --remove-section submodule.subname && > git config --remove-section -f .gitmodules submodule.subname && > - rm .gitmodules > + git reset --hard pristine-gitmodules > ' >
Ditto
Show 38 quoted lines
> @@ -1152,8 +1156,37 @@ test_expect_success '.gitmodules ignore=untracked suppresses submodules with unt > test_cmp expect output && > git config --add -f .gitmodules submodule.subname.ignore untracked && > git config --add -f .gitmodules submodule.subname.path sm && > + cat > expect-modified-gitmodules << EOF && > +On branch main > +Your branch and '\''upstream'\'' have diverged, > +and have 2 and 2 different commits each, respectively. > + (use "git pull" to merge the remote branch into yours) > + > +Changes to be committed: > + (use "git restore --staged <file>..." to unstage) > + modified: sm > + > +Changes not staged for commit: > + (use "git add <file>..." to update what will be committed) > + (use "git restore <file>..." to discard changes in working directory) > + modified: .gitmodules > + modified: dir1/modified > + > +Submodule changes to be committed: > + > +* sm $head...$new_head (1): > + > Add bar > + > +Untracked files: > + (use "git add <file>..." to include in what will be committed) > + dir1/untracked > + dir2/modified > + dir2/untracked > + untracked > + > +EOF > git status >output && > - test_cmp expect output && > + test_cmp expect-modified-gitmodules output && > git config -f .gitmodules --remove-section submodule.subname > '
That another giant snapshot makes me a bit wary, since it's harder to tell whether the "modifed .gitmodules" and "unmodified .gitmodules" are really checking the same things, but there might not be a way around it. The following tests check various combinations of values (dirty, untracked, etc) and sources "--ignore-submodules", ".git/config ignore=" and ".gitmodules ignore=". For the .gitmodules tests, we really do have to modify .gitmodules to test that it gives us the behavior we want.
As a hack, we could preemptively modify .gitmodules, so that it's modified in all of the snapshots we're diffing. That feels too hacky to me, but maybe others think it's fine.
(Side note: I recall a previous conversation with Junio about how we shouldn't be changing behavior based on .gitmodules. If we had that, we wouldn't need to worry about this right now.)