{"thread":{"id":"24922","subject":"[PATCH 1/2] maint-reflog-beyond-horizon: fix broken test_must_fail calls","startedAt":"2010-08-31T14:49:18Z","lastAt":"2010-09-01T10:55:54Z","messageCount":17,"participants":["Jon Seymour","Jeff King","Junio C Hamano","Jonathan Nieder","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"149419","messageId":"1283266160-11665-1-git-send-email-jon.seymour@gmail.com","threadId":"24922","inReplyTo":null,"subject":"[PATCH 0/2] fix quotes in maint-reflog-beyond-horizon and detached-stash","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2010-08-31T14:49:18Z","receivedAt":"2010-08-31T14:49:18Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"In two recent series, I introduced tests that use test_must_fail incorrectly.\n\nIn effect, I was calling:\n\n test_must_fail \"foo bar\"\n\nbut this was failing because \"foo bar\" is not comamnd and not because\nthe command \"foo\" fails with arguments \"bar\".\n\nThese two patches fix these problems.\n\nIf necessary, I can re-roll the original series. Otherwise, these patches\ncan be squashed with the test-oriented commits currently in next.\n\nJon Seymour (2):\n maint-reflog-beyond-horizon: fix broken test_must_fail calls\n detached-stash: fix broken test_must_fail calls\n\n t/t1503-rev-parse-verify.sh | 4 ++--\n t/t3903-stash.sh | 36 ++++++++++++++++++------------------\n 2 files changed, 20 insertions(+), 20 deletions(-)\n\n-- \n1.7.2.80.g22196.dirty\n"},{"id":"149418","messageId":"1283266160-11665-2-git-send-email-jon.seymour@gmail.com","threadId":"24922","inReplyTo":"1283266160-11665-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH 1/2] maint-reflog-beyond-horizon: fix broken test_must_fail calls","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2010-08-31T14:49:19Z","receivedAt":"2010-08-31T14:49:19Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"Some tests in maint-reflog-beyond-horizon are calling test_must_fail\nin such a way that the arguments to test_must_fail do, indeed, fail\nbut not in the manner expected by the test.\n\nThis patch removes the unnecessary and unhelpful double quotes.\n\nSigned-off-by: Jon Seymour \n---\n t/t1503-rev-parse-verify.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex 61092f7..100f857 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -111,8 +111,8 @@ test_expect_success 'master@{n} for various n' '\n \tgit rev-parse --verify master@{0} &&\n \tgit rev-parse --verify master@{1} &&\n \tgit rev-parse --verify master@{$Nm1} &&\n-\ttest_must_fail \"git rev-parse --verify master@{$N}\" &&\n-\ttest_must_fail \"git rev-parse --verify master@{$Np1}\"\n+\ttest_must_fail git rev-parse --verify master@{$N} &&\n+\ttest_must_fail git rev-parse --verify master@{$Np1}\n '\n \n test_done\n-- \n1.7.2.80.g22196.dirty\n"},{"id":"149420","messageId":"1283266160-11665-3-git-send-email-jon.seymour@gmail.com","threadId":"24922","inReplyTo":"1283266160-11665-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH 2/2] detached-stash: fix broken test_must_fail calls","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2010-08-31T14:49:20Z","receivedAt":"2010-08-31T14:49:20Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"Some tests in detached-stash are calling test_must_fail\nin such a way that the arguments to test_must_fail do, indeed, fail\nbut not in the manner expected by the test.\n\nThis patch removes the unnecessary and unhelpful double quotes.\n\nSigned-off-by: Jon Seymour \n---\n t/t3903-stash.sh | 36 ++++++++++++++++++------------------\n 1 files changed, 18 insertions(+), 18 deletions(-)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex ea9f979..d99f27a 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -435,7 +435,7 @@ test_expect_success 'stash drop - fail early if specified stash is not a stash r\n \tgit stash &&\n \techo bar > file &&\n \tgit stash &&\n-\ttest_must_fail \"git stash drop $(git rev-parse stash@{0})\" &&\n+\ttest_must_fail git stash drop $(git rev-parse stash@{0}) &&\n \tgit stash pop &&\n \ttest bar = \"$(cat file)\" &&\n \tgit reset --hard HEAD\n@@ -449,7 +449,7 @@ test_expect_success 'stash pop - fail early if specified stash is not a stash re\n \tgit stash &&\n \techo bar > file &&\n \tgit stash &&\n-\ttest_must_fail \"git stash pop $(git rev-parse stash@{0})\" &&\n+\ttest_must_fail git stash pop $(git rev-parse stash@{0}) &&\n \tgit stash pop &&\n \ttest bar = \"$(cat file)\" &&\n \tgit reset --hard HEAD\n@@ -462,31 +462,31 @@ test_expect_success 'ref with non-existant reflog' '\n \tgit add file2 &&\n \tgit stash &&\n \t! \"git rev-parse --quiet --verify does-not-exist\" &&\n-\ttest_must_fail \"git stash drop does-not-exist\" &&\n-\ttest_must_fail \"git stash drop does-not-exist@{0}\" &&\n-\ttest_must_fail \"git stash pop does-not-exist\" &&\n-\ttest_must_fail \"git stash pop does-not-exist@{0}\" &&\n-\ttest_must_fail \"git stash apply does-not-exist\" &&\n-\ttest_must_fail \"git stash apply does-not-exist@{0}\" &&\n-\ttest_must_fail \"git stash show does-not-exist\" &&\n-\ttest_must_fail \"git stash show does-not-exist@{0}\" &&\n-\ttest_must_fail \"git stash branch tmp does-not-exist\" &&\n-\ttest_must_fail \"git stash branch tmp does-not-exist@{0}\" &&\n+\ttest_must_fail git stash drop does-not-exist &&\n+\ttest_must_fail git stash drop does-not-exist@{0} &&\n+\ttest_must_fail git stash pop does-not-exist &&\n+\ttest_must_fail git stash pop does-not-exist@{0} &&\n+\ttest_must_fail git stash apply does-not-exist &&\n+\ttest_must_fail git stash apply does-not-exist@{0} &&\n+\ttest_must_fail git stash show does-not-exist &&\n+\ttest_must_fail git stash show does-not-exist@{0} &&\n+\ttest_must_fail git stash branch tmp does-not-exist &&\n+\ttest_must_fail git stash branch tmp does-not-exist@{0} &&\n \tgit stash drop\n '\n \n test_expect_success 'invalid ref of the form stash@{n}, n >= N' '\n \tgit stash clear &&\n-\ttest_must_fail \"git stash drop stash@{0}\" &&\n+\ttest_must_fail git stash drop stash@{0} &&\n \techo bar5 > file &&\n \techo bar6 > file2 &&\n \tgit add file2 &&\n \tgit stash &&\n-\ttest_must_fail \"git drop stash@{1}\" &&\n-\ttest_must_fail \"git pop stash@{1}\" &&\n-\ttest_must_fail \"git apply stash@{1}\" &&\n-\ttest_must_fail \"git show stash@{1}\" &&\n-\ttest_must_fail \"git branch tmp stash@{1}\" &&\n+\ttest_must_fail git drop stash@{1} &&\n+\ttest_must_fail git pop stash@{1} &&\n+\ttest_must_fail git apply stash@{1} &&\n+\ttest_must_fail git show stash@{1} &&\n+\ttest_must_fail git branch tmp stash@{1} &&\n \tgit stash drop\n '\n \n-- \n1.7.2.80.g22196.dirty\n"},{"id":"149437","messageId":"20100831155457.GB11014@sigill.intra.peff.net","threadId":"24922","inReplyTo":"1283266160-11665-1-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH 0/2] fix quotes in maint-reflog-beyond-horizon and detached-stash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-08-31T15:54:57Z","receivedAt":"2010-08-31T15:54:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 01, 2010 at 12:49:18AM +1000, Jon Seymour wrote:\n\n> In two recent series, I introduced tests that use test_must_fail incorrectly.\n> \n> In effect, I was calling:\n> \n> test_must_fail \"foo bar\"\n> \n> but this was failing because \"foo bar\" is not comamnd and not because\n> the command \"foo\" fails with arguments \"bar\".\n> \n> These two patches fix these problems.\n\nI grepped around and I think you got all of the cases. I think we can\ncatch this in the test harness itself, though, with the following\nseries:\n\n [1/2]: tests: make test_must_fail more verbose\n [2/2]: tests: make test_must_fail fail on missing commands\n\nWith this series, I get (on pu):\n\n $ ./t1503-rev-parse-verify.sh -v -i\n [...]\n eval: 9: git rev-parse --verify master@{4}: not found\n test_must_fail: command not found: git rev-parse --verify master@{4}\n not ok - 7 master@{n} for various n\n\n-Peff\n"},{"id":"149438","messageId":"20100831155636.GA11530@sigill.intra.peff.net","threadId":"24922","inReplyTo":"20100831155457.GB11014@sigill.intra.peff.net","subject":"[PATCH 1/2] tests: make test_must_fail more verbose","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-08-31T15:56:36Z","receivedAt":"2010-08-31T15:56:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Because test_must_fail fails when a command succeeds, the\ncommand frequently does not produce any output (since, after\nall, it thought it was succeeding). So let's have\ntest_must_fail itself report that a problem occurred.\n\nSigned-off-by: Jeff King \n---\n t/test-lib.sh | 10 +++++++++-\n 1 files changed, 9 insertions(+), 1 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 3a3d4c4..285bfd8 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -591,7 +591,15 @@ test_path_is_missing () {\n \n test_must_fail () {\n \t\"$@\"\n-\ttest $? -gt 0 -a $? -le 129 -o $? -gt 192\n+\texit_code=$?\n+\tif test $exit_code = 0; then\n+\t\techo >&2 \"test_must_fail: command succeeded: $*\"\n+\t\treturn 1\n+\telif test $exit_code -gt 129 -a $exit_code -le 192; then\n+\t\techo >&2 \"test_must_fail: died by signal: $*\"\n+\t\treturn 1\n+\tfi\n+\treturn 0\n }\n \n # Similar to test_must_fail, but tolerates success, too. This is\n-- \n1.7.2.2.119.gf9c33\n"},{"id":"149439","messageId":"20100831155652.GB11530@sigill.intra.peff.net","threadId":"24922","inReplyTo":"20100831155457.GB11014@sigill.intra.peff.net","subject":"[PATCH 2/2] tests: make test_must_fail fail on missing commands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-08-31T15:56:53Z","receivedAt":"2010-08-31T15:56:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The point of it is to run a command that produces failure. A\nmissing command is more likely an error in the test script\n(e.g., using 'test_must_fail \"command with arguments\"', or\nrelying on a missing command).\n\nSigned-off-by: Jeff King \n---\n t/test-lib.sh | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 285bfd8..dbb13af 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -598,6 +598,9 @@ test_must_fail () {\n \telif test $exit_code -gt 129 -a $exit_code -le 192; then\n \t\techo >&2 \"test_must_fail: died by signal: $*\"\n \t\treturn 1\n+\telif test $exit_code = 127; then\n+\t\techo >&2 \"test_must_fail: command not found: $*\"\n+\t\treturn 1\n \tfi\n \treturn 0\n }\n-- \n1.7.2.2.119.gf9c33\n"},{"id":"149454","messageId":"7v7hj6d8im.fsf@alter.siamese.dyndns.org","threadId":"24922","inReplyTo":"20100831155652.GB11530@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] tests: make test_must_fail fail on missing commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-31T16:58:09Z","receivedAt":"2010-08-31T16:58:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> The point of it is to run a command that produces failure. A\n> missing command is more likely an error in the test script\n> (e.g., using 'test_must_fail \"command with arguments\"', or\n> relying on a missing command).\n>\n> Signed-off-by: Jeff King \n> ---\n> t/test-lib.sh | 3 +++\n> 1 files changed, 3 insertions(+), 0 deletions(-)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 285bfd8..dbb13af 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -598,6 +598,9 @@ test_must_fail () {\n> \telif test $exit_code -gt 129 -a $exit_code -le 192; then\n> \t\techo >&2 \"test_must_fail: died by signal: $*\"\n> \t\treturn 1\n> +\telif test $exit_code = 127; then\n> +\t\techo >&2 \"test_must_fail: command not found: $*\"\n> +\t\treturn 1\n> \tfi\n\nHmm. One worry is that if we ever exit(127) ourselves this would be\nconfused, but hopefully we are not that clueless.\n"},{"id":"149456","messageId":"7vzkw2btpv.fsf@alter.siamese.dyndns.org","threadId":"24922","inReplyTo":"1283266160-11665-1-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH 0/2] fix quotes in maint-reflog-beyond-horizon and detached-stash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-31T17:03:08Z","receivedAt":"2010-08-31T17:03:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jon Seymour writes:\n\n> In two recent series, I introduced tests that use test_must_fail incorrectly.\n>\n> In effect, I was calling:\n>\n> test_must_fail \"foo bar\"\n>\n> but this was failing because \"foo bar\" is not comamnd and not because\n> the command \"foo\" fails with arguments \"bar\".\n\nBlush; sorry for not noticing them earlier, and thanks for the fix.\n"},{"id":"149457","messageId":"20100831171055.GO2315@burratino","threadId":"24922","inReplyTo":"20100831155636.GA11530@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] tests: make test_must_fail more verbose","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-31T17:10:55Z","receivedAt":"2010-08-31T17:10:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -591,7 +591,15 @@ test_path_is_missing () {\n> \n> test_must_fail () {\n> \t\"$@\"\n> -\ttest $? -gt 0 -a $? -le 129 -o $? -gt 192\n> +\texit_code=$?\n> +\tif test $exit_code = 0; then\n> +\t\techo >&2 \"test_must_fail: command succeeded: $*\"\n> +\t\treturn 1\n> +\telif test $exit_code -gt 129 -a $exit_code -le 192; then\n> +\t\techo >&2 \"test_must_fail: died by signal: $*\"\n> +\t\treturn 1\n> +\tfi\n> +\treturn 0\n> }\n\nCan the exit status (e.g. from a shell function) be negative?\n\nThough your patch does not affect this, a command interrupted by a\nsignal will receive exit status > 192 in shells like ksh93. Posix\nsays:\n\n\tAs explained in other sections, certain exit status values\n\thave been reserved for special uses and should be used by\n\tapplications only for those purposes:\n\n\t126\n\t\tA file to be executed was found, but it was not an\n\t\texecutable utility.\n\t127\n\t\tA utility to be executed was not found.\n\t>128\n\t\tA command was interrupted by a signal.\n\nUnfortunately that does not agree with git usage.\n\n\t129\n\t\tIncorrect command-line usage, or help requested\n\t\t(rather than SIGHUP).\n\t255\n\t\tFailed to create pipe for child process, fork\n\t\tfailed, execvp failed, wait failed, invalid\n\t\tpathspec for add --patch, \"git archive\" or\n\t\t\"git daemon\" failure (rather than signal 127).\n\nHere's a test_might_fail patch for consistency.\n\n-- 8< --\nSubject: tests: make test_might_fail more verbose\n\nLet test_might_fail say something about its failures for consistency\nwith test_must_fail.\n\nCc: Jeff King \nSigned-off-by: Jonathan Nieder \n---\n t/test-lib.sh | 7 ++++++-\n 1 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 285bfd8..506787c 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -615,7 +615,12 @@ test_must_fail () {\n \n test_might_fail () {\n \t\"$@\"\n-\ttest $? -ge 0 -a $? -le 129 -o $? -gt 192\n+\texit_code=$?\n+\tif test $exit_code -gt 129 -a $exit_code -le 192; then\n+\t\techo >&2 \"test_might_fail: died by signal: $*\"\n+\t\treturn 1\n+\tfi\n+\treturn 0\n }\n \n # test_cmp is a helper function to compare actual and expected output.\n-- \n1.7.2.2\n"},{"id":"149460","messageId":"20100831172528.GA15020@sigill.intra.peff.net","threadId":"24922","inReplyTo":"7v7hj6d8im.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] tests: make test_must_fail fail on missing commands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-08-31T17:25:28Z","receivedAt":"2010-08-31T17:25:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 31, 2010 at 09:58:09AM -0700, Junio C Hamano wrote:\n\n> > +\telif test $exit_code = 127; then\n> > +\t\techo >&2 \"test_must_fail: command not found: $*\"\n> > +\t\treturn 1\n> > \tfi\n> \n> Hmm. One worry is that if we ever exit(127) ourselves this would be\n> confused, but hopefully we are not that clueless.\n\nYeah, I considered that but dismissed it. I don't think we are that\nclueless, and even if we were, then I think test_must_fail is doing a\ngood thing by pointing it out to us.\n\n-Peff\n"},{"id":"149461","messageId":"20100831172657.GP2315@burratino","threadId":"24922","inReplyTo":"20100831155652.GB11530@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] tests: make test_must_fail fail on missing commands","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-31T17:26:57Z","receivedAt":"2010-08-31T17:26:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> The point of it is to run a command that produces failure. A\n> missing command is more likely an error in the test script\n\nMakes sense. Here's the corresponding change for test_might_fail.\n\n-- 8< --\nSubject: tests: make test_might_fail fail on missing commands\n\nDetect and report hard-to-notice spelling mistakes like\n\n test_might_fail \"git config --unset whatever\"\n\n(the extra quotes prevent the shell from running git as intended;\ninstead, the shell looks for a \"git config --unset whatever\" file).\n\nCc: Jeff King \nSigned-off-by: Jonathan Nieder \n---\n t/test-lib.sh | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 16ceb53..7da490d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -622,6 +622,9 @@ test_might_fail () {\n \tif test $exit_code -gt 129 -a $exit_code -le 192; then\n \t\techo >&2 \"test_might_fail: died by signal: $*\"\n \t\treturn 1\n+\telif test $exit_code = 127; then\n+\t\techo >&2 \"test_might_fail: command not found: $*\"\n+\t\treturn 1\n \tfi\n \treturn 0\n }\n-- \n1.7.2.2\n"},{"id":"149465","messageId":"20100831180618.GB15020@sigill.intra.peff.net","threadId":"24922","inReplyTo":"20100831171055.GO2315@burratino","subject":"Re: [PATCH 1/2] tests: make test_must_fail more verbose","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-08-31T18:06:18Z","receivedAt":"2010-08-31T18:06:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 31, 2010 at 12:10:55PM -0500, Jonathan Nieder wrote:\n\n> > -\ttest $? -gt 0 -a $? -le 129 -o $? -gt 192\n> [...]\n> Can the exit status (e.g. from a shell function) be negative?\n\nI don't believe so. Both bash and dash complain about \"return -1\" (and\nPOSIX says the number must be \"an unsigned decimal integer\"). More\nimportantly, though, is how shells interpret an exit status from a\ncommand that has high bits set. Both bash and dash truncate it to 8-bits\nand treat it as unsigned. I would think any reasonable shell would do\nthat (no, I don't have a Solaris 8 box handy to test on. :) ).\n\n> Though your patch does not affect this, a command interrupted by a\n> signal will receive exit status > 192 in shells like ksh93.\n\nYeah, that is from 5a26973 (t/test-lib.sh: exit with small negagive int\nis ok with test_must_fail, 2008-07-12). I'm not sure I agree with the\nintent of that commit. We should perhaps just eliminate all the places\nwhere we use negative exit codes.\n\n> Posix says:\n> \tAs explained in other sections, certain exit status values\n> \thave been reserved for special uses and should be used by\n> \tapplications only for those purposes:\n> \n> \t126\n> \t\tA file to be executed was found, but it was not an\n> \t\texecutable utility.\n> \t127\n> \t\tA utility to be executed was not found.\n> \t>128\n> \t\tA command was interrupted by a signal.\n\nYup, that's what I have always assumed to be the case.\n\n> Unfortunately that does not agree with git usage.\n> \n> \t129\n> \t\tIncorrect command-line usage, or help requested\n> \t\t(rather than SIGHUP).\n\nYeah, we might consider changing that. We indeed confuse it with SIGHUP:\n\n $ git log -h 2>/dev/null; echo $?\n 129\n $ git log -p >/dev/null; echo $? [now killall -HUP git elsewhere]\n 129\n\nWe also use 128 for our generic die. I don't know if this is ever\nproblematic. It has the high bit set like a signal, but you don't\nactually check the bits in the shell. And the actual representation at\nthe C level is not the same.\n\nSo part of me is in favor of changing these to something more \"normal\".\nI am slightly worried, though, about breaking some caller. It is a\nuser-visible interface, though I suspect 99% of callers only ever cared\nwhether it was 0 or not.\n\n> Here's a test_might_fail patch for consistency.\n\nI didn't even know about that one. :)\n\nThey should definitely be kept in sync. I wonder if it is worth\nimplemening must_fail in terms of might_fail.\n\n> test_might_fail () {\n> \t\"$@\"\n> -\ttest $? -ge 0 -a $? -le 129 -o $? -gt 192\n> +\texit_code=$?\n> +\tif test $exit_code -gt 129 -a $exit_code -le 192; then\n> +\t\techo >&2 \"test_might_fail: died by signal: $*\"\n> +\t\treturn 1\n> +\tfi\n> +\treturn 0\n\nI wonder if 2/2 would need the \"missing command\" bit, too. Hrm.\n\n-Peff\n"},{"id":"149466","messageId":"20100831180847.GC15020@sigill.intra.peff.net","threadId":"24922","inReplyTo":"20100831172657.GP2315@burratino","subject":"Re: [PATCH 2/2] tests: make test_must_fail fail on missing commands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-08-31T18:08:48Z","receivedAt":"2010-08-31T18:08:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 31, 2010 at 12:26:57PM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > The point of it is to run a command that produces failure. A\n> > missing command is more likely an error in the test script\n> \n> Makes sense. Here's the corresponding change for test_might_fail.\n\nI think this is probably worth doing. Unless somebody is doing something\nsilly like:\n\n test_might_fail command_that_might_exist\n\nBut that seems a pretty contrived scenario (I am imagining something\nlike \"call sync now, but if we don't have it, don't fail\". But in the\ntest scripts that seems unlikely).\n\n-Peff\n"},{"id":"149469","messageId":"AANLkTimpaD7gW56sLqFzHOe+32QkXMOCkpQqJuaZNZav@mail.gmail.com","threadId":"24922","inReplyTo":"20100831180847.GC15020@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] tests: make test_must_fail fail on missing commands","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-31T18:14:14Z","receivedAt":"2010-08-31T18:14:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Aug 31, 2010 at 18:08, Jeff King wrote:\n> On Tue, Aug 31, 2010 at 12:26:57PM -0500, Jonathan Nieder wrote:\n>\n>> Jeff King wrote:\n>>\n>> > The point of it is to run a command that produces failure. A\n>> > missing command is more likely an error in the test script\n>>\n>> Makes sense.  Here's the corresponding change for test_might_fail.\n>\n> I think this is probably worth doing. Unless somebody is doing something\n> silly like:\n>\n>  test_might_fail command_that_might_exist\n>\n> But that seems a pretty contrived scenario (I am imagining something\n> like \"call sync now, but if we don't have it, don't fail\". But in the\n> test scripts that seems unlikely).\n\nYes, test_must_fail is only for git commands, not external stuff like\nsync, although some naughty people have gone and used it for grep/test\nin a few places instead of using \"!\" it seems.\n"},{"id":"149490","messageId":"20100831193515.GV2315@burratino","threadId":"24922","inReplyTo":"20100831180618.GB15020@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] tests: make test_must_fail more verbose","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-31T19:35:15Z","receivedAt":"2010-08-31T19:35:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n> On Tue, Aug 31, 2010 at 12:10:55PM -0500, Jonathan Nieder wrote:\n\n>> Can the exit status (e.g. from a shell function) be negative?\n>\n> I don't believe so. Both bash and dash complain about \"return -1\" (and\n> POSIX says the number must be \"an unsigned decimal integer\").\n\nThanks for checking; missed that. It is clearer in POSIX that the\nexit status from an external command will be truncated to an 8-bit\nunsigned integer, which as you say is the most important thing.\n"},{"id":"149525","messageId":"AANLkTi=aGdLj4vRKs8dR64JO=c8eC3SHj0_hkh6Wutaw@mail.gmail.com","threadId":"24922","inReplyTo":"20100831155636.GA11530@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] tests: make test_must_fail more verbose","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2010-09-01T03:37:31Z","receivedAt":"2010-09-01T03:37:31Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Wed, Sep 1, 2010 at 1:56 AM, Jeff King wrote:\n> Because test_must_fail fails when a command succeeds, the\n> command frequently does not produce any output (since, after\n> all, it thought it was succeeding). So let's have\n> test_must_fail itself report that a problem occurred.\n>\n> Signed-off-by: Jeff King \n\nJeff,\n\nNice fix - thank you!\n\nIt is nice to see that my initial sloppiness inspired a thoughtful\nremediation. I'll try not to rely of gitters being always so helpful\n:-)\n\njon.\n\n> ---\n>  t/test-lib.sh |   10 +++++++++-\n>  1 files changed, 9 insertions(+), 1 deletions(-)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 3a3d4c4..285bfd8 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -591,7 +591,15 @@ test_path_is_missing () {\n>\n>  test_must_fail () {\n>        \"$@\"\n> -       test $? -gt 0 -a $? -le 129 -o $? -gt 192\n> +       exit_code=$?\n> +       if test $exit_code = 0; then\n> +               echo >&2 \"test_must_fail: command succeeded: $*\"\n> +               return 1\n> +       elif test $exit_code -gt 129 -a $exit_code -le 192; then\n> +               echo >&2 \"test_must_fail: died by signal: $*\"\n> +               return 1\n> +       fi\n> +       return 0\n>  }\n>\n>  # Similar to test_must_fail, but tolerates success, too.  This is\n> --\n> 1.7.2.2.119.gf9c33\n>\n>\n"},{"id":"149552","messageId":"AANLkTinU7nV09NMvsZ+u9JCzeDsLqw3gS-7Y=i0Eqk_X@mail.gmail.com","threadId":"24922","inReplyTo":"AANLkTi=aGdLj4vRKs8dR64JO=c8eC3SHj0_hkh6Wutaw@mail.gmail.com","subject":"Re: [PATCH 1/2] tests: make test_must_fail more verbose","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-01T10:55:54Z","receivedAt":"2010-09-01T10:55:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Sep 1, 2010 at 03:37, Jon Seymour wrote:\n> On Wed, Sep 1, 2010 at 1:56 AM, Jeff King wrote:\n>> Because test_must_fail fails when a command succeeds, the\n>> command frequently does not produce any output (since, after\n>> all, it thought it was succeeding). So let's have\n>> test_must_fail itself report that a problem occurred.\n>>\n>> Signed-off-by: Jeff King \n>\n> Jeff,\n>\n> Nice fix - thank you!\n>\n> It is nice to see that my initial sloppiness inspired a thoughtful\n> remediation. I'll try not to rely of gitters being always so helpful\n> :-)\n\nFWIW I plan to make this sort of thing better for everything when I\npick up my \"WIP: Report intra-test progress with TAP subtests\" series\nagain. I.e. use >&5 instead of >&2 and turn this sort of thing into\nsubtests.\n\nBut if you're interested in picking up someone else's slack it could\nuse some help :) I don't know when I'll get around to fixing it up:\nhttp://github.com/avar/git/commit/e2ac35a8e49ceec98ca512bf106ce04c93c84b5c\n"}]}