Re: [PATCH v2 4/4] t7503: add tests for pre-merge-hook

[Josh Steadmon <steadmon@google.com>]

On 2019.07.30 09:13, Martin Ågren wrote:
> On Tue, 30 Jul 2019 at 01:43, Josh Steadmon <steadmon@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.