Calvin Wan <calvinwan@xxxxxxxxxx> writes: > @@ -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 :) > @@ -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. > @@ -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. > ' > > 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 > @@ -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.)
