Subject: Re: [PATCH 4/5] t6022, t6046: test expected behavior instead of testing a proxy for it

From: SZEDER Gábor <[email-masked]>
Date: Fri, 13 Mar 2020 18:30:16 +0100

On Fri, Mar 13, 2020 at 10:18:29AM -0700, Elijah Newren wrote:
> On Fri, Mar 13, 2020 at 10:12 AM SZEDER Gábor <[email-masked]> wrote:
> >
> > On Thu, Feb 27, 2020 at 12:14:23AM +0000, Elijah Newren via GitGitGadget wrote:
> > > diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
> > > index 6f196aaf276..d97cf48495b 100755
> > > --- a/t/t6022-merge-rename.sh
> > > +++ b/t/t6022-merge-rename.sh
> > > @@ -242,12 +242,23 @@ test_expect_success 'merge of identical changes in a renamed file' '
> > >       rm -f A M N &&
> > >       git reset --hard &&
> > >       git checkout change+rename &&
> > > +
> > > +     test-tool chmtime =31337 B &&
> > > +     test-tool chmtime --get B >old-mtime &&
> >
> > Here you compare the mtime of 'B' before ...
> >
> > >       GIT_MERGE_VERBOSITY=3 git merge change >out &&
> > > -     test_i18ngrep "^Skipped B" out &&
> > > +
> > > +     test-tool chmtime --get B >new-mtime &&
> > > +     test_cmp old-mtime new-mtime &&
> >
> > ... and after the merge.
> >
> > > +
> > >       git reset --hard HEAD^ &&
> > >       git checkout change &&
> > > +
> > > +     test-tool chmtime =-1 M &&
> > > +     test-tool chmtime --get M >old-mtime &&
> >
> > Here, however, you compare the mtime of 'M' before the merge ...
> >
> > >       GIT_MERGE_VERBOSITY=3 git merge change+rename >out &&
> > > -     test_i18ngrep ! "^Skipped B" out
> > > +
> > > +     test-tool chmtime --get B >new-mtime &&
> > > +     test $(cat old-mtime) -lt $(cat new-mtime)
> >
> > ... with the mtime of 'B' after the merge, i.e. the mtimes of two
> > different files.
> >
> > Is that right?
> 
> No, sounds like a copy-and-paste-and-forget-to-update bug.  Thanks for
> spotting it.  Since you found it, do you want to submit a patch with
> the fix?

No :)  There is no file 'B' in branch 'change', so I think the fix
requires your expertise to make sure that the test checks what it was
supposed to check.