{"thread":{"id":"27713","subject":"[RFC/PATCH] test: skip clean-up when running under --immediate mode","startedAt":"2011-06-27T18:02:22Z","lastAt":"2011-07-01T10:49:26Z","messageCount":4,"participants":["Junio C Hamano","Jeff King","Michael J Gruber","Sverre Rabbelier"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"170581","messageId":"7vmxh3xidt.fsf@alter.siamese.dyndns.org","threadId":"27713","inReplyTo":null,"subject":"[RFC/PATCH] test: skip clean-up when running under --immediate mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-27T18:02:22Z","receivedAt":"2011-06-27T18:02:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Some tests try to be too careful about cleaning themselves up and\ndo\n\n    test_expect_success description '\n        set-up some test refs and/or configuration &&\n        test_when_finished \"revert the above changes\" &&\n\tthe real test\n    '\n\nWhich is nice to make sure that a potential failure would not have\nunexpected interaction with the next test. This however interferes when\n\"the real test\" fails and we want to see what is going on, by running the\ntest with --immediate mode and descending into its trash directory after\nthe test stops. The precondition to run the real test and cause it to fail\nis all gone after the clean-up procedure defined by test_when_finished is\ndone.\n\nUpdate test_run_ which is the workhorse of running a test script\ncalled from test_expect_success and test_expect_failure, so that we do not\nrun clean-up script defined with test_when_finished when a test that is\nexpected to succeed fails under the --immediate mode.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * Likes, dislikes?\n\n t/test-lib.sh |   12 ++++++++++--\n 1 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8c57a00..e36e67a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -446,9 +446,14 @@ test_debug () {\n \n test_run_ () {\n \ttest_cleanup=:\n+\texpecting_failure=$1\n \teval >&3 2>&4 \"$1\"\n \teval_ret=$?\n-\teval >&3 2>&4 \"$test_cleanup\"\n+\n+\tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n+\tthen\n+\t\teval >&3 2>&4 \"$test_cleanup\"\n+\tfi\n \tif test \"$verbose\" = \"t\" && test -n \"$HARNESS_ACTIVE\"; then\n \t\techo \"\"\n \tfi\n@@ -497,7 +502,7 @@ test_expect_failure () {\n \tif ! test_skip \"$@\"\n \tthen\n \t\tsay >&3 \"checking known breakage: $2\"\n-\t\ttest_run_ \"$2\"\n+\t\ttest_run_ \"$2\" expecting_failure\n \t\tif [ \"$?\" = 0 -a \"$eval_ret\" = 0 ]\n \t\tthen\n \t\t\ttest_known_broken_ok_ \"$1\"\n@@ -774,6 +779,9 @@ test_cmp() {\n #\n # except that the greeting and config --unset must both succeed for\n # the test to pass.\n+#\n+# Note that under --immediate mode, no clean-up is done to help diagnose\n+# what went wrong.\n \n test_when_finished () {\n \ttest_cleanup=\"{ $*\n-- \n1.7.6\n"},{"id":"170655","messageId":"20110628225147.GD4192@sigill.intra.peff.net","threadId":"27713","inReplyTo":"7vmxh3xidt.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] test: skip clean-up when running under --immediate mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-28T22:51:47Z","receivedAt":"2011-06-28T22:51:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 27, 2011 at 11:02:22AM -0700, Junio C Hamano wrote:\n\n> Some tests try to be too careful about cleaning themselves up and\n> do\n> \n>     test_expect_success description '\n>         set-up some test refs and/or configuration &&\n>         test_when_finished \"revert the above changes\" &&\n> \tthe real test\n>     '\n> \n> Which is nice to make sure that a potential failure would not have\n> unexpected interaction with the next test. This however interferes when\n> \"the real test\" fails and we want to see what is going on, by running the\n> test with --immediate mode and descending into its trash directory after\n> the test stops. The precondition to run the real test and cause it to fail\n> is all gone after the clean-up procedure defined by test_when_finished is\n> done.\n> \n> Update test_run_ which is the workhorse of running a test script\n> called from test_expect_success and test_expect_failure, so that we do not\n> run clean-up script defined with test_when_finished when a test that is\n> expected to succeed fails under the --immediate mode.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n>  * Likes, dislikes?\n\nThis sounds to me like the right thing to do. The current behavior has\nnever been a problem for me, but that is probably because\ntest_when_finished is relatively new. If I were to run into this\nsituation, your patch does exactly what I would expect.\n\n-Peff\n"},{"id":"170699","messageId":"4E0C1B8E.5070507@drmicha.warpmail.net","threadId":"27713","inReplyTo":"7vmxh3xidt.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] test: skip clean-up when running under --immediate mode","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-06-30T06:45:34Z","receivedAt":"2011-06-30T06:45:34Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 27.06.2011 20:02:\n> Some tests try to be too careful about cleaning themselves up and\n> do\n> \n>     test_expect_success description '\n>         set-up some test refs and/or configuration &&\n>         test_when_finished \"revert the above changes\" &&\n> \tthe real test\n>     '\n> \n> Which is nice to make sure that a potential failure would not have\n> unexpected interaction with the next test. This however interferes when\n> \"the real test\" fails and we want to see what is going on, by running the\n> test with --immediate mode and descending into its trash directory after\n> the test stops. The precondition to run the real test and cause it to fail\n> is all gone after the clean-up procedure defined by test_when_finished is\n> done.\n> \n> Update test_run_ which is the workhorse of running a test script\n> called from test_expect_success and test_expect_failure, so that we do not\n> run clean-up script defined with test_when_finished when a test that is\n> expected to succeed fails under the --immediate mode.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n>  * Likes, dislikes?\n\nLikes!\n\n> \n>  t/test-lib.sh |   12 ++++++++++--\n>  1 files changed, 10 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 8c57a00..e36e67a 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -446,9 +446,14 @@ test_debug () {\n>  \n>  test_run_ () {\n>  \ttest_cleanup=:\n> +\texpecting_failure=$1\n>  \teval >&3 2>&4 \"$1\"\n>  \teval_ret=$?\n> -\teval >&3 2>&4 \"$test_cleanup\"\n> +\n> +\tif test -z \"$immediate\" || test $eval_ret = 0 || test -n \"$expecting_failure\"\n\nOne may argue that a \"test_expect_failure\" which succeeds is a fail and\nshould stop with --immediate (so that it's easier to spot fixed bugs),\nbut...\n\n> +\tthen\n> +\t\teval >&3 2>&4 \"$test_cleanup\"\n> +\tfi\n>  \tif test \"$verbose\" = \"t\" && test -n \"$HARNESS_ACTIVE\"; then\n>  \t\techo \"\"\n>  \tfi\n> @@ -497,7 +502,7 @@ test_expect_failure () {\n>  \tif ! test_skip \"$@\"\n>  \tthen\n>  \t\tsay >&3 \"checking known breakage: $2\"\n> -\t\ttest_run_ \"$2\"\n> +\t\ttest_run_ \"$2\" expecting_failure\n>  \t\tif [ \"$?\" = 0 -a \"$eval_ret\" = 0 ]\n>  \t\tthen\n>  \t\t\ttest_known_broken_ok_ \"$1\"\n\n...we would have to change that here also, and it would be a change in\nbehaviour of the test suite. Anyway, that is orthogonal to this patch\nwhich I do like.\n\n> @@ -774,6 +779,9 @@ test_cmp() {\n>  #\n>  # except that the greeting and config --unset must both succeed for\n>  # the test to pass.\n> +#\n> +# Note that under --immediate mode, no clean-up is done to help diagnose\n> +# what went wrong.\n>  \n>  test_when_finished () {\n>  \ttest_cleanup=\"{ $*\n"},{"id":"170740","messageId":"BANLkTimzFDquxjsZHz9OfatR=oe+R3xzTg@mail.gmail.com","threadId":"27713","inReplyTo":"7vmxh3xidt.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] test: skip clean-up when running under --immediate mode","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-07-01T10:49:26Z","receivedAt":"2011-07-01T10:49:26Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jun 27, 2011 at 20:02, Junio C Hamano <gitster@pobox.com> wrote:\n>  * Likes, dislikes?\n\nMakes a lot of sense, the 'test_when_finished'-ed code will get\nexecuted when the test doesn't fail anyway, so I don't think there\nneed to be a way to turn this behavior off. (I.e., the patch is good\nas-is).\n\n-- \nCheers,\n\nSverre Rabbelier\n"}]}