On 2019.07.30 09:13, Martin Ågren wrote: > On Tue, 30 Jul 2019 at 01:43, Josh Steadmon <stead...@google.com> wrote: > > > > On 2019.07.29 22:04, Martin Ågren wrote: > > > This script seems to me like if it passes 100%, we can be fairly sure > > > we're ok, but [...] > > > Will squash these as you said in V3. Will also think about whether > > another test approach would make more sense here. > > Thinking a bit more about this, this test uses two identical hooks, runs > some commands and verifies that "the" hook was run (or not, with > --no-verify). If the implementation started calling the wrong hook > (pre-commit / pre-merge) or both hooks, we wouldn't notice. > > Please forgive my braindump below, I'm on the run so I'm just throwing > this out there: > > Perhaps (first do some modernizing of this script, to protect various > setup steps, use "write_script", etc, then) make the existing hook a > tiny bit pre-commit-specific, e.g., by doing something like "echo > pre-commit >>executed-hooks", then at select places check "test_cmp > executed-hooks pre-commit" (against "echo pre-commit >pre-commit"), > "test_path_is_missing executed-hooks", and so on, coupled with some > "test_when_finished 'rm -f executed_hooks'". Then the tests added for > this series would use a very similar hook, appending and checking for > "pre-merge[-commit]", That should make us fairly certain that we're > running precisely the wanted hook, I think. > > Martin
That sounds like a reasonable approach, thank you for the suggestions. I will work on this for V3.
