Re: [PATCH 2/2] t1020-subdirectory.sh: check hook pwd, $GIT_PREFIX
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Jan 10, 2015, 08:25 UTC
- Message-ID
- <54B0E1EE.2020301@kdbg.org>
- In-Reply-To
- <1420872598-9609-3-git-send-email-rhansen@bbn.com>
Am 10.01.2015 um 07:49 schrieb Richard Hansen:
Show 22 quoted lines
> Make sure hooks are executed at the top-level directory and that > GIT_PREFIX is set (as documented). > > Signed-off-by: Richard Hansen <rhansen@bbn.com> > --- > t/t1020-subdirectory.sh | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh > index 2edb4f2..03bb0a2 100755 > --- a/t/t1020-subdirectory.sh > +++ b/t/t1020-subdirectory.sh > @@ -128,6 +128,23 @@ test_expect_success !MINGW '!alias expansion' ' > test_cmp expect actual > ' > > +test_expect_success 'hook pwd' ' > + pwd >expect && > + ( > + rm -f actual && > + mkdir -p .git/hooks && > + ! test -e .git/hooks/post-checkout &&
What is the purpose of this test?
Show 5 quoted lines
> + cat <<-\EOF >.git/hooks/post-checkout && > + #!/bin/sh > + pwd >actual > + EOF > + chmod +x .git/hooks/post-checkout &&
Use write_script() to construct a shell script.
> + (cd dir && git checkout -- two) && > + rm -f .git/hooks/post-checkout
This cleanup would be skipped if the checkout fails for some reason. Use test_when_finished.
> + ) &&
The outer sub-shell us unnecessary, isn't it?
> + test_cmp expect actual
If 'git checkout' runs the hook from the wrong directory, there would not exist a file 'actual' at this point because it was rm -f'd earlier, and the test would fail. Perhaps it would make sense to document this failure case by inserting
test_path_is_file actual &&
before the test_cmp?
Which makes me think: Would the test for existence of 'actual' be sufficient? Then the test_cmp could be omitted. The advantage is that we do not depend on how the `pwd` is formatted: With or without symbolic links in any leading path or c:/foo vs. /c/foo on Windows. (I anticipate that the test as written fails on Windows because 'expect' is in c:/foo form and 'actual' is in /c/foo form.)
Show 23 quoted lines
> +' > + > test_expect_success 'GIT_PREFIX for !alias' ' > printf "dir/" >expect && > ( > @@ -154,6 +171,23 @@ test_expect_success 'GIT_PREFIX for built-ins' ' > test_cmp expect actual > ' > > +test_expect_success 'GIT_PREFIX for hooks' ' > + printf "dir/" >expect && > + ( > + rm -f actual && > + mkdir -p .git/hooks && > + ! test -e .git/hooks/post-checkout && > + cat <<-\EOF >.git/hooks/post-checkout && > + #!/bin/sh > + printf %s "$GIT_PREFIX" >actual > + EOF > + chmod +x .git/hooks/post-checkout && > + (cd dir && git checkout -- two) && > + rm -f .git/hooks/post-checkout > + ) &&
The comments about the sub-shell, write_script, and clean-up apply here, too.
Show 7 quoted lines
> + test_cmp expect actual > +' > + > test_expect_success 'no file/rev ambiguity check inside .git' ' > git commit -a -m 1 && > ( >
-- Hannes