On 2019.07.29 22:04, Martin Ågren wrote: > On Fri, 19 Jul 2019 at 00:57, Josh Steadmon <steadmon@xxxxxxxxxx> wrote: > > +test_expect_success '--no-verify with succeeding hook (merge)' ' > > + > > + git checkout side && > > + git merge --no-verify -m "merge master" master && > > + git checkout master > > + > > +' > > This test doesn't even try to verify that the hook was actually ignored. > That left me puzzled for a while... > > > +test_expect_success '--no-verify with failing hook (merge)' ' > > + > > + git checkout side && > > + git merge --no-verify -m "merge master" master && > > + git checkout master > > + > > +' > > ... but this would then (most likely) fail, so we would notice > something's wrong. > > This script seems to me like if it passes 100%, we can be fairly sure > we're ok, but if some individual test fails, we shouldn't be surprised > if its oneline description is a bit off compared to the bug. Similarly, > quite a few tests could pass, despite their oneline description inducing > the thought of "but surely, if /that/ were the problem, /those/ tests > would fail". > > Anyway, I realize this is just following the existing approach. I guess > you could argue it has served us well for a long time. > > I would probably prefer seeing the various hunks in this patch being > squashed into the relevant commits (1/4 vs 3/4) to make those patches > more self-describing. Will squash these as you said in V3. Will also think about whether another test approach would make more sense here.
