Re: [PATCH v2 1/8] show, log: provide a --remerge-diff capability
- From
Elijah Newren <newren@gmail.com>
- Date
- Dec 28, 2021, 23:45 UTC
- Message-ID
- <CABPp-BHtnFWLpPxXF56BbMSayDOp9Qm1GhL3xCz7pQPcEJZ2Nw@mail.gmail.com>
- In-Reply-To
- <YcuXQQHsl8iVdK8R@camp.crustytoothpaste.net>
On Tue, Dec 28, 2021 at 3:01 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:
Show 66 quoted lines
>
> On 2021-12-28 at 22:34:03, Elijah Newren wrote:
> > CC'ing brian in case he has comments on the sha256 stuff and whether
> > he thinks there's a cleaner way to make my tests work with sha256.
> > (brian: See the very end of the email.)
> >
> > On Tue, Dec 28, 2021 at 2:56 AM Johannes Altmanninger <aclopte@gmail.com> wrote:
> > >
> > > On Sat, Dec 25, 2021 at 07:59:12AM +0000, Elijah Newren via GitGitGadget wrote:
> > > > +test_expect_success 'remerge-diff with both a resolved conflict and an unrelated change' '
> > > > + git log -1 --oneline ab_resolution >tmp &&
> > > > + cat <<-EOF >>tmp &&
> > > > + diff --git a/numbers b/numbers
> > > > + index a1fb731..6875544 100644
> > > > + --- a/numbers
> > > > + +++ b/numbers
> > > > + @@ -1,13 +1,9 @@
> > > > + 1
> > > > + 2
> > > > + -<<<<<<< b0ed5cb (change_a)
> > > > + -three
> > > > + -=======
> > > > + -tres
> > > > + ->>>>>>> 6cd3f82 (change_b)
> > > > + +drei
> > >
> > > nice
> > >
> > > > + 4
> > > > + 5
> > > > + 6
> > > > + 7
> > > > + -eight
> > > > + +acht
> > > > + 9
> > > > + EOF
> > > > + # Hashes above are sha1; rip them out so test works with sha256
> > > > + sed -e "s/[0-9a-f]\{7,\}/HASH/g" tmp >expect &&
> > >
> > > Right, sha256 could cause many noisy test changes. I wonder if there is a
> > > more general way to avoid this; maybe default to SHA1 for existing tests?
> >
> > Not "could", but "does". And this is not something to be avoided.
> > The default testsuite we run in CI involves a run of
> > GIT_TEST_DEFAULT_HASH=sha256 under linux-clang. Making these tests
> > SHA1-only just reduces our coverage and makes the transition to SHA256
> > harder; I think that's the opposite of the direction we want to go.
> >
> > These changes I've made here are sufficient to make these tests work
> > under sha256; you can see the test results here:
> > https://github.com/gitgitgadget/git/runs/4646949283?check_suite_focus=true.
> > Under "Run ci/run-build-and-tests.sh" note that there are two runs of
> > tests, and the second has "export GIT_TEST_DEFAULT_HASH=sha256"
> > preceding it.
> >
> > There might be a cleaner way to make these tests sha256-compatible,
> > but this seemed like a pretty simple way to me.
>
> The question here is, do we care very much about testing these specific
> hashes? If so, then we should use test_oid_cache to set up some OIDs
> and make sure they're correct for both SHA-1 and SHA-256, and then
> replace them in the code with calls to test_oid.
>
> However, my impression is that we probably don't care very much about
> what the specific values are, and in that case, this is completely fine.
> We do similar things elsewhere in the testsuite.Thanks for chiming in. Your impression is right; I don't care about the specific hashes, just the general form of the diff content, so I'll keep it as is.