Re: [PATCH v2 1/8] show, log: provide a --remerge-diff capability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Dec 28, 2021 at 3:01 PM brian m. carlson
<sandals@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxx> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux