Re: [PATCH 09/12] t4150: am with post-applypatch hook
On Sun, Jul 5, 2015 at 11:58 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: On 2015-07-02 20:16, Paul Tan wrote: diff --git a/t/t4150-am.sh b/t/t4150-am.sh index dd6fe81..62b678c 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -275,6 +275,48 @@ test_expect_success 'am with failing pre-applypatch hook' ' test_cmp_rev first HEAD ' +test_expect_success 'am with post-applypatch hook' ' + test_when_finished rm -f .git/hooks/post-applypatch + rm -fr .git/rebase-apply + git reset --hard + git checkout first + mkdir -p .git/hooks + cat .git/hooks/post-applypatch -\EOF + #!/bin/sh + git rev-parse HEAD head.actual + git diff second diff.actual + exit 0 + EOF + chmod +x .git/hooks/post-applypatch + git am patch1 + test_path_is_missing .git/rebase-apply + test_cmp_rev second HEAD + git rev-parse second head.expected + test_cmp head.expected head.actual + git diff second diff.expected + test_cmp diff.expected diff.actual +' + +test_expect_success 'am with failing post-applypatch hook' ' + test_when_finished rm -f .git/hooks/post-applypatch + rm -fr .git/rebase-apply + git reset --hard + git checkout first + mkdir -p .git/hooks + cat .git/hooks/post-applypatch -\EOF + #!/bin/sh + git rev-parse HEAD head.actual + exit 1 + EOF + chmod +x .git/hooks/post-applypatch + git am patch1 + test_path_is_missing .git/rebase-apply + git diff --exit-code second + test_cmp_rev second HEAD + git rev-parse second head.expected + test_cmp head.expected head.actual +' These 2 tests as well as the previous patches look to me as if they could be refactored (the paradigm is the same: add a certain hook after resetting and then apply the patch, verify that the hook ran/failed)... do you think there is a chance for that? I had a look, but I think that while it is true that the overall sequence of each test is the same, the details differ enough that there's no obvious way to refactor the tests sensibly. For example, the contents of the hook scripts are not the same, as we need to check that the hooks are run at the correct stage of git-am execution. And as such, the verification tests are also different as well. Junio did correctly point out though that we can shave off 2 lines if the write_script helper is used (the shebang and the chmod). Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/12] t4150: am with post-applypatch hook
Hi Paul, On 2015-07-07 08:47, Paul Tan wrote: On Sun, Jul 5, 2015 at 11:58 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: On 2015-07-02 20:16, Paul Tan wrote: diff --git a/t/t4150-am.sh b/t/t4150-am.sh index dd6fe81..62b678c 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -275,6 +275,48 @@ test_expect_success 'am with failing pre-applypatch hook' ' test_cmp_rev first HEAD ' +test_expect_success 'am with post-applypatch hook' ' + test_when_finished rm -f .git/hooks/post-applypatch + rm -fr .git/rebase-apply + git reset --hard + git checkout first + mkdir -p .git/hooks + cat .git/hooks/post-applypatch -\EOF + #!/bin/sh + git rev-parse HEAD head.actual + git diff second diff.actual + exit 0 + EOF + chmod +x .git/hooks/post-applypatch + git am patch1 + test_path_is_missing .git/rebase-apply + test_cmp_rev second HEAD + git rev-parse second head.expected + test_cmp head.expected head.actual + git diff second diff.expected + test_cmp diff.expected diff.actual +' + +test_expect_success 'am with failing post-applypatch hook' ' + test_when_finished rm -f .git/hooks/post-applypatch + rm -fr .git/rebase-apply + git reset --hard + git checkout first + mkdir -p .git/hooks + cat .git/hooks/post-applypatch -\EOF + #!/bin/sh + git rev-parse HEAD head.actual + exit 1 + EOF + chmod +x .git/hooks/post-applypatch + git am patch1 + test_path_is_missing .git/rebase-apply + git diff --exit-code second + test_cmp_rev second HEAD + git rev-parse second head.expected + test_cmp head.expected head.actual +' These 2 tests as well as the previous patches look to me as if they could be refactored (the paradigm is the same: add a certain hook after resetting and then apply the patch, verify that the hook ran/failed)... do you think there is a chance for that? I had a look, but I think that while it is true that the overall sequence of each test is the same, the details differ enough that there's no obvious way to refactor the tests sensibly. For example, the contents of the hook scripts are not the same, as we need to check that the hooks are run at the correct stage of git-am execution. And as such, the verification tests are also different as well. Yeah, makes sense. Thanks for the clarification! Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/12] t4150: am with post-applypatch hook
Hi Paul, On 2015-07-02 20:16, Paul Tan wrote: diff --git a/t/t4150-am.sh b/t/t4150-am.sh index dd6fe81..62b678c 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -275,6 +275,48 @@ test_expect_success 'am with failing pre-applypatch hook' ' test_cmp_rev first HEAD ' +test_expect_success 'am with post-applypatch hook' ' + test_when_finished rm -f .git/hooks/post-applypatch + rm -fr .git/rebase-apply + git reset --hard + git checkout first + mkdir -p .git/hooks + cat .git/hooks/post-applypatch -\EOF + #!/bin/sh + git rev-parse HEAD head.actual + git diff second diff.actual + exit 0 + EOF + chmod +x .git/hooks/post-applypatch + git am patch1 + test_path_is_missing .git/rebase-apply + test_cmp_rev second HEAD + git rev-parse second head.expected + test_cmp head.expected head.actual + git diff second diff.expected + test_cmp diff.expected diff.actual +' + +test_expect_success 'am with failing post-applypatch hook' ' + test_when_finished rm -f .git/hooks/post-applypatch + rm -fr .git/rebase-apply + git reset --hard + git checkout first + mkdir -p .git/hooks + cat .git/hooks/post-applypatch -\EOF + #!/bin/sh + git rev-parse HEAD head.actual + exit 1 + EOF + chmod +x .git/hooks/post-applypatch + git am patch1 + test_path_is_missing .git/rebase-apply + git diff --exit-code second + test_cmp_rev second HEAD + git rev-parse second head.expected + test_cmp head.expected head.actual +' These 2 tests as well as the previous patches look to me as if they could be refactored (the paradigm is the same: add a certain hook after resetting and then apply the patch, verify that the hook ran/failed)... do you think there is a chance for that? Just to make sure: I think the patch series looks good, and if it would be too cumbersome (or just not feasible) to simplify those test cases, I am totally fine with them as-are. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
