On Fri, 19 Jul 2019 at 00:57, Josh Steadmon <stead...@google.com> 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. Martin
