{"thread":{"id":"24444","subject":"[PATCH] t/t3700: convert two uses of negation operator '!' to use test_must_fail","startedAt":"2010-07-20T15:24:47Z","lastAt":"2010-07-22T18:21:10Z","messageCount":40,"participants":["Brandon Casey","Ævar Arnfjörð Bjarmason","Jared Hance","Junio C Hamano","Jonathan Nieder","Erick Mattos","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"145854","messageId":"tMTpk3TmiYV54AYDiNJMBdnlXhSooJQQ1gRoAEsSwmcSwJ9ROgOpr75wpEQNx6_kZkqBtD71_QU@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":null,"subject":"[PATCH] t/t3700: convert two uses of negation operator '!' to use test_must_fail","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-07-20T15:24:47Z","receivedAt":"2010-07-20T15:24:47Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nThese two lines use the negation '!' operator to negate the result of a\nsimple command. Since these commands do not contain any pipes or other\ncomplexities, the test_must_fail function can be used and is preferred\nsince it will additionally detect termination due to a signal.\n\nThis was noticed because the second use of '!' does not include a space\nbetween the '!' and the opening parens. Ksh interprets this as follows:\n\n !(pattern-list)\n Matches anything except one of the given patterns.\n\nKsh performs a file glob using the pattern-list and then tries to execute\nthe first file in the list. If a space is added between the '!' and the\nopen parens, then Ksh will not interpret it as a pattern list, but in this\ncase, it is preferred to use test_must_fail, so lets do so.\n\nSigned-off-by: Brandon Casey \n---\n t/t3700-add.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex 47fbf53..d03495d 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -268,7 +268,7 @@ test_expect_success 'git add --dry-run of existing changed file' \"\n \n test_expect_success 'git add --dry-run of non-existing file' \"\n \techo ignored-file >>.gitignore &&\n-\t! (git add --dry-run track-this ignored-file >actual 2>&1) &&\n+\ttest_must_fail git add --dry-run track-this ignored-file >actual 2>&1 &&\n \techo \\\"fatal: pathspec 'ignored-file' did not match any files\\\" | test_cmp - actual\n \"\n \n@@ -281,7 +281,7 @@ add 'track-this'\n EOF\n \n test_expect_success 'git add --dry-run --ignore-missing of non-existing file' '\n-\t!(git add --dry-run --ignore-missing track-this ignored-file >actual 2>&1) &&\n+\ttest_must_fail git add --dry-run --ignore-missing track-this ignored-file >actual 2>&1 &&\n \ttest_cmp expect actual\n '\n \n-- \n1.6.6.2\n"},{"id":"145859","messageId":"AANLkTil9jA8Dva_KqW67c1ZgWk9_a5S1rBViui8Jn0Os@mail.gmail.com","threadId":"24444","inReplyTo":"tMTpk3TmiYV54AYDiNJMBdnlXhSooJQQ1gRoAEsSwmcSwJ9ROgOpr75wpEQNx6_kZkqBtD71_QU@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/t3700: convert two uses of negation operator '!' to use test_must_fail","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T15:55:12Z","receivedAt":"2010-07-20T15:55:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 20, 2010 at 15:24, Brandon Casey wrote:\n\n> These two lines use the negation '!' operator to negate the result of a\n> simple command.  Since these commands do not contain any pipes or other\n> complexities, the test_must_fail function can be used and is preferred\n> since it will additionally detect termination due to a signal.\n\nMaybe I'm missing something, but unless `git add --dry-run` is special\nin being killed due to a signal this seems misguided. We actually\nprefer to use !, from t/README:\n\n - test_must_fail \n\n Run a git command and ensure it fails in a controlled way. Use\n this instead of \"! \" to fail when git commands\n segfault.\n\n> This was noticed because the second use of '!' does not include a space\n> between the '!' and the opening parens.  Ksh interprets this as follows:\n>\n>   !(pattern-list)\n>      Matches anything except one of the given patterns.\n>\n> Ksh performs a file glob using the pattern-list and then tries to execute\n> the first file in the list.  If a space is added between the '!' and the\n> open parens, then Ksh will not interpret it as a pattern list, but in this\n> case, it is preferred to use test_must_fail, so lets do so.\n\nIsn't this a completely seperate thing? Was this test really the only\nbit in the test suite that did \"!foo\" instead of \"! foo\" ?\n\nDoes the test pass for you if you just:\n\n @@ -281,7 +281,7 @@ add 'track-this'\n  EOF\n\n  test_expect_success 'git add --dry-run --ignore-missing of\nnon-existing file' '\n -       !(git add --dry-run --ignore-missing track-this\nignored-file >actual 2>&1) &&\n +       ! (git add --dry-run --ignore-missing track-this\nignored-file >actual 2>&1) &&\n        test_cmp expect actual\n  '\n\n?\n\n\n> Signed-off-by: Brandon Casey \n> ---\n>  t/t3700-add.sh |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t3700-add.sh b/t/t3700-add.sh\n> index 47fbf53..d03495d 100755\n> --- a/t/t3700-add.sh\n> +++ b/t/t3700-add.sh\n> @@ -268,7 +268,7 @@ test_expect_success 'git add --dry-run of existing changed file' \"\n>\n>  test_expect_success 'git add --dry-run of non-existing file' \"\n>        echo ignored-file >>.gitignore &&\n> -       ! (git add --dry-run track-this ignored-file >actual 2>&1) &&\n> +       test_must_fail git add --dry-run track-this ignored-file >actual 2>&1 &&\n>        echo \\\"fatal: pathspec 'ignored-file' did not match any files\\\" | test_cmp - actual\n>  \"\n>\n> @@ -281,7 +281,7 @@ add 'track-this'\n>  EOF\n>\n>  test_expect_success 'git add --dry-run --ignore-missing of non-existing file' '\n> -       !(git add --dry-run --ignore-missing track-this ignored-file >actual 2>&1) &&\n> +       test_must_fail git add --dry-run --ignore-missing track-this ignored-file >actual 2>&1 &&\n>        test_cmp expect actual\n>  '\n>\n> --\n> 1.6.6.2\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"145865","messageId":"fVE942wHC3SFihkQG8AthPTKiTZtYJ9zmR2TT7F5OlkGD4IA9xPcMA@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":"AANLkTil9jA8Dva_KqW67c1ZgWk9_a5S1rBViui8Jn0Os@mail.gmail.com","subject":"Re: [PATCH] t/t3700: convert two uses of negation operator '!' to use test_must_fail","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-07-20T16:32:33Z","receivedAt":"2010-07-20T16:32:33Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 07/20/2010 10:55 AM, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Jul 20, 2010 at 15:24, Brandon Casey wrote:\n> \n>> These two lines use the negation '!' operator to negate the result of a\n>> simple command. Since these commands do not contain any pipes or other\n>> complexities, the test_must_fail function can be used and is preferred\n>> since it will additionally detect termination due to a signal.\n> \n> Maybe I'm missing something, but unless `git add --dry-run` is special\n> in being killed due to a signal this seems misguided. We actually\n> prefer to use !, from t/README:\n> \n> - test_must_fail \n> \n> Run a git command and ensure it fails in a controlled way. Use\n> this instead of \"! \" to fail when git commands\n> segfault.\n\nI think you have misunderstood the explanation of test_must_fail. The\nparagraph you quoted actually recommends using test_must_fail instead\nof \"! \".\n\nIt says:\n\n Use this instead of \"! \" to fail when git commands\n segfault.\n\nOr with a slight rewording:\n\n Use test_must_fail instead of \"! \" since test_must_fail\n will fail when segfaults.\n\nSee, if \"! \" is used, then if \"\" is terminated\ndue to some flaw in git (like a segfault), then the statement will still\nbe interpreted as a success. When test_must_fail is used, termination\ndue to segfault or other signal is detected, and the statement will\nfail.\n\n>> This was noticed because the second use of '!' does not include a space\n>> between the '!' and the opening parens. Ksh interprets this as follows:\n>>\n>> !(pattern-list)\n>> Matches anything except one of the given patterns.\n>>\n>> Ksh performs a file glob using the pattern-list and then tries to execute\n>> the first file in the list. If a space is added between the '!' and the\n>> open parens, then Ksh will not interpret it as a pattern list, but in this\n>> case, it is preferred to use test_must_fail, so lets do so.\n> \n> Isn't this a completely seperate thing? Was this test really the only\n> bit in the test suite that did \"!foo\" instead of \"! foo\" ?\n\nThis was the only instance of \"!()\" that was failing for me. I didn't\nlook before, but now that I have, there is another instance of \"!()\" in\nt5541 that should be fixed. t5541 hasn't caused a problem for me because\nGIT_TEST_HTTPD must be set in order to enable it, and I haven't done so.\n\n> Does the test pass for you if you just:\n\nYes.\n\n> @@ -281,7 +281,7 @@ add 'track-this'\n> EOF\n> \n> test_expect_success 'git add --dry-run --ignore-missing of\n> non-existing file' '\n> - !(git add --dry-run --ignore-missing track-this\n> ignored-file >actual 2>&1) &&\n> + ! (git add --dry-run --ignore-missing track-this\n> ignored-file >actual 2>&1) &&\n> test_cmp expect actual\n> '\n> \n> ?\n"},{"id":"145866","messageId":"20100720163822.GA8492@localhost.localdomain","threadId":"24444","inReplyTo":"fVE942wHC3SFihkQG8AthPTKiTZtYJ9zmR2TT7F5OlkGD4IA9xPcMA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/t3700: convert two uses of negation operator '!' to use test_must_fail","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-07-20T16:38:22Z","receivedAt":"2010-07-20T16:38:22Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"On Tue, Jul 20, 2010 at 11:32:33AM -0500, Brandon Casey wrote:\n> I think you have misunderstood the explanation of test_must_fail. The\n> paragraph you quoted actually recommends using test_must_fail instead\n> of \"! \".\n> \n> It says:\n> \n> Use this instead of \"! \" to fail when git commands\n> segfault.\n> \n> Or with a slight rewording:\n> \n> Use test_must_fail instead of \"! \" since test_must_fail\n> will fail when segfaults.\n\nI think the wording of description of test_must_fail is slightly\nambiguous. I read it to mean that:\n\n Use test_must_fail only when you are testing to see if git will\n segfault.\n\nRather than:\n \n Use test_must_fail to be safe from git segfaults.\n\n\nPerhaps the description should be updated to be a bit more clear?\n"},{"id":"145871","messageId":"0JXkybOAPrkw1RCkgKLY0ocfkmfqHFq_bWFMVWrzymAet2VX-veTSoZP1hBzIyN5JSrPw-IZjfI@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":"20100720163822.GA8492@localhost.localdomain","subject":"[PATCH] t/README: clarify test_must_fail description","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-07-20T17:17:12Z","receivedAt":"2010-07-20T17:17:12Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nSome have found the wording of the description to be somewhat ambiguous\nwith respect to when it is desirable to use test_must_fail instead of\n\"! \". Tweak the wording somewhat to hopefully clarify that\nit is _because_ test_must_fail can detect segmentation fault that it is\ndesirable to use it instead of \"! \".\n\nSigned-off-by: Brandon Casey \n---\n\n\nOn 07/20/2010 11:38 AM, Jared Hance wrote:\n> I think the wording of description of test_must_fail is slightly\n> ambiguous. I read it to mean that:\n> \n> Use test_must_fail only when you are testing to see if git will\n> segfault.\n\nI think that is a correct interpretation. But I ask you this:\nAre there times when we would _not_ want to test for segfault? :)\n\n> Rather than:\n> \n> Use test_must_fail to be safe from git segfaults.\n> \n> \n> Perhaps the description should be updated to be a bit more clear?\n\nHow about this?\n\n\n t/README | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex b906ceb..a830daa 100644\n--- a/t/README\n+++ b/t/README\n@@ -451,8 +451,8 @@ library for your script to use.\n - test_must_fail \n \n Run a git command and ensure it fails in a controlled way. Use\n- this instead of \"! \" to fail when git commands\n- segfault.\n+ this instead of \"! \" since it will fail when git\n+ commands segfault.\n \n - test_might_fail \n \n-- \n1.6.6.2\n"},{"id":"145875","messageId":"AANLkTilpspywp68veHtYbtfZytHr_0IThksmtkGAjWRg@mail.gmail.com","threadId":"24444","inReplyTo":"20100720163822.GA8492@localhost.localdomain","subject":"Re: [PATCH] t/t3700: convert two uses of negation operator '!' to use test_must_fail","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T17:52:42Z","receivedAt":"2010-07-20T17:52:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 20, 2010 at 16:38, Jared Hance wrote:\n> On Tue, Jul 20, 2010 at 11:32:33AM -0500, Brandon Casey wrote:\n>> I think you have misunderstood the explanation of test_must_fail.  The\n>> paragraph you quoted actually recommends using test_must_fail instead\n>> of \"! \".\n>>\n>> It says:\n>>\n>>    Use this instead of \"! \" to fail when git commands\n>>    segfault.\n>>\n>> Or with a slight rewording:\n>>\n>>    Use test_must_fail instead of \"! \" since test_must_fail\n>>    will fail when segfaults.\n>\n> I think the wording of description of test_must_fail is slightly\n> ambiguous. I read it to mean that:\n>\n>    Use test_must_fail only when you are testing to see if git will\n>    segfault.\n\nI've interpreted it to mean that as well, but it's starting to look\nlike a good example of a garden path sentence.\n\nAnyway, it looks like we're wrong and Brandon was right. But I'm going\nto submit a doc patch to t/README. Here's the existing use of !\nv.s. test_must_fail:\n\n $ cat *.sh | grep -c 'test_must_fail git'\n 863\n $ cat *sh | grep -c '! git '\n 30\n\nI.e. ! is only used to negate non-git commands like test.\n\nThe example in the comments for test_must_fail in test-lib.sh backs this up:\n\n # Writing this as \"! git checkout ../outerspace\" is wrong, because\n # the failure could be due to a segv. We want a controlled failure.\n\n> Rather than:\n>\n>    Use test_must_fail to be safe from git segfaults.\n"},{"id":"145876","messageId":"7veieym3sh.fsf@alter.siamese.dyndns.org","threadId":"24444","inReplyTo":"0JXkybOAPrkw1RCkgKLY0ocfkmfqHFq_bWFMVWrzymAet2VX-veTSoZP1hBzIyN5JSrPw-IZjfI@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-20T18:00:46Z","receivedAt":"2010-07-20T18:00:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> From: Brandon Casey \n>\n> Some have found the wording of the description to be somewhat ambiguous\n> with respect to when it is desirable to use test_must_fail instead of\n> \"! \". Tweak the wording somewhat to hopefully clarify that\n> it is _because_ test_must_fail can detect segmentation fault that it is\n> desirable to use it instead of \"! \".\n>\n> Signed-off-by: Brandon Casey \n> ---\n>\n>\n> On 07/20/2010 11:38 AM, Jared Hance wrote:\n>> I think the wording of description of test_must_fail is slightly\n>> ambiguous. I read it to mean that:\n>> \n>> Use test_must_fail only when you are testing to see if git will\n>> segfault.\n>\n> I think that is a correct interpretation. But I ask you this:\n> Are there times when we would _not_ want to test for segfault? :)\n>\n>> Rather than:\n>> \n>> Use test_must_fail to be safe from git segfaults.\n>> \n>> \n>> Perhaps the description should be updated to be a bit more clear?\n>\n> How about this?\n>\n>\n> t/README | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/README b/t/README\n> index b906ceb..a830daa 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -451,8 +451,8 @@ library for your script to use.\n> - test_must_fail \n> \n> Run a git command and ensure it fails in a controlled way. Use\n> - this instead of \"! \" to fail when git commands\n> - segfault.\n> + this instead of \"! \" since it will fail when git\n> + commands segfault.\n\nI found your earlier\n\n Use test_must_fail instead of \"! \" since test_must_fail\n will fail when segfaults.\n\nslightly clearer.\n\n - \"it\" in \"since it will fail\" is a bit ambiguous: is it \"!\" or\n \"test_must_fail\"?\n\n - it is not obvious if \"fail\" in \"since it will fail\" is a good thing or\n a bad thing; as we are discussing test_MUST_fail, it may even be a good\n thing that it \"will fail\"---which is not what we want our audience to\n read from this.\n\nHow about being more explicit?\n\n Run a git command and ensure it fails in a controlled way. Use\n this instead of \"! \". When git-command dies due to a\n segfault, test_must_fail diagnoses it as an error; \"! \"\n treats it as just another expected failure. letting such a bug go\n unnoticed.\n\n> \n> - test_might_fail \n> \n> -- \n> 1.6.6.2\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"145878","messageId":"AANLkTinLOLzmA9XSDYKsKwxV1Byvp-hd82JbjuSTNWb3@mail.gmail.com","threadId":"24444","inReplyTo":"7veieym3sh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T18:06:58Z","receivedAt":"2010-07-20T18:06:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 20, 2010 at 18:00, Junio C Hamano wrote:\n\n>    Run a git command and ensure it fails in a controlled way.  Use\n>    this instead of \"! \".  When git-command dies due to a\n>    segfault, test_must_fail diagnoses it as an error; \"! \"\n>    treats it as just another expected failure. letting such a bug go\n>    unnoticed.\n\nTo add to that:\n\n Don't use test_must_fail to negate the return values of commands\n on the system like grep, sed etc. If we can't trust that the core\n utilities won't randomly segfault we might as well die horribly.\n"},{"id":"145879","messageId":"20100720181431.GA12857@localhost.localdomain","threadId":"24444","inReplyTo":"AANLkTinLOLzmA9XSDYKsKwxV1Byvp-hd82JbjuSTNWb3@mail.gmail.com","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-07-20T18:14:31Z","receivedAt":"2010-07-20T18:14:31Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"I've written a patch that converted most (all?) of the \"! git\" to\n\"test_must_fail git\". I'll send it as soon as I make sure I didn't\nsomehow break the testsuite with it.\n"},{"id":"145880","messageId":"gSJqoMSCdnNZVG9-DiMIs9GCIi5KJ7c0V35BPIQOHLVdP1L6ZeO3iA@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":"7veieym3sh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-07-20T18:19:14Z","receivedAt":"2010-07-20T18:19:14Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 07/20/2010 01:00 PM, Junio C Hamano wrote:\n\n> How about being more explicit?\n> \n> Run a git command and ensure it fails in a controlled way. Use\n> this instead of \"! \". When git-command dies due to a\n> segfault, test_must_fail diagnoses it as an error; \"! \"\n> treats it as just another expected failure. letting such a bug go\n> unnoticed.\n\nMuch better. If you want the last sentence to be grammatically correct,\nyou could rephrase it like this:\n\n ...just another expected failure, which would let such a bug go unnoticed.\n\n-brandon\n"},{"id":"145882","messageId":"AANLkTilf4Z0FI9jkREJx6EJHv1OH1J4XZOQmE1rR_Mu4@mail.gmail.com","threadId":"24444","inReplyTo":"fVE942wHC3SFihkQG8AthPTKiTZtYJ9zmR2TT7F5OlkGD4IA9xPcMA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/t3700: convert two uses of negation operator '!' to use test_must_fail","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T18:25:49Z","receivedAt":"2010-07-20T18:25:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 20, 2010 at 16:32, Brandon Casey wrote:\n\n>>> This was noticed because the second use of '!' does not include a space\n>>> between the '!' and the opening parens.  Ksh interprets this as follows:\n>>>\n>>>   !(pattern-list)\n>>>      Matches anything except one of the given patterns.\n>>>\n>>> Ksh performs a file glob using the pattern-list and then tries to execute\n>>> the first file in the list.  If a space is added between the '!' and the\n>>> open parens, then Ksh will not interpret it as a pattern list, but in this\n>>> case, it is preferred to use test_must_fail, so lets do so.\n>>\n>> Isn't this a completely seperate thing? Was this test really the only\n>> bit in the test suite that did \"!foo\" instead of \"! foo\" ?\n>\n> This was the only instance of \"!()\" that was failing for me.  I didn't\n> look before, but now that I have, there is another instance of \"!()\" in\n> t5541 that should be fixed.  t5541 hasn't caused a problem for me because\n> GIT_TEST_HTTPD must be set in order to enable it, and I haven't done so.\n>\n>> Does the test pass for you if you just:\n>\n> Yes.\n>\n>>     @@ -281,7 +281,7 @@ add 'track-this'\n>>      EOF\n>>\n>>      test_expect_success 'git add --dry-run --ignore-missing of\n>> non-existing file' '\n>>     -       !(git add --dry-run --ignore-missing track-this\n>> ignored-file >actual 2>&1) &&\n>>     +       ! (git add --dry-run --ignore-missing track-this\n>> ignored-file >actual 2>&1) &&\n>>            test_cmp expect actual\n>>      '\n\nA seperate patch to clean up those two + doc patch to the \"Do's,\ndon'ts & things to keep in mind\" section in t/README would be really\nuseful IMO. I had no idea that \"!cmd\" was an issue no ksh.\n"},{"id":"145884","messageId":"7v1vaym27n.fsf@alter.siamese.dyndns.org","threadId":"24444","inReplyTo":"AANLkTinLOLzmA9XSDYKsKwxV1Byvp-hd82JbjuSTNWb3@mail.gmail.com","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-20T18:34:52Z","receivedAt":"2010-07-20T18:34:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Tue, Jul 20, 2010 at 18:00, Junio C Hamano wrote:\n>\n>>    Run a git command and ensure it fails in a controlled way.  Use\n>>    this instead of \"! \".  When git-command dies due to a\n>>    segfault, test_must_fail diagnoses it as an error; \"! \"\n>>    treats it as just another expected failure. letting such a bug go\n>>    unnoticed.\n>\n> To add to that:\n>\n> Don't use test_must_fail to negate the return values of commands\n> on the system like grep, sed etc. If we can't trust that the core\n> utilities won't randomly segfault we might as well die horribly.\n\nI think you are being incoherent. If we can't trust system \"grep\" and it\nrandomly segfaults, then a test:\n\n git some-command >actual &&\n ! grep string-that-should-not-be-in-the-output actual\n\nwould _pass_ when the command segfaults. I do agree with you that \"We\nmight as well die horribly\", and the way you do so is by protecting the\ntest with test_must_fail, like this:\n\n git some-command >actual &&\n test_must_fail grep string-that-should-not-be-in-the-output actual\n\nHaving said that, as we _do_ trust system tools to a certain degree, we do\nnot care very deeply about this. IOW, I wouldn't want to see a patch that\nrewrites \"! grep\" to \"test_must_fail grep\".\n\nThanks.\n"},{"id":"145887","messageId":"AANLkTil5eq2radUKvle7Ez48CDRfb8dvWcEobXzGaKNA@mail.gmail.com","threadId":"24444","inReplyTo":"7v1vaym27n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T18:43:29Z","receivedAt":"2010-07-20T18:43:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 20, 2010 at 18:34, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> On Tue, Jul 20, 2010 at 18:00, Junio C Hamano wrote:\n>>\n>>>    Run a git command and ensure it fails in a controlled way.  Use\n>>>    this instead of \"! \".  When git-command dies due to a\n>>>    segfault, test_must_fail diagnoses it as an error; \"! \"\n>>>    treats it as just another expected failure. letting such a bug go\n>>>    unnoticed.\n>>\n>> To add to that:\n>>\n>>     Don't use test_must_fail to negate the return values of commands\n>>     on the system like grep, sed etc. If we can't trust that the core\n>>     utilities won't randomly segfault we might as well die horribly.\n>\n> I think you are being incoherent.  If we can't trust system \"grep\" and it\n> randomly segfaults, then a test:\n>\n>    git some-command >actual &&\n>    ! grep string-that-should-not-be-in-the-output actual\n>\n> would _pass_ when the command segfaults.  I do agree with you that \"We\n> might as well die horribly\", and the way you do so is by protecting the\n> test with test_must_fail, like this:\n>\n>    git some-command >actual &&\n>    test_must_fail grep string-that-should-not-be-in-the-output actual\n>\n> Having said that, as we _do_ trust system tools to a certain degree, we do\n> not care very deeply about this.  IOW, I wouldn't want to see a patch that\n> rewrites \"! grep\" to \"test_must_fail grep\".\n\nAn individual test would pass, yes. But if test or grep are\nsegfaulting we're going to bail out horribly eventually anyway, so I\ndon't think it's worth the effort to guard them with test_must_fail,\nand I wouldn't write tests to do that. I'd just use !.\n\nThat's what we seem to be doing in the tests so far, i.e. test_must_fail\nis reserved for git commands only.\n"},{"id":"145889","messageId":"fe4308a0ab1c3d7296efa986d5cfe63c6ff4014a.1279652856.git.jaredhance@gmail.com","threadId":"24444","inReplyTo":"20100720181431.GA12857@localhost.localdomain","subject":"[PATCH] Convert \"! git\" to \"test_must_fail\" git.","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-07-20T19:09:04Z","receivedAt":"2010-07-20T19:09:04Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"test_must_fail prevents problems with segfault, so it is better to use\nrather than \"! git\".\n\nSigned-off-by: Jared Hance \n---\n t/t1001-read-tree-m-2way.sh | 2 +-\n t/t3301-notes.sh | 6 +++---\n t/t3306-notes-prune.sh | 8 ++++----\n t/t3410-rebase-preserve-dropped-merges.sh | 8 ++++----\n t/t6037-merge-ours-theirs.sh | 2 +-\n t/t7509-commit.sh | 4 ++--\n t/t7607-merge-overwrite.sh | 12 ++++++------\n t/t7810-grep.sh | 4 ++--\n t/t9100-git-svn-basic.sh | 2 +-\n t/t9130-git-svn-authors-file.sh | 4 ++--\n t/t9139-git-svn-non-utf8-commitencoding.sh | 2 +-\n t/t9140-git-svn-reset.sh | 2 +-\n 12 files changed, 28 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh\nindex 0c562bb..93ca84f 100755\n--- a/t/t1001-read-tree-m-2way.sh\n+++ b/t/t1001-read-tree-m-2way.sh\n@@ -359,7 +359,7 @@ test_expect_success \\\n \n test_expect_success \\\n 'a/b (untracked) vs a, plus c/d case test.' \\\n- '! git read-tree -u -m \"$treeH\" \"$treeM\" &&\n+ 'test_must_fail git read-tree -u -m \"$treeH\" \"$treeM\" &&\n git ls-files --stage &&\n test -f a/b'\n \ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 2d67a40..421c988 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -299,7 +299,7 @@ cat expect-F >> expect-rm-F\n test_expect_success 'verify note removal with -F /dev/null' '\n \tgit log -4 > output &&\n \ttest_cmp expect-rm-F output &&\n-\t! git notes show\n+\ttest_must_fail git notes show\n '\n \n test_expect_success 'do not create empty note with -m \"\" (setup)' '\n@@ -309,7 +309,7 @@ test_expect_success 'do not create empty note with -m \"\" (setup)' '\n test_expect_success 'verify non-creation of note with -m \"\"' '\n \tgit log -4 > output &&\n \ttest_cmp expect-rm-F output &&\n-\t! git notes show\n+\ttest_must_fail git notes show\n '\n \n cat > expect-combine_m_and_F << EOF\n@@ -357,7 +357,7 @@ cat expect-multiline >> expect-rm-remove\n test_expect_success 'verify note removal with \"git notes remove\"' '\n \tgit log -4 > output &&\n \ttest_cmp expect-rm-remove output &&\n-\t! git notes show HEAD^\n+\ttest_must_fail git notes show HEAD^\n '\n \n cat > expect << EOF\ndiff --git a/t/t3306-notes-prune.sh b/t/t3306-notes-prune.sh\nindex b455404..c428217 100755\n--- a/t/t3306-notes-prune.sh\n+++ b/t/t3306-notes-prune.sh\n@@ -67,7 +67,7 @@ test_expect_success 'remove some commits' '\n \n test_expect_success 'verify that commits are gone' '\n \n-\t! git cat-file -p 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n+\ttest_must_fail git cat-file -p 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n \tgit cat-file -p 08341ad9e94faa089d60fd3f523affb25c6da189 &&\n \tgit cat-file -p ab5f302035f2e7aaf04265f08b42034c23256e1f\n '\n@@ -106,7 +106,7 @@ test_expect_success 'prune notes' '\n \n test_expect_success 'verify that notes are gone' '\n \n-\t! git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n+\ttest_must_fail git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n \tgit notes show 08341ad9e94faa089d60fd3f523affb25c6da189 &&\n \tgit notes show ab5f302035f2e7aaf04265f08b42034c23256e1f\n '\n@@ -130,8 +130,8 @@ test_expect_success 'prune -v notes' '\n \n test_expect_success 'verify that notes are gone' '\n \n-\t! git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n-\t! git notes show 08341ad9e94faa089d60fd3f523affb25c6da189 &&\n+\ttest_must_fail git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n+\ttest_must_fail git notes show 08341ad9e94faa089d60fd3f523affb25c6da189 &&\n \tgit notes show ab5f302035f2e7aaf04265f08b42034c23256e1f\n '\n \ndiff --git a/t/t3410-rebase-preserve-dropped-merges.sh b/t/t3410-rebase-preserve-dropped-merges.sh\nindex c49143a..6f73b95 100755\n--- a/t/t3410-rebase-preserve-dropped-merges.sh\n+++ b/t/t3410-rebase-preserve-dropped-merges.sh\n@@ -43,11 +43,11 @@ test_expect_success 'setup' '\n # G2 = same changes as G\n test_expect_success 'skip same-resolution merges with -p' '\n \tgit checkout H &&\n-\t! git merge E &&\n+\ttest_must_fail git merge E &&\n \ttest_commit L file1 23 &&\n \tgit checkout I &&\n \ttest_commit G2 file1 3 &&\n-\t! git merge E &&\n+\ttest_must_fail git merge E &&\n \ttest_commit J file1 23 &&\n \ttest_commit K file7 file7 &&\n \tgit rebase -i -p L &&\n@@ -65,11 +65,11 @@ test_expect_success 'skip same-resolution merges with -p' '\n # G2 = different changes as G\n test_expect_success 'keep different-resolution merges with -p' '\n \tgit checkout H &&\n-\t! git merge E &&\n+\ttest_must_fail git merge E &&\n \ttest_commit L2 file1 23 &&\n \tgit checkout I &&\n \ttest_commit G3 file1 4 &&\n-\t! git merge E &&\n+\ttest_must_fail git merge E &&\n \ttest_commit J2 file1 24 &&\n \ttest_commit K2 file7 file7 &&\n \ttest_must_fail git rebase -i -p L2 &&\ndiff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh\nindex 8ab3d61..2cf42c7 100755\n--- a/t/t6037-merge-ours-theirs.sh\n+++ b/t/t6037-merge-ours-theirs.sh\n@@ -58,7 +58,7 @@ test_expect_success 'pull with -X' '\n \tgit reset --hard master && git pull -s recursive -X ours . side &&\n \tgit reset --hard master && git pull -s recursive -Xtheirs . side &&\n \tgit reset --hard master && git pull -s recursive -X theirs . side &&\n-\tgit reset --hard master && ! git pull -s recursive -X bork . side\n+\tgit reset --hard master && test_must_fail git pull -s recursive -X bork . side\n '\n \n test_done\ndiff --git a/t/t7509-commit.sh b/t/t7509-commit.sh\nindex 3ea33db..643ab03 100755\n--- a/t/t7509-commit.sh\n+++ b/t/t7509-commit.sh\n@@ -111,7 +111,7 @@ test_expect_success '--amend option with empty author' '\n \ttest_when_finished \"git checkout Initial\" &&\n \techo \"Empty author test\" >>foo &&\n \ttest_tick &&\n-\t! git commit -a -m \"empty author\" --amend 2>err &&\n+\ttest_must_fail git commit -a -m \"empty author\" --amend 2>err &&\n \tgrep \"empty ident\" err\n '\n \n@@ -125,7 +125,7 @@ test_expect_success '--amend option with missing author' '\n \ttest_when_finished \"git checkout Initial\" &&\n \techo \"Missing author test\" >>foo &&\n \ttest_tick &&\n-\t! git commit -a -m \"malformed author\" --amend 2>err &&\n+\ttest_must_fail git commit -a -m \"malformed author\" --amend 2>err &&\n \tgrep \"empty ident\" err\n '\n \ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex 49f4e15..d82349a 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -31,7 +31,7 @@ test_expect_success 'setup' '\n test_expect_success 'will not overwrite untracked file' '\n \tgit reset --hard c1 &&\n \tcat important > c2.c &&\n-\t! git merge c2 &&\n+\ttest_must_fail git merge c2 &&\n \ttest_cmp important c2.c\n '\n \n@@ -39,7 +39,7 @@ test_expect_success 'will not overwrite new file' '\n \tgit reset --hard c1 &&\n \tcat important > c2.c &&\n \tgit add c2.c &&\n-\t! git merge c2 &&\n+\ttest_must_fail git merge c2 &&\n \ttest_cmp important c2.c\n '\n \n@@ -48,7 +48,7 @@ test_expect_success 'will not overwrite staged changes' '\n \tcat important > c2.c &&\n \tgit add c2.c &&\n \trm c2.c &&\n-\t! git merge c2 &&\n+\ttest_must_fail git merge c2 &&\n \tgit checkout c2.c &&\n \ttest_cmp important c2.c\n '\n@@ -58,7 +58,7 @@ test_expect_success 'will not overwrite removed file' '\n \tgit rm c1.c &&\n \tgit commit -m \"rm c1.c\" &&\n \tcat important > c1.c &&\n-\t! git merge c1a &&\n+\ttest_must_fail git merge c1a &&\n \ttest_cmp important c1.c\n '\n \n@@ -68,7 +68,7 @@ test_expect_success 'will not overwrite re-added file' '\n \tgit commit -m \"rm c1.c\" &&\n \tcat important > c1.c &&\n \tgit add c1.c &&\n-\t! git merge c1a &&\n+\ttest_must_fail git merge c1a &&\n \ttest_cmp important c1.c\n '\n \n@@ -79,7 +79,7 @@ test_expect_success 'will not overwrite removed file with staged changes' '\n \tcat important > c1.c &&\n \tgit add c1.c &&\n \trm c1.c &&\n-\t! git merge c1a &&\n+\ttest_must_fail git merge c1a &&\n \tgit checkout c1.c &&\n \ttest_cmp important c1.c\n '\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 8a63227..b20edff 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -65,7 +65,7 @@ do\n \n \ttest_expect_success \"grep -w $L (w)\" '\n \t\t: >expected &&\n-\t\t! git grep -n -w -e \"^w\" >actual &&\n+\t\ttest_must_fail git grep -n -w -e \"^w\" >actual &&\n \t\ttest_cmp expected actual\n \t'\n \n@@ -134,7 +134,7 @@ do\n \t'\n \n \ttest_expect_success \"grep -c $L (no /dev/null)\" '\n-\t\t! git grep -c test $H | grep /dev/null\n+\t\ttest_must_fail git grep -c test $H | grep /dev/null\n '\n \n \ttest_expect_success \"grep --max-depth -1 $L\" '\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 13766ab..9e86e9f 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -245,7 +245,7 @@ test_expect_success 'dcommit $rev does not clobber current branch' '\n \ttest $old_head = $(git rev-parse HEAD) &&\n \ttest refs/heads/my-bar = $(git symbolic-ref HEAD) &&\n \tgit log refs/remotes/bar | grep \"change 1\" &&\n-\t! git log refs/remotes/bar | grep \"change 2\" &&\n+\ttest_must_fail git log refs/remotes/bar | grep \"change 2\" &&\n \tgit checkout master &&\n \tgit branch -D my-bar\n \t'\ndiff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh\nindex 134411e..3c4f319 100755\n--- a/t/t9130-git-svn-authors-file.sh\n+++ b/t/t9130-git-svn-authors-file.sh\n@@ -20,7 +20,7 @@ test_expect_success 'setup svnrepo' '\n \t'\n \n test_expect_success 'start import with incomplete authors file' '\n-\t! git svn clone --authors-file=svn-authors \"$svnrepo\" x\n+\ttest_must_fail git svn clone --authors-file=svn-authors \"$svnrepo\" x\n \t'\n \n test_expect_success 'imported 2 revisions successfully' '\n@@ -63,7 +63,7 @@ test_expect_success 'authors-file against globs' '\n \t'\n \n test_expect_success 'fetch fails on ee' '\n-\t( cd aa-work && ! git svn fetch --authors-file=../svn-authors )\n+\t( cd aa-work && test_must_fail git svn fetch --authors-file=../svn-authors )\n \t'\n \n tmp_config_get () {\ndiff --git a/t/t9139-git-svn-non-utf8-commitencoding.sh b/t/t9139-git-svn-non-utf8-commitencoding.sh\nindex f337959..22d80b0 100755\n--- a/t/t9139-git-svn-non-utf8-commitencoding.sh\n+++ b/t/t9139-git-svn-non-utf8-commitencoding.sh\n@@ -39,7 +39,7 @@ do\n \t(\n \t\tcd $H &&\n \t\tgit config --unset i18n.commitencoding &&\n-\t\t! git svn dcommit\n+\t\ttest_must_fail git svn dcommit\n \t)\n \t'\n done\ndiff --git a/t/t9140-git-svn-reset.sh b/t/t9140-git-svn-reset.sh\nindex 0735526..e855904 100755\n--- a/t/t9140-git-svn-reset.sh\n+++ b/t/t9140-git-svn-reset.sh\n@@ -41,7 +41,7 @@ test_expect_success 'modify hidden file in SVN repo' '\n test_expect_success 'fetch fails on modified hidden file' '\n \t( cd g &&\n \t git svn find-rev refs/remotes/git-svn > ../expect &&\n-\t ! git svn fetch 2> ../errors &&\n+\t test_must_fail git svn fetch 2> ../errors &&\n \t git svn find-rev refs/remotes/git-svn > ../expect2 ) &&\n \tfgrep \"not found in commit\" errors &&\n \ttest_cmp expect expect2\n-- \n1.7.1.1\n"},{"id":"145891","messageId":"20100720191655.GA1705@burratino","threadId":"24444","inReplyTo":"AANLkTil5eq2radUKvle7Ez48CDRfb8dvWcEobXzGaKNA@mail.gmail.com","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-20T19:16:56Z","receivedAt":"2010-07-20T19:16:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> That's what we seem to be doing in the tests so far, i.e. test_must_fail\n> is reserved for git commands only.\n\ntest_must_fail relies on conventions for return value that cannot\nnecessarily be relied on from outside utilities.\n\nThanks for cleaning up my mess.\nJonathan\n"},{"id":"145895","messageId":"_BUqzkdWhTIktk0rYFvr9jdvISwfrShKmEsXaqgCBwm0X5Urcv2osQ@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":"fe4308a0ab1c3d7296efa986d5cfe63c6ff4014a.1279652856.git.jaredhance@gmail.com","subject":"Re: [PATCH] Convert \"! git\" to \"test_must_fail\" git.","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-07-20T19:42:17Z","receivedAt":"2010-07-20T19:42:17Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"\nAre you sure these are working properly? If I recall\ncorrectly, test_must_fail does not act the same as '!'\nwhen a '|' is involved. I believe the '!' negates the\nresult of the entire statement, but test_must_fail only\noperates on the arguments preceding the '|'.\n\nThe rest looks good, and I'm surprised there were so many.\n\nOn 07/20/2010 02:09 PM, Jared Hance wrote:\n> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\n> index 8a63227..b20edff 100755\n> --- a/t/t7810-grep.sh\n> +++ b/t/t7810-grep.sh\n\n> @@ -134,7 +134,7 @@ do\n> \t'\n> \n> \ttest_expect_success \"grep -c $L (no /dev/null)\" '\n> -\t\t! git grep -c test $H | grep /dev/null\n> +\t\ttest_must_fail git grep -c test $H | grep /dev/null\n> '\n> \n> \ttest_expect_success \"grep --max-depth -1 $L\" '\n> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\n> index 13766ab..9e86e9f 100755\n> --- a/t/t9100-git-svn-basic.sh\n> +++ b/t/t9100-git-svn-basic.sh\n> @@ -245,7 +245,7 @@ test_expect_success 'dcommit $rev does not clobber current branch' '\n> \ttest $old_head = $(git rev-parse HEAD) &&\n> \ttest refs/heads/my-bar = $(git symbolic-ref HEAD) &&\n> \tgit log refs/remotes/bar | grep \"change 1\" &&\n> -\t! git log refs/remotes/bar | grep \"change 2\" &&\n> +\ttest_must_fail git log refs/remotes/bar | grep \"change 2\" &&\n> \tgit checkout master &&\n> \tgit branch -D my-bar\n> \t'\n"},{"id":"145897","messageId":"20100720194807.GA2259@burratino","threadId":"24444","inReplyTo":"_BUqzkdWhTIktk0rYFvr9jdvISwfrShKmEsXaqgCBwm0X5Urcv2osQ@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] Convert \"! git\" to \"test_must_fail\" git.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-20T19:48:07Z","receivedAt":"2010-07-20T19:48:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Brandon Casey wrote:\n\n> I believe the '!' negates the\n> result of the entire statement, but test_must_fail only\n> operates on the arguments preceding the '|'.\n\nRight. For example:\n\n> On 07/20/2010 02:09 PM, Jared Hance wrote:\n\n[...]\n>> +++ b/t/t7810-grep.sh\n>> @@ -134,7 +134,7 @@ do\n>> \t'\n>> \n>> \ttest_expect_success \"grep -c $L (no /dev/null)\" '\n>> -\t\t! git grep -c test $H | grep /dev/null\n>> +\t\ttest_must_fail git grep -c test $H | grep /dev/null\n\nshould be\n\n\tgit grep -c test $H | ! grep /dev/null\n\nor if it is considered important to test the exit code of\ngit grep,\n\n\tgit grep -c test $H >actual &&\n\t! grep /dev/null actual\n\nRegards,\nJonathan\n"},{"id":"145898","messageId":"7f2yYiHON5cy37krPeX7WXFeoN42YYjGc04eHV92atxS4vuERQ3MCA@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":"20100720194807.GA2259@burratino","subject":"Re: [PATCH] Convert \"! git\" to \"test_must_fail\" git.","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-07-20T19:59:04Z","receivedAt":"2010-07-20T19:59:04Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 07/20/2010 02:48 PM, Jonathan Nieder wrote:\n> Brandon Casey wrote:\n> \n>> I believe the '!' negates the\n>> result of the entire statement, but test_must_fail only\n>> operates on the arguments preceding the '|'.\n> \n> Right. For example:\n> \n>> On 07/20/2010 02:09 PM, Jared Hance wrote:\n> \n> [...]\n>>> +++ b/t/t7810-grep.sh\n>>> @@ -134,7 +134,7 @@ do\n>>> \t'\n>>> \n>>> \ttest_expect_success \"grep -c $L (no /dev/null)\" '\n>>> -\t\t! git grep -c test $H | grep /dev/null\n>>> +\t\ttest_must_fail git grep -c test $H | grep /dev/null\n> \n> should be\n> \n> \tgit grep -c test $H | ! grep /dev/null\n\nI seem to recall that that doesn't work either.\n\n $ /bin/bash -c 'echo foo | ! grep bar && echo success || echo failure'\n /bin/bash: -c: line 0: syntax error near unexpected token `!'\n /bin/bash: -c: line 0: `echo foo | ! grep bar && echo success || echo failure'\n\n-brandon\n"},{"id":"145904","messageId":"AANLkTimjTxbw0FCujPNbkzuFAbdXZBgWMmDTQeAegvNq@mail.gmail.com","threadId":"24444","inReplyTo":"20100720191655.GA1705@burratino","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T20:49:45Z","receivedAt":"2010-07-20T20:49:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 20, 2010 at 19:16, Jonathan Nieder wrote:\n> Ęvar Arnfjörš Bjarmason wrote:\n>\n>> That's what we seem to be doing in the tests so far, i.e. test_must_fail\n>> is reserved for git commands only.\n>\n> test_must_fail relies on conventions for return value that cannot\n> necessarily be relied on from outside utilities.\n\nRight, someone should send a patch for these:\n\n ack 'test_must_fail (?!git)' *sh\n\n:)\n"},{"id":"145905","messageId":"8HvhdiflWJtex2eC6n_6Q38YcvRRYhnh0scnq4s56M4wdwT_YlAiOw@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":"AANLkTimjTxbw0FCujPNbkzuFAbdXZBgWMmDTQeAegvNq@mail.gmail.com","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-07-20T21:12:35Z","receivedAt":"2010-07-20T21:12:35Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 07/20/2010 03:49 PM, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Jul 20, 2010 at 19:16, Jonathan Nieder wrote:\n>> Ęvar Arnfjörš Bjarmason wrote:\n>>\n>>> That's what we seem to be doing in the tests so far, i.e. test_must_fail\n>>> is reserved for git commands only.\n>> test_must_fail relies on conventions for return value that cannot\n>> necessarily be relied on from outside utilities.\n> \n> Right, someone should send a patch for these:\n> \n> ack 'test_must_fail (?!git)' *sh\n> \n> :)\n\nYou joke, but thanks to your prodding, I discovered these broken\ntests that should definitely all be fixed:\n\n $ perl -ne 'm/test_must_fail +[^ ]+=/ && print' *sh\n\n test_must_fail PAGER= git reflog show delta &&\n test_must_fail PAGER= git reflog show epsilon &&\n test_must_fail PAGER= git reflog show epsilon\n test_must_fail PAGER= git reflog show zeta &&\n test_must_fail PAGER= git reflog show eta &&\n test_must_fail PAGER= git reflog show eta\n test_must_fail PAGER= git reflog show beta\n test_must_fail MSG=\"yet another note\" git notes add -c deadbeef &&\n\none-shot variable assignment does not work with test_must_fail.\n\nSee e2007832552ccea9befed9003580c494f09e666e for an explanation.\n\n-brandon\n"},{"id":"145906","messageId":"AANLkTinStfwbIWdtowoPpYpuT99tfweHBJ6U5m0oRtOc@mail.gmail.com","threadId":"24444","inReplyTo":"8HvhdiflWJtex2eC6n_6Q38YcvRRYhnh0scnq4s56M4wdwT_YlAiOw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/README: clarify test_must_fail description","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T21:25:33Z","receivedAt":"2010-07-20T21:25:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 20, 2010 at 21:12, Brandon Casey wrote:\n> On 07/20/2010 03:49 PM, Ævar Arnfjörð Bjarmason wrote:\n>> On Tue, Jul 20, 2010 at 19:16, Jonathan Nieder wrote:\n>>> Ęvar Arnfjörš Bjarmason wrote:\n>>>\n>>>> That's what we seem to be doing in the tests so far, i.e. test_must_fail\n>>>> is reserved for git commands only.\n>>> test_must_fail relies on conventions for return value that cannot\n>>> necessarily be relied on from outside utilities.\n>>\n>> Right, someone should send a patch for these:\n>>\n>>     ack 'test_must_fail (?!git)' *sh\n>>\n>> :)\n>\n> You joke, but thanks to your prodding, I discovered these broken\n> tests that should definitely all be fixed:\n\nOh I'm completely serious, I'm just too lazy to do these myself today :)\n\n>   $ perl -ne 'm/test_must_fail +[^ ]+=/ && print' *sh\n>\n>        test_must_fail PAGER= git reflog show delta &&\n>        test_must_fail PAGER= git reflog show epsilon &&\n>        test_must_fail PAGER= git reflog show epsilon\n>        test_must_fail PAGER= git reflog show zeta &&\n>        test_must_fail PAGER= git reflog show eta &&\n>        test_must_fail PAGER= git reflog show eta\n>        test_must_fail PAGER= git reflog show beta\n>        test_must_fail MSG=\"yet another note\" git notes add -c deadbeef &&\n>\n> one-shot variable assignment does not work with test_must_fail.\n>\n> See e2007832552ccea9befed9003580c494f09e666e for an explanation.\n\nGood catch.\n"},{"id":"145907","messageId":"iU5XdZGtMeaspoCqSJIp6Y--60TPVkZUrm3SdW86dsTZkNYZWqbSppLBrMXyL1rVqqYtHm94ACo@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":"8HvhdiflWJtex2eC6n_6Q38YcvRRYhnh0scnq4s56M4wdwT_YlAiOw@cipher.nrlssc.navy.mil","subject":"[PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-07-20T21:55:31Z","receivedAt":"2010-07-20T21:55:31Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"No time to investigate, but here is an example patch and the\nresults of running the affected tests. Looks like reflog may\nbe creating a reflog when it is not supposed to.\n\nErick, I added you to cc since you appear to be the author of the tests\nin question.\n\n$ ./t2017-checkout-orphan.sh\n\nnot ok - 8 --orphan does not make reflog when core.logAllRefUpdates = false\n#\n# git checkout master &&\n# git config core.logAllRefUpdates false &&\n# git checkout --orphan epsilon &&\n# ! test -f .git/logs/refs/heads/epsilon &&\n# (\n# PAGER= &&\n# export PAGER &&\n# test_must_fail git reflog show epsilon\n# ) &&\n# git commit -m Epsilon &&\n# ! test -f .git/logs/refs/heads/epsilon &&\n# (\n# PAGER= &&\n# export PAGER &&\n# test_must_fail git reflog show epsilon\n# )\n#\n\n\n$ ./t3200-branch.sh\n\nnot ok - 32 checkout -b does not make reflog when core.logAllRefUpdates = false\n#\n# git checkout master &&\n# git config core.logAllRefUpdates false &&\n# git checkout -b beta &&\n# ! test -f .git/logs/refs/heads/beta &&\n# (\n# PAGER= &&\n# export PAGER &&\n# test_must_fail git reflog show beta\n# )\n#\n\n\n\n--->8---\nFrom: Brandon Casey \n\nSee e2007832552ccea9befed9003580c494f09e666e\n---\n t/t2017-checkout-orphan.sh | 36 ++++++++++++++++++++++++++++++------\n t/t3200-branch.sh | 6 +++++-\n t/t3301-notes.sh | 6 +++++-\n 3 files changed, 40 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh\nindex be88d4b..81cb393 100755\n--- a/t/t2017-checkout-orphan.sh\n+++ b/t/t2017-checkout-orphan.sh\n@@ -69,7 +69,11 @@ test_expect_success '--orphan makes reflog by default' '\n \tgit config --unset core.logAllRefUpdates &&\n \tgit checkout --orphan delta &&\n \t! test -f .git/logs/refs/heads/delta &&\n-\ttest_must_fail PAGER= git reflog show delta &&\n+\t(\n+\t\tPAGER= &&\n+\t\texport PAGER &&\n+\t\ttest_must_fail git reflog show delta\n+\t) &&\n \tgit commit -m Delta &&\n \ttest -f .git/logs/refs/heads/delta &&\n \tPAGER= git reflog show delta\n@@ -80,17 +84,29 @@ test_expect_success '--orphan does not make reflog when core.logAllRefUpdates =\n \tgit config core.logAllRefUpdates false &&\n \tgit checkout --orphan epsilon &&\n \t! test -f .git/logs/refs/heads/epsilon &&\n-\ttest_must_fail PAGER= git reflog show epsilon &&\n+\t(\n+\t\tPAGER= &&\n+\t\texport PAGER &&\n+\t\ttest_must_fail git reflog show epsilon\n+\t) &&\n \tgit commit -m Epsilon &&\n \t! test -f .git/logs/refs/heads/epsilon &&\n-\ttest_must_fail PAGER= git reflog show epsilon\n+\t(\n+\t\tPAGER= &&\n+\t\texport PAGER &&\n+\t\ttest_must_fail git reflog show epsilon\n+\t)\n '\n \n test_expect_success '--orphan with -l makes reflog when core.logAllRefUpdates = false' '\n \tgit checkout master &&\n \tgit checkout -l --orphan zeta &&\n \ttest -f .git/logs/refs/heads/zeta &&\n-\ttest_must_fail PAGER= git reflog show zeta &&\n+\t(\n+\t\tPAGER= &&\n+\t\texport PAGER &&\n+\t\ttest_must_fail git reflog show zeta\n+\t) &&\n \tgit commit -m Zeta &&\n \tPAGER= git reflog show zeta\n '\n@@ -99,10 +115,18 @@ test_expect_success 'giving up --orphan not committed when -l and core.logAllRef\n \tgit checkout master &&\n \tgit checkout -l --orphan eta &&\n \ttest -f .git/logs/refs/heads/eta &&\n-\ttest_must_fail PAGER= git reflog show eta &&\n+\t(\n+\t\tPAGER= &&\n+\t\texport PAGER &&\n+\t\ttest_must_fail git reflog show eta\n+\t) &&\n \tgit checkout master &&\n \t! test -f .git/logs/refs/heads/eta &&\n-\ttest_must_fail PAGER= git reflog show eta\n+\t(\n+\t\tPAGER= &&\n+\t\texport PAGER &&\n+\t\ttest_must_fail git reflog show eta\n+\t)\n '\n \n test_expect_success '--orphan is rejected with an existing name' '\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 859b99a..bf7747d 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -237,7 +237,11 @@ test_expect_success 'checkout -b does not make reflog when core.logAllRefUpdates\n \tgit config core.logAllRefUpdates false &&\n \tgit checkout -b beta &&\n \t! test -f .git/logs/refs/heads/beta &&\n-\ttest_must_fail PAGER= git reflog show beta\n+\t(\n+\t\tPAGER= &&\n+\t\texport PAGER &&\n+\t\ttest_must_fail git reflog show beta\n+\t)\n '\n \n test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates = false' '\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 2d67a40..1d82f79 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -693,7 +693,11 @@ test_expect_success 'create note from non-existing note with \"git notes add -c\"\n \tgit add a10 &&\n \ttest_tick &&\n \tgit commit -m 10th &&\n-\ttest_must_fail MSG=\"yet another note\" git notes add -c deadbeef &&\n+\t(\n+\t\tMSG=\"yet another note\" &&\n+\t\texport MSG &&\n+\t\ttest_must_fail git notes add -c deadbeef\n+\t) &&\n \ttest_must_fail git notes list HEAD\n '\n \n-- \n1.6.6.2\n"},{"id":"145911","messageId":"d0f3d6c2c02ae611aec4c301ff45a9878db7a681.1279667857.git.jaredhance@gmail.com","threadId":"24444","inReplyTo":"_BUqzkdWhTIktk0rYFvr9jdvISwfrShKmEsXaqgCBwm0X5Urcv2osQ@cipher.nrlssc.navy.mil","subject":"[PATCH v2] Convert \"! git\" to \"test_must_fail git\"","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-07-20T23:18:34Z","receivedAt":"2010-07-20T23:18:34Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"test_must_fail will account for segfaults in git, so it should be used\ninstead of \"! git\"\n\nThis patch does not change any of the commands that use pipes.\n\nSigned-off-by: Jared Hance \n---\n t/t1001-read-tree-m-2way.sh | 2 +-\n t/t3301-notes.sh | 6 +++---\n t/t3306-notes-prune.sh | 8 ++++----\n t/t3410-rebase-preserve-dropped-merges.sh | 8 ++++----\n t/t6037-merge-ours-theirs.sh | 2 +-\n t/t7509-commit.sh | 4 ++--\n t/t7607-merge-overwrite.sh | 12 ++++++------\n t/t7810-grep.sh | 2 +-\n t/t9130-git-svn-authors-file.sh | 4 ++--\n t/t9139-git-svn-non-utf8-commitencoding.sh | 2 +-\n t/t9140-git-svn-reset.sh | 2 +-\n 11 files changed, 26 insertions(+), 26 deletions(-)\n\ndiff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh\nindex 0c562bb..93ca84f 100755\n--- a/t/t1001-read-tree-m-2way.sh\n+++ b/t/t1001-read-tree-m-2way.sh\n@@ -359,7 +359,7 @@ test_expect_success \\\n \n test_expect_success \\\n 'a/b (untracked) vs a, plus c/d case test.' \\\n- '! git read-tree -u -m \"$treeH\" \"$treeM\" &&\n+ 'test_must_fail git read-tree -u -m \"$treeH\" \"$treeM\" &&\n git ls-files --stage &&\n test -f a/b'\n \ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 2d67a40..421c988 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -299,7 +299,7 @@ cat expect-F >> expect-rm-F\n test_expect_success 'verify note removal with -F /dev/null' '\n \tgit log -4 > output &&\n \ttest_cmp expect-rm-F output &&\n-\t! git notes show\n+\ttest_must_fail git notes show\n '\n \n test_expect_success 'do not create empty note with -m \"\" (setup)' '\n@@ -309,7 +309,7 @@ test_expect_success 'do not create empty note with -m \"\" (setup)' '\n test_expect_success 'verify non-creation of note with -m \"\"' '\n \tgit log -4 > output &&\n \ttest_cmp expect-rm-F output &&\n-\t! git notes show\n+\ttest_must_fail git notes show\n '\n \n cat > expect-combine_m_and_F << EOF\n@@ -357,7 +357,7 @@ cat expect-multiline >> expect-rm-remove\n test_expect_success 'verify note removal with \"git notes remove\"' '\n \tgit log -4 > output &&\n \ttest_cmp expect-rm-remove output &&\n-\t! git notes show HEAD^\n+\ttest_must_fail git notes show HEAD^\n '\n \n cat > expect << EOF\ndiff --git a/t/t3306-notes-prune.sh b/t/t3306-notes-prune.sh\nindex b455404..c428217 100755\n--- a/t/t3306-notes-prune.sh\n+++ b/t/t3306-notes-prune.sh\n@@ -67,7 +67,7 @@ test_expect_success 'remove some commits' '\n \n test_expect_success 'verify that commits are gone' '\n \n-\t! git cat-file -p 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n+\ttest_must_fail git cat-file -p 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n \tgit cat-file -p 08341ad9e94faa089d60fd3f523affb25c6da189 &&\n \tgit cat-file -p ab5f302035f2e7aaf04265f08b42034c23256e1f\n '\n@@ -106,7 +106,7 @@ test_expect_success 'prune notes' '\n \n test_expect_success 'verify that notes are gone' '\n \n-\t! git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n+\ttest_must_fail git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n \tgit notes show 08341ad9e94faa089d60fd3f523affb25c6da189 &&\n \tgit notes show ab5f302035f2e7aaf04265f08b42034c23256e1f\n '\n@@ -130,8 +130,8 @@ test_expect_success 'prune -v notes' '\n \n test_expect_success 'verify that notes are gone' '\n \n-\t! git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n-\t! git notes show 08341ad9e94faa089d60fd3f523affb25c6da189 &&\n+\ttest_must_fail git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&\n+\ttest_must_fail git notes show 08341ad9e94faa089d60fd3f523affb25c6da189 &&\n \tgit notes show ab5f302035f2e7aaf04265f08b42034c23256e1f\n '\n \ndiff --git a/t/t3410-rebase-preserve-dropped-merges.sh b/t/t3410-rebase-preserve-dropped-merges.sh\nindex c49143a..6f73b95 100755\n--- a/t/t3410-rebase-preserve-dropped-merges.sh\n+++ b/t/t3410-rebase-preserve-dropped-merges.sh\n@@ -43,11 +43,11 @@ test_expect_success 'setup' '\n # G2 = same changes as G\n test_expect_success 'skip same-resolution merges with -p' '\n \tgit checkout H &&\n-\t! git merge E &&\n+\ttest_must_fail git merge E &&\n \ttest_commit L file1 23 &&\n \tgit checkout I &&\n \ttest_commit G2 file1 3 &&\n-\t! git merge E &&\n+\ttest_must_fail git merge E &&\n \ttest_commit J file1 23 &&\n \ttest_commit K file7 file7 &&\n \tgit rebase -i -p L &&\n@@ -65,11 +65,11 @@ test_expect_success 'skip same-resolution merges with -p' '\n # G2 = different changes as G\n test_expect_success 'keep different-resolution merges with -p' '\n \tgit checkout H &&\n-\t! git merge E &&\n+\ttest_must_fail git merge E &&\n \ttest_commit L2 file1 23 &&\n \tgit checkout I &&\n \ttest_commit G3 file1 4 &&\n-\t! git merge E &&\n+\ttest_must_fail git merge E &&\n \ttest_commit J2 file1 24 &&\n \ttest_commit K2 file7 file7 &&\n \ttest_must_fail git rebase -i -p L2 &&\ndiff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh\nindex 8ab3d61..2cf42c7 100755\n--- a/t/t6037-merge-ours-theirs.sh\n+++ b/t/t6037-merge-ours-theirs.sh\n@@ -58,7 +58,7 @@ test_expect_success 'pull with -X' '\n \tgit reset --hard master && git pull -s recursive -X ours . side &&\n \tgit reset --hard master && git pull -s recursive -Xtheirs . side &&\n \tgit reset --hard master && git pull -s recursive -X theirs . side &&\n-\tgit reset --hard master && ! git pull -s recursive -X bork . side\n+\tgit reset --hard master && test_must_fail git pull -s recursive -X bork . side\n '\n \n test_done\ndiff --git a/t/t7509-commit.sh b/t/t7509-commit.sh\nindex 3ea33db..643ab03 100755\n--- a/t/t7509-commit.sh\n+++ b/t/t7509-commit.sh\n@@ -111,7 +111,7 @@ test_expect_success '--amend option with empty author' '\n \ttest_when_finished \"git checkout Initial\" &&\n \techo \"Empty author test\" >>foo &&\n \ttest_tick &&\n-\t! git commit -a -m \"empty author\" --amend 2>err &&\n+\ttest_must_fail git commit -a -m \"empty author\" --amend 2>err &&\n \tgrep \"empty ident\" err\n '\n \n@@ -125,7 +125,7 @@ test_expect_success '--amend option with missing author' '\n \ttest_when_finished \"git checkout Initial\" &&\n \techo \"Missing author test\" >>foo &&\n \ttest_tick &&\n-\t! git commit -a -m \"malformed author\" --amend 2>err &&\n+\ttest_must_fail git commit -a -m \"malformed author\" --amend 2>err &&\n \tgrep \"empty ident\" err\n '\n \ndiff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh\nindex 49f4e15..d82349a 100755\n--- a/t/t7607-merge-overwrite.sh\n+++ b/t/t7607-merge-overwrite.sh\n@@ -31,7 +31,7 @@ test_expect_success 'setup' '\n test_expect_success 'will not overwrite untracked file' '\n \tgit reset --hard c1 &&\n \tcat important > c2.c &&\n-\t! git merge c2 &&\n+\ttest_must_fail git merge c2 &&\n \ttest_cmp important c2.c\n '\n \n@@ -39,7 +39,7 @@ test_expect_success 'will not overwrite new file' '\n \tgit reset --hard c1 &&\n \tcat important > c2.c &&\n \tgit add c2.c &&\n-\t! git merge c2 &&\n+\ttest_must_fail git merge c2 &&\n \ttest_cmp important c2.c\n '\n \n@@ -48,7 +48,7 @@ test_expect_success 'will not overwrite staged changes' '\n \tcat important > c2.c &&\n \tgit add c2.c &&\n \trm c2.c &&\n-\t! git merge c2 &&\n+\ttest_must_fail git merge c2 &&\n \tgit checkout c2.c &&\n \ttest_cmp important c2.c\n '\n@@ -58,7 +58,7 @@ test_expect_success 'will not overwrite removed file' '\n \tgit rm c1.c &&\n \tgit commit -m \"rm c1.c\" &&\n \tcat important > c1.c &&\n-\t! git merge c1a &&\n+\ttest_must_fail git merge c1a &&\n \ttest_cmp important c1.c\n '\n \n@@ -68,7 +68,7 @@ test_expect_success 'will not overwrite re-added file' '\n \tgit commit -m \"rm c1.c\" &&\n \tcat important > c1.c &&\n \tgit add c1.c &&\n-\t! git merge c1a &&\n+\ttest_must_fail git merge c1a &&\n \ttest_cmp important c1.c\n '\n \n@@ -79,7 +79,7 @@ test_expect_success 'will not overwrite removed file with staged changes' '\n \tcat important > c1.c &&\n \tgit add c1.c &&\n \trm c1.c &&\n-\t! git merge c1a &&\n+\ttest_must_fail git merge c1a &&\n \tgit checkout c1.c &&\n \ttest_cmp important c1.c\n '\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 8a63227..023f225 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -65,7 +65,7 @@ do\n \n \ttest_expect_success \"grep -w $L (w)\" '\n \t\t: >expected &&\n-\t\t! git grep -n -w -e \"^w\" >actual &&\n+\t\ttest_must_fail git grep -n -w -e \"^w\" >actual &&\n \t\ttest_cmp expected actual\n \t'\n \ndiff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh\nindex 134411e..3c4f319 100755\n--- a/t/t9130-git-svn-authors-file.sh\n+++ b/t/t9130-git-svn-authors-file.sh\n@@ -20,7 +20,7 @@ test_expect_success 'setup svnrepo' '\n \t'\n \n test_expect_success 'start import with incomplete authors file' '\n-\t! git svn clone --authors-file=svn-authors \"$svnrepo\" x\n+\ttest_must_fail git svn clone --authors-file=svn-authors \"$svnrepo\" x\n \t'\n \n test_expect_success 'imported 2 revisions successfully' '\n@@ -63,7 +63,7 @@ test_expect_success 'authors-file against globs' '\n \t'\n \n test_expect_success 'fetch fails on ee' '\n-\t( cd aa-work && ! git svn fetch --authors-file=../svn-authors )\n+\t( cd aa-work && test_must_fail git svn fetch --authors-file=../svn-authors )\n \t'\n \n tmp_config_get () {\ndiff --git a/t/t9139-git-svn-non-utf8-commitencoding.sh b/t/t9139-git-svn-non-utf8-commitencoding.sh\nindex f337959..22d80b0 100755\n--- a/t/t9139-git-svn-non-utf8-commitencoding.sh\n+++ b/t/t9139-git-svn-non-utf8-commitencoding.sh\n@@ -39,7 +39,7 @@ do\n \t(\n \t\tcd $H &&\n \t\tgit config --unset i18n.commitencoding &&\n-\t\t! git svn dcommit\n+\t\ttest_must_fail git svn dcommit\n \t)\n \t'\n done\ndiff --git a/t/t9140-git-svn-reset.sh b/t/t9140-git-svn-reset.sh\nindex 0735526..e855904 100755\n--- a/t/t9140-git-svn-reset.sh\n+++ b/t/t9140-git-svn-reset.sh\n@@ -41,7 +41,7 @@ test_expect_success 'modify hidden file in SVN repo' '\n test_expect_success 'fetch fails on modified hidden file' '\n \t( cd g &&\n \t git svn find-rev refs/remotes/git-svn > ../expect &&\n-\t ! git svn fetch 2> ../errors &&\n+\t test_must_fail git svn fetch 2> ../errors &&\n \t git svn find-rev refs/remotes/git-svn > ../expect2 ) &&\n \tfgrep \"not found in commit\" errors &&\n \ttest_cmp expect expect2\n-- \n1.7.1.1\n"},{"id":"145912","messageId":"AANLkTilY0fYxKlWLYGU4f4ZtJzsMSSnIIi0YnywKU5DU@mail.gmail.com","threadId":"24444","inReplyTo":"iU5XdZGtMeaspoCqSJIp6Y--60TPVkZUrm3SdW86dsTZkNYZWqbSppLBrMXyL1rVqqYtHm94ACo@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Erick Mattos","fromEmail":"erick.mattos@gmail.com","sentAt":"2010-07-20T23:19:02Z","receivedAt":"2010-07-20T23:19:02Z","isPatch":true,"sender":{"key":"erick.mattos@gmail.com","avatar":"https://avatars.githubusercontent.com/u/134001?v=4"},"body":"Hi,\n\n2010/7/20 Brandon Casey \n>\n> No time to investigate, but here is an example patch and the\n> results of running the affected tests.  Looks like reflog may\n> be creating a reflog when it is not supposed to.\n>\n> Erick, I added you to cc since you appear to be the author of the tests\n> in question.\n\nThanks for your kindness.\n\n> $ ./t2017-checkout-orphan.sh\n> \n> not ok - 8 --orphan does not make reflog when core.logAllRefUpdates = false\n> #\n> #               git checkout master &&\n> #               git config core.logAllRefUpdates false &&\n> #               git checkout --orphan epsilon &&\n> #               ! test -f .git/logs/refs/heads/epsilon &&\n> #               (\n> #                       PAGER= &&\n> #                       export PAGER &&\n> #                       test_must_fail git reflog show epsilon\n> #               ) &&\n> #               git commit -m Epsilon &&\n> #               ! test -f .git/logs/refs/heads/epsilon &&\n> #               (\n> #                       PAGER= &&\n> #                       export PAGER &&\n> #                       test_must_fail git reflog show epsilon\n> #               )\n> #\n> \n>\n> $ ./t3200-branch.sh\n> \n> not ok - 32 checkout -b does not make reflog when core.logAllRefUpdates = false\n> #\n> #               git checkout master &&\n> #               git config core.logAllRefUpdates false &&\n> #               git checkout -b beta &&\n> #               ! test -f .git/logs/refs/heads/beta &&\n> #               (\n> #                       PAGER= &&\n> #                       export PAGER &&\n> #                       test_must_fail git reflog show beta\n> #               )\n> #\n> \n\nYou have made cosmetic changes which do not do the same as the original.\n\n>\n> --->8---\n> From: Brandon Casey \n>\n> See e2007832552ccea9befed9003580c494f09e666e\n> ---\n>  t/t2017-checkout-orphan.sh |   36 ++++++++++++++++++++++++++++++------\n>  t/t3200-branch.sh          |    6 +++++-\n>  t/t3301-notes.sh           |    6 +++++-\n>  3 files changed, 40 insertions(+), 8 deletions(-)\n>\n> diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh\n> index be88d4b..81cb393 100755\n> --- a/t/t2017-checkout-orphan.sh\n> +++ b/t/t2017-checkout-orphan.sh\n> @@ -69,7 +69,11 @@ test_expect_success '--orphan makes reflog by default' '\n>        git config --unset core.logAllRefUpdates &&\n>        git checkout --orphan delta &&\n>        ! test -f .git/logs/refs/heads/delta &&\n> -       test_must_fail PAGER= git reflog show delta &&\n> +       (\n> +               PAGER= &&\n> +               export PAGER &&\n> +               test_must_fail git reflog show delta\n> +       ) &&\n>        git commit -m Delta &&\n>        test -f .git/logs/refs/heads/delta &&\n>        PAGER= git reflog show delta\n> @@ -80,17 +84,29 @@ test_expect_success '--orphan does not make reflog when core.logAllRefUpdates =\n>        git config core.logAllRefUpdates false &&\n>        git checkout --orphan epsilon &&\n>        ! test -f .git/logs/refs/heads/epsilon &&\n> -       test_must_fail PAGER= git reflog show epsilon &&\n> +       (\n> +               PAGER= &&\n> +               export PAGER &&\n> +               test_must_fail git reflog show epsilon\n> +       ) &&\n>        git commit -m Epsilon &&\n>        ! test -f .git/logs/refs/heads/epsilon &&\n> -       test_must_fail PAGER= git reflog show epsilon\n> +       (\n> +               PAGER= &&\n> +               export PAGER &&\n> +               test_must_fail git reflog show epsilon\n> +       )\n>  '\n>\n>  test_expect_success '--orphan with -l makes reflog when core.logAllRefUpdates = false' '\n>        git checkout master &&\n>        git checkout -l --orphan zeta &&\n>        test -f .git/logs/refs/heads/zeta &&\n> -       test_must_fail PAGER= git reflog show zeta &&\n> +       (\n> +               PAGER= &&\n> +               export PAGER &&\n> +               test_must_fail git reflog show zeta\n> +       ) &&\n>        git commit -m Zeta &&\n>        PAGER= git reflog show zeta\n>  '\n> @@ -99,10 +115,18 @@ test_expect_success 'giving up --orphan not committed when -l and core.logAllRef\n>        git checkout master &&\n>        git checkout -l --orphan eta &&\n>        test -f .git/logs/refs/heads/eta &&\n> -       test_must_fail PAGER= git reflog show eta &&\n> +       (\n> +               PAGER= &&\n> +               export PAGER &&\n> +               test_must_fail git reflog show eta\n> +       ) &&\n>        git checkout master &&\n>        ! test -f .git/logs/refs/heads/eta &&\n> -       test_must_fail PAGER= git reflog show eta\n> +       (\n> +               PAGER= &&\n> +               export PAGER &&\n> +               test_must_fail git reflog show eta\n> +       )\n>  '\n>\n>  test_expect_success '--orphan is rejected with an existing name' '\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index 859b99a..bf7747d 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -237,7 +237,11 @@ test_expect_success 'checkout -b does not make reflog when core.logAllRefUpdates\n>        git config core.logAllRefUpdates false &&\n>        git checkout -b beta &&\n>        ! test -f .git/logs/refs/heads/beta &&\n> -       test_must_fail PAGER= git reflog show beta\n> +       (\n> +               PAGER= &&\n> +               export PAGER &&\n> +               test_must_fail git reflog show beta\n> +       )\n>  '\n>\n>  test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates = false' '\n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index 2d67a40..1d82f79 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -693,7 +693,11 @@ test_expect_success 'create note from non-existing note with \"git notes add -c\"\n>        git add a10 &&\n>        test_tick &&\n>        git commit -m 10th &&\n> -       test_must_fail MSG=\"yet another note\" git notes add -c deadbeef &&\n> +       (\n> +               MSG=\"yet another note\" &&\n> +               export MSG &&\n> +               test_must_fail git notes add -c deadbeef\n> +       ) &&\n>        test_must_fail git notes list HEAD\n>  '\n>\n> --\n> 1.6.6.2\n>\n\nI don't like this proposed patch because I don't see an improvement\nwhen:\n* you change a line to three;\n* the original line is clear enough;\n* the resulting lines improves nothing;\n* it is for cosmetic purposes only inside a script;\n* and it uses more resources, even if little more as by creating a\n subshell environment, to do the same.\n\nBest regards\n"},{"id":"145914","messageId":"AANLkTilzC8iMikfBieS_pcChP7_F4hA6bT1ydWHD4etP@mail.gmail.com","threadId":"24444","inReplyTo":"iU5XdZGtMeaspoCqSJIp6Y--60TPVkZUrm3SdW86dsTZkNYZWqbSppLBrMXyL1rVqqYtHm94ACo@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T23:44:23Z","receivedAt":"2010-07-20T23:44:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 20, 2010 at 21:55, Brandon Casey wrote:\n\n> -       test_must_fail PAGER= git reflog show delta &&\n> +       (\n> +               PAGER= &&\n> +               export PAGER &&\n> +               test_must_fail git reflog show delta\n> +       ) &&\n\nYou must use \"export PAGER;\", not \"export PAGER &&\". export doesn't\nreturn zero on all systems when exporting, see previous changes in\nthis regard in t/.\n"},{"id":"145915","messageId":"AANLkTim9hFDUoS-RsPTyaElQW4iC5_3HH66aJ9R-DiQJ@mail.gmail.com","threadId":"24444","inReplyTo":"AANLkTilzC8iMikfBieS_pcChP7_F4hA6bT1ydWHD4etP@mail.gmail.com","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T23:45:11Z","receivedAt":"2010-07-20T23:45:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 20, 2010 at 23:44, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Jul 20, 2010 at 21:55, Brandon Casey wrote:\n>\n>> -       test_must_fail PAGER= git reflog show delta &&\n>> +       (\n>> +               PAGER= &&\n>> +               export PAGER &&\n>> +               test_must_fail git reflog show delta\n>> +       ) &&\n>\n> You must use \"export PAGER;\", not \"export PAGER &&\". export doesn't\n> return zero on all systems when exporting, see previous changes in\n> this regard in t/.\n\nActually, see the t/README docs which explicitly mention this. Yay docs.\n"},{"id":"145916","messageId":"20100721000101.GB4282@burratino","threadId":"24444","inReplyTo":"AANLkTilzC8iMikfBieS_pcChP7_F4hA6bT1ydWHD4etP@mail.gmail.com","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-21T00:01:01Z","receivedAt":"2010-07-21T00:01:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> You must use \"export PAGER;\", not \"export PAGER &&\". export doesn't\n> return zero on all systems when exporting, see previous changes in\n> this regard in t/.\n\nNope. Sorry I missed this before.\n\ndiff --git a/t/README b/t/README\nindex b906ceb..f81998b 100644\n--- a/t/README\n+++ b/t/README\n@@ -259,11 +259,11 @@ Do:\n \ttest ...\n \n That way all of the commands in your tests will succeed or fail. If\n- you must ignore the return value of something (e.g. the return\n- value of export is unportable) it's best to indicate so explicitly\n- with a semicolon:\n+ you must ignore the return value of something (e.g., the return\n+ after unsetting a variable that was already unset is unportable) it's\n+ best to indicate so explicitly with a semicolon:\n \n-\texport HLAGH;\n+\tunset HLAGH;\n \tgit merge hla &&\n \tgit push gh &&\n \ttest ...\n-- \n"},{"id":"145917","messageId":"AANLkTik0gKFfDCmcLZnn4WFCFY3Lb5zRgrzKLAgD5qH6@mail.gmail.com","threadId":"24444","inReplyTo":"20100721000101.GB4282@burratino","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-21T00:09:13Z","receivedAt":"2010-07-21T00:09:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jul 21, 2010 at 00:01, Jonathan Nieder wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> You must use \"export PAGER;\", not \"export PAGER &&\". export doesn't\n>> return zero on all systems when exporting, see previous changes in\n>> this regard in t/.\n>\n> Nope.  Sorry I missed this before.\n>\n> diff --git a/t/README b/t/README\n> index b906ceb..f81998b 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -259,11 +259,11 @@ Do:\n>        test ...\n>\n>    That way all of the commands in your tests will succeed or fail. If\n> -   you must ignore the return value of something (e.g. the return\n> -   value of export is unportable) it's best to indicate so explicitly\n> -   with a semicolon:\n> +   you must ignore the return value of something (e.g., the return\n> +   after unsetting a variable that was already unset is unportable) it's\n> +   best to indicate so explicitly with a semicolon:\n\nWe should have examples for both export and unset, but the prose\nshould mention both IMO\n"},{"id":"145918","messageId":"20100721001402.GE4282@burratino","threadId":"24444","inReplyTo":"AANLkTik0gKFfDCmcLZnn4WFCFY3Lb5zRgrzKLAgD5qH6@mail.gmail.com","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-21T00:14:02Z","receivedAt":"2010-07-21T00:14:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> We should have examples for both export and unset\n\nWhat is unportable for “export” is the effect of exporting an unset\nvariable. I am not even sure whether the return value is unportable,\nbut it doesn’t matter; that is an example of a “Don’t” rather than a\n“Do it this way”.\n\nSee v1.5.6-rc0~61 (filter-branch: fix variable export logic,\n2008-05-13) for an example.\n\n> but the prose\n> should mention both IMO\n\nYes, thanks for putting this portability guide together.\n\nJonathan\n"},{"id":"145919","messageId":"AANLkTil8gyaOcjHGm0tLTdYDdTT4VckUr9nELNiBlKNa@mail.gmail.com","threadId":"24444","inReplyTo":"20100721001402.GE4282@burratino","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-21T00:34:38Z","receivedAt":"2010-07-21T00:34:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jul 21, 2010 at 00:14, Jonathan Nieder wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> We should have examples for both export and unset\n>\n> What is unportable for “export” is the effect of exporting an unset\n> variable.  I am not even sure whether the return value is unportable,\n> but it doesn’t matter; that is an example of a “Don’t” rather than a\n> “Do it this way”.\n\nI didn't know that. It'd be good if it were mentioned in the docs. I\nthought it was just export in general.\n\n> See v1.5.6-rc0~61 (filter-branch: fix variable export logic,\n> 2008-05-13) for an example.\n\nAs an aside, how do you make these 61-commits-after-rc0 commit ids. Is\nthere a sha1->that tool that I haven't spotted?\n\n>> but the prose\n>> should mention both IMO\n>\n> Yes, thanks for putting this portability guide together.\n\nThanks for improving it, this change is a definite improvement, I just\nhad a \"wait, what's the export doing there then?\" question when\nreading it.\n"},{"id":"145921","messageId":"20100721010539.GF4282@burratino","threadId":"24444","inReplyTo":"AANLkTil8gyaOcjHGm0tLTdYDdTT4VckUr9nELNiBlKNa@mail.gmail.com","subject":"git name-rev for fun and profit (Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-21T01:05:39Z","receivedAt":"2010-07-21T01:05:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> As an aside, how do you make these 61-commits-after-rc0 commit ids. Is\n> there a sha1->that tool that I haven't spotted?\n\n GIT_PAGER_IN_USE=1 git log --all-match --grep=shell --grep=export |\n git -p name-rev --tags --stdin\n\nv1.5.6-rc0~61 is 61 commits before rc0 (well, the 61st-generation\ngrandparent using the first parent where there is a choice). The\ndistinction matters because “61 commits _after_ 1.5.6-rc0” is not\nwell-defined.\n\nIf you want the latter sort of description anyway, ‘git describe’\nmight help.\n"},{"id":"145931","messageId":"AANLkTil38MCB--6J0jOPk_0aVTPLWNM-PA318zHZu8xx@mail.gmail.com","threadId":"24444","inReplyTo":"20100721010539.GF4282@burratino","subject":"Re: git name-rev for fun and profit (Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-21T11:32:49Z","receivedAt":"2010-07-21T11:32:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jul 21, 2010 at 01:05, Jonathan Nieder wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> As an aside, how do you make these 61-commits-after-rc0 commit ids. Is\n>> there a sha1->that tool that I haven't spotted?\n>\n>  GIT_PAGER_IN_USE=1 git log --all-match --grep=shell --grep=export |\n>  git -p name-rev --tags --stdin\n>\n> v1.5.6-rc0~61 is 61 commits before rc0 (well, the 61st-generation\n> grandparent using the first parent where there is a choice).  The\n> distinction matters because “61 commits _after_ 1.5.6-rc0” is not\n> well-defined.\n\nThanks, added that as an alias in my .gitconfig:\n\n abbrev-commit = \"!f() { git name-rev --tags \\\"$@\\\" | sed\n's|.*tags/||;s|\\\\([0-9a-f]\\\\{7\\\\}\\\\).*|\\\\1|'; }; f\"\n\n> If you want the latter sort of description anyway, ‘git describe’\n> might help.\n\ngit describe was actually more like what I wanted. I'd used it before,\nbut forgotten it. Thanks.\n"},{"id":"145940","messageId":"20100721152311.GA12726@burratino","threadId":"24444","inReplyTo":"AANLkTinhyFD4RhLLxS-jj-oX5VWqGyy7AiXJ3VJlcU2W@mail.gmail.com","subject":"[PATCH] gitweb: clarify search results page when no matching commit found","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-21T15:23:11Z","receivedAt":"2010-07-21T15:23:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Erick Mattos wrote:\n\n> I did a search in gitweb\n> (http://git.kernel.org/?p=git/git.git;a=summary) by commit\n> e2007832552ccea9befed9003580c494f09e666e.\n> \n> Looks as gitweb's search is broken and giving false results.\n\nMaybe something like the following would help.\n\n-- 8< --\nWhen searching commits for a string that never occurs, the results\npage looks something like this:\n\n\tprojects / foo.git / search \\o/\n\tsummary | ... | tree [commit] search: [ kfjdkas ] [ ]re\n\tfirst ⋅ prev ⋅ next\n\n\tMerge branch 'maint'\n\n\tFoo: a demonstration project\n\nWithout a list of hits to compare it to, the header describing the\ncommit named by the hash parameter (usually HEAD) may itself look\nlike a hit. Add some text (“No match.”) to replace the empty\nlist of hits to avoid this confusion.\n\nNoticed-by: Erick Mattos \nSigned-off-by: Jonathan Nieder \n---\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex cedc357..a47eed2 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6522,12 +6522,13 @@ sub git_search {\n \t\t\t$paging_nav .= \" ⋅ next\";\n \t\t}\n \n-\t\tif ($#commitlist >= 100) {\n-\t\t}\n-\n \t\tgit_print_page_nav('','', $hash,$co{'tree'},$hash, $paging_nav);\n \t\tgit_print_header_div('commit', esc_html($co{'title'}), $hash);\n-\t\tgit_search_grep_body(\\@commitlist, 0, 99, $next_link);\n+\t\tif ($page == 0 && $#commitlist == 0) {\n+\t\t\tprint 'No match.\\n';\n+\t\t} else {\n+\t\t\tgit_search_grep_body(\\@commitlist, 0, 99, $next_link);\n+\t\t}\n \t}\n \n \tif ($searchtype eq 'pickaxe') {\n-- \n"},{"id":"145941","messageId":"QGT_6Gjw_prM4Z_TThjuP3s9CSH7c4P6hnnNpVLNYxcJL6N_1HxuGQ@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":"AANLkTilY0fYxKlWLYGU4f4ZtJzsMSSnIIi0YnywKU5DU@mail.gmail.com","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-07-21T15:32:12Z","receivedAt":"2010-07-21T15:32:12Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 07/20/2010 06:19 PM, Erick Mattos wrote:\n> 2010/7/20 Brandon Casey \n>> No time to investigate, but here is an example patch and the\n>> results of running the affected tests.\n\n> You have made cosmetic changes which do not do the same as the original.\n\nNope, look closer. The changes are not cosmetic.\n\nTry this:\n\n run_it () { \"$@\"; }; run_it foo= true && echo success || echo failure\n\nYou probably get something like this (if you're using bash):\n\n bash: foo=: command not found\n\nThat's because the one-shot variable assignment doesn't work when\nused like this. It also means that the original tests which do:\n\n test_must_fail PAGER= git ...\n\nare broken. We ran into this problem a while back and fixed it in\nthe commit that I referenced (e2007832). I fixed the new instances\nin t2017, t3200, and t3301 in the patch that I sent.\n\nFor the tests in t2017 and t3200 that now fail, the originals seem to\nexpect 'git reflog show' to return non-zero when asked to show the reflog\nfor a ref which doesn't have a log. reflog does not currently return\nnon-zero in this case. Either the tests should be updated to reflect\nthe actual behavior of 'reflog show', or 'reflog show' should be updated\nto return non-zero when passed a ref without a log.\n\n-brandon\n\n>> --->8---\n>> From: Brandon Casey \n>>\n>> See e2007832552ccea9befed9003580c494f09e666e\n>> ---\n>> t/t2017-checkout-orphan.sh | 36 ++++++++++++++++++++++++++++++------\n>> t/t3200-branch.sh | 6 +++++-\n>> t/t3301-notes.sh | 6 +++++-\n>> 3 files changed, 40 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh\n>> index be88d4b..81cb393 100755\n>> --- a/t/t2017-checkout-orphan.sh\n>> +++ b/t/t2017-checkout-orphan.sh\n\n>> @@ -80,17 +84,29 @@ test_expect_success '--orphan does not make reflog when core.logAllRefUpdates =\n>> git config core.logAllRefUpdates false &&\n>> git checkout --orphan epsilon &&\n>> ! test -f .git/logs/refs/heads/epsilon &&\n>> - test_must_fail PAGER= git reflog show epsilon &&\n>> + (\n>> + PAGER= &&\n>> + export PAGER &&\n>> + test_must_fail git reflog show epsilon\n>> + ) &&\n>> git commit -m Epsilon &&\n>> ! test -f .git/logs/refs/heads/epsilon &&\n>> - test_must_fail PAGER= git reflog show epsilon\n>> + (\n>> + PAGER= &&\n>> + export PAGER &&\n>> + test_must_fail git reflog show epsilon\n>> + )\n>> '\n\n>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n>> index 859b99a..bf7747d 100755\n>> --- a/t/t3200-branch.sh\n>> +++ b/t/t3200-branch.sh\n>> @@ -237,7 +237,11 @@ test_expect_success 'checkout -b does not make reflog when core.logAllRefUpdates\n>> git config core.logAllRefUpdates false &&\n>> git checkout -b beta &&\n>> ! test -f .git/logs/refs/heads/beta &&\n>> - test_must_fail PAGER= git reflog show beta\n>> + (\n>> + PAGER= &&\n>> + export PAGER &&\n>> + test_must_fail git reflog show beta\n>> + )\n>> '\n"},{"id":"145947","messageId":"201007211951.18439.jnareb@gmail.com","threadId":"24444","inReplyTo":"20100721152311.GA12726@burratino","subject":"Re: [PATCH] gitweb: clarify search results page when no matching commit found","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-07-21T17:51:16Z","receivedAt":"2010-07-21T17:51:16Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 21 Jul 2010, Jonathan Nieder wrote:\n> Erick Mattos wrote:\n> \n> > I did a search in gitweb\n> > (http://git.kernel.org/?p=git/git.git;a=summary) by commit\n> > e2007832552ccea9befed9003580c494f09e666e.\n> > \n> > Looks as gitweb's search is broken and giving false results.\n> \n> Maybe something like the following would help.\n> \n> -- 8< --\n> When searching commits for a string that never occurs, the results\n> page looks something like this:\n> \n> \tprojects / foo.git / search \\o/\n> \tsummary | ... | tree [commit] search: [ kfjdkas ] [ ]re\n> \tfirst ⋅ prev ⋅ next\n> \n> \tMerge branch 'maint'\n> \n> \tFoo: a demonstration project\n> \n> Without a list of hits to compare it to, the header describing the\n> commit named by the hash parameter (usually HEAD) may itself look\n> like a hit. Add some text (“No match.”) to replace the empty\n> list of hits to avoid this confusion.\n> \n> Noticed-by: Erick Mattos \n> Signed-off-by: Jonathan Nieder \n\nVery good idea\n\nAcked-by: Jakub Narebski \n\n> ---\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index cedc357..a47eed2 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -6522,12 +6522,13 @@ sub git_search {\n> \t\t\t$paging_nav .= \" ⋅ next\";\n> \t\t}\n> \n> -\t\tif ($#commitlist >= 100) {\n> -\t\t}\n> -\n\nP.S. I wonder WTF was that...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"145950","messageId":"7v1vawk50n.fsf@alter.siamese.dyndns.org","threadId":"24444","inReplyTo":"iU5XdZGtMeaspoCqSJIp6Y--60TPVkZUrm3SdW86dsTZkNYZWqbSppLBrMXyL1rVqqYtHm94ACo@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-21T19:29:28Z","receivedAt":"2010-07-21T19:29:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> No time to investigate, but here is an example patch and the\n> results of running the affected tests. Looks like reflog may\n> be creating a reflog when it is not supposed to.\n\nYour later analysis is correct; \"git reflog show \" does not\ncomplain when there is no reflog for , which might or might not be\na bug.\n\nBecause these tests are not about behaviour of \"git reflog show\" command,\nlet's do this for now.\n\nThanks.\n\n-- >8 --\nSubject: tests: correct \"does reflog exist\" tests\n\nThese two tests were not about how \"git reflog show \" exits when\nthere is no reflog, but were about whether \"checkout\" and \"branch\" create\nor not create reflog when creating a new , update the tests to\ncheck it in a more direct way, namely using \"git rev-parse --verify\".\n\nAlso lose tests based on \"test -f .git/logs/refs/heads/\" from\nnearby, to avoid exposing this particular implementation detail\nunnecessarily.\n\n---\n t/t2017-checkout-orphan.sh | 47 +++++++------------------------------------\n t/t3200-branch.sh | 13 ++---------\n 2 files changed, 11 insertions(+), 49 deletions(-)\n\ndiff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh\nindex 81cb393..2d2f63f 100755\n--- a/t/t2017-checkout-orphan.sh\n+++ b/t/t2017-checkout-orphan.sh\n@@ -68,65 +68,34 @@ test_expect_success '--orphan makes reflog by default' '\n \tgit checkout master &&\n \tgit config --unset core.logAllRefUpdates &&\n \tgit checkout --orphan delta &&\n-\t! test -f .git/logs/refs/heads/delta &&\n-\t(\n-\t\tPAGER= &&\n-\t\texport PAGER &&\n-\t\ttest_must_fail git reflog show delta\n-\t) &&\n+\ttest_must_fail git rev-parse --verify delta@{0} &&\n \tgit commit -m Delta &&\n-\ttest -f .git/logs/refs/heads/delta &&\n-\tPAGER= git reflog show delta\n+\tgit rev-parse --verify delta@{0}\n '\n \n test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '\n \tgit checkout master &&\n \tgit config core.logAllRefUpdates false &&\n \tgit checkout --orphan epsilon &&\n-\t! test -f .git/logs/refs/heads/epsilon &&\n-\t(\n-\t\tPAGER= &&\n-\t\texport PAGER &&\n-\t\ttest_must_fail git reflog show epsilon\n-\t) &&\n+\ttest_must_fail git rev-parse --verify epsilon@{0} &&\n \tgit commit -m Epsilon &&\n-\t! test -f .git/logs/refs/heads/epsilon &&\n-\t(\n-\t\tPAGER= &&\n-\t\texport PAGER &&\n-\t\ttest_must_fail git reflog show epsilon\n-\t)\n+\ttest_must_fail git rev-parse --verify epsilon@{0}\n '\n \n test_expect_success '--orphan with -l makes reflog when core.logAllRefUpdates = false' '\n \tgit checkout master &&\n \tgit checkout -l --orphan zeta &&\n-\ttest -f .git/logs/refs/heads/zeta &&\n-\t(\n-\t\tPAGER= &&\n-\t\texport PAGER &&\n-\t\ttest_must_fail git reflog show zeta\n-\t) &&\n+\ttest_must_fail git rev-parse --verify zeta@{0} &&\n \tgit commit -m Zeta &&\n-\tPAGER= git reflog show zeta\n+\tgit rev-parse --verify zeta@{0}\n '\n \n test_expect_success 'giving up --orphan not committed when -l and core.logAllRefUpdates = false deletes reflog' '\n \tgit checkout master &&\n \tgit checkout -l --orphan eta &&\n-\ttest -f .git/logs/refs/heads/eta &&\n-\t(\n-\t\tPAGER= &&\n-\t\texport PAGER &&\n-\t\ttest_must_fail git reflog show eta\n-\t) &&\n+\ttest_must_fail git rev-parse --verify eta@{0} &&\n \tgit checkout master &&\n-\t! test -f .git/logs/refs/heads/eta &&\n-\t(\n-\t\tPAGER= &&\n-\t\texport PAGER &&\n-\t\ttest_must_fail git reflog show eta\n-\t)\n+\ttest_must_fail git rev-parse --verify eta@{0}\n '\n \n test_expect_success '--orphan is rejected with an existing name' '\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex bf7747d..f54a533 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -228,28 +228,21 @@ test_expect_success 'checkout -b makes reflog by default' '\n \tgit checkout master &&\n \tgit config --unset core.logAllRefUpdates &&\n \tgit checkout -b alpha &&\n-\ttest -f .git/logs/refs/heads/alpha &&\n-\tPAGER= git reflog show alpha\n+\tgit rev-parse --verify alpha@{0}\n '\n \n test_expect_success 'checkout -b does not make reflog when core.logAllRefUpdates = false' '\n \tgit checkout master &&\n \tgit config core.logAllRefUpdates false &&\n \tgit checkout -b beta &&\n-\t! test -f .git/logs/refs/heads/beta &&\n-\t(\n-\t\tPAGER= &&\n-\t\texport PAGER &&\n-\t\ttest_must_fail git reflog show beta\n-\t)\n+\ttest_must_fail git rev-parse --verify beta@{0}\n '\n \n test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates = false' '\n \tgit checkout master &&\n \tgit checkout -lb gamma &&\n \tgit config --unset core.logAllRefUpdates &&\n-\ttest -f .git/logs/refs/heads/gamma &&\n-\tPAGER= git reflog show gamma\n+\tgit rev-parse --verify gamma@{0}\n '\n \n test_expect_success 'avoid ambiguous track' '\n"},{"id":"145952","messageId":"20100721195036.GA1825@burratino","threadId":"24444","inReplyTo":"201007211951.18439.jnareb@gmail.com","subject":"[PATCH v2] gitweb: clarify search results page when no matching commit found","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-21T19:50:36Z","receivedAt":"2010-07-21T19:50:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When searching commits for a string that never occurs, the results\npage looks something like this:\n\n\tprojects / foo.git / search \\o/\n\tsummary | ... | tree [commit] search: [ kfjdkas ] [ ]re\n\tfirst ⋅ prev ⋅ next\n\n\tMerge branch 'maint'\n\n\tFoo: a demonstration project\n\nWithout a list of hits to compare it to, the header describing the\ncommit named by the hash parameter (usually HEAD) may itself look\nlike a hit. Add some text (“No match.”) to replace the empty\nlist of hits to avoid this confusion.\n\nWhile at it, remove some nearby dead code, left behind from a\nsimplification a few years ago (v1.5.4-rc0~276^2~4, 2007-11-01).\n\nNoticed-by: Erick Mattos \nSigned-off-by: Jonathan Nieder \nAcked-by: Jakub Narebski \n---\nJakub Narebski wrote:\n\n> Very good idea\n\nShoddy implementation, though. Apparently (yes, I should know this;\nand testing reminded me before I sent the wrong patch) the index of\nthe last element ($#array) of an empty array is -1, not 0.\n\nSorry about that.\n\n gitweb/gitweb.perl | 11 +++++++----\n 1 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex cedc357..801575d 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -6522,12 +6522,13 @@ sub git_search {\n \t\t\t$paging_nav .= \" ⋅ next\";\n \t\t}\n \n-\t\tif ($#commitlist >= 100) {\n-\t\t}\n-\n \t\tgit_print_page_nav('','', $hash,$co{'tree'},$hash, $paging_nav);\n \t\tgit_print_header_div('commit', esc_html($co{'title'}), $hash);\n-\t\tgit_search_grep_body(\\@commitlist, 0, 99, $next_link);\n+\t\tif ($page == 0 && !@commitlist) {\n+\t\t\tprint \"

No match.\\n

\";\n+\t\t} else {\n+\t\t\tgit_search_grep_body(\\@commitlist, 0, 99, $next_link);\n+\t\t}\n \t}\n \n \tif ($searchtype eq 'pickaxe') {\n-- \n"},{"id":"145974","messageId":"AANLkTikUs2kSqeNxzCW0fag4K_4yuQSX1JKwk4pFd0Ni@mail.gmail.com","threadId":"24444","inReplyTo":"QGT_6Gjw_prM4Z_TThjuP3s9CSH7c4P6hnnNpVLNYxcJL6N_1HxuGQ@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Erick Mattos","fromEmail":"erick.mattos@gmail.com","sentAt":"2010-07-22T00:28:16Z","receivedAt":"2010-07-22T00:28:16Z","isPatch":true,"sender":{"key":"erick.mattos@gmail.com","avatar":"https://avatars.githubusercontent.com/u/134001?v=4"},"body":"Hi,\n\n2010/7/21 Brandon Casey :\n> On 07/20/2010 06:19 PM, Erick Mattos wrote:\n>> 2010/7/20 Brandon Casey \n>>> No time to investigate, but here is an example patch and the\n>>> results of running the affected tests.\n>\n>> You have made cosmetic changes which do not do the same as the original.\n>?\n> Nope, look closer.  The changes are not cosmetic.\n\nNow I see it. I was not completely aware of the problem, only of your\nemail. Maybe I should subscribe to the list at last... ;-D\n\n> Try this:\n>\n>   run_it () { \"$@\"; }; run_it foo= true && echo success || echo failure\n>\n> You probably get something like this (if you're using bash):\n>\n>   bash: foo=: command not found\n\nIt would work if it was like:\n\nrun_it () { eval \"$@\"; }; run_it \"foo= true\" && echo success || echo failure\n\nBut I think your approach is better shaped for a fast solution.\n\n> That's because the one-shot variable assignment doesn't work when\n> used like this.  It also means that the original tests which do:\n>\n>   test_must_fail PAGER= git ...\n>\n> are broken.  We ran into this problem a while back and fixed it in\n> the commit that I referenced (e2007832).  I fixed the new instances\n> in t2017, t3200, and t3301 in the patch that I sent.\n>\n> For the tests in t2017 and t3200 that now fail, the originals seem to\n> expect 'git reflog show' to return non-zero when asked to show the reflog\n> for a ref which doesn't have a log.  reflog does not currently return\n> non-zero in this case.  Either the tests should be updated to reflect\n> the actual behavior of 'reflog show', or 'reflog show' should be updated\n> to return non-zero when passed a ref without a log.\n\nSo fixing it is a need.\n\nOn t2017 and t3200 the problem that appeared was because no message\nand error had been generated when there is no reflog. Git reflog when\nrun on a ref with a nonexistent reflog file exits with 0 saying\nnothing.\n\nI don't see this as a correct behavior but as those tests were just to\nenforce the previous \"! test -f...\" test which is already enough for\nchecking the intended behavior then I would think it was good enough\njust to wipe out the \"problematic git reflog\" commands until deciding\nhow quiet git reflog command should be when there is no reflog file to\nshow.\n\nAlthough I just realized Junio sent an email following this thread and\nI bet he could give a better solution or his directions. Going to\nread it now.\n\nThanks for your clarifications.\n\nRegards\n"},{"id":"145976","messageId":"AANLkTin5NPzFwMuQYcdaTaCEqmlrzRyIpEl7btV1LDRS@mail.gmail.com","threadId":"24444","inReplyTo":"7v1vawk50n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Erick Mattos","fromEmail":"erick.mattos@gmail.com","sentAt":"2010-07-22T00:32:01Z","receivedAt":"2010-07-22T00:32:01Z","isPatch":true,"sender":{"key":"erick.mattos@gmail.com","avatar":"https://avatars.githubusercontent.com/u/134001?v=4"},"body":"Hi,\n\n2010/7/21 Junio C Hamano :\n> Brandon Casey writes:\n>\n>> No time to investigate, but here is an example patch and the\n>> results of running the affected tests.  Looks like reflog may\n>> be creating a reflog when it is not supposed to.\n>\n> Your later analysis is correct; \"git reflog show \" does not\n> complain when there is no reflog for , which might or might not be\n> a bug.\n\nI think is not a bug but almost one.\n\n> Because these tests are not about behaviour of \"git reflog show\" command,\n> let's do this for now.\n>\n> Thanks.\n>\n> -- >8 --\n> Subject: tests: correct \"does reflog exist\" tests\n>\n> These two tests were not about how \"git reflog show \" exits when\n> there is no reflog, but were about whether \"checkout\" and \"branch\" create\n> or not create reflog when creating a new , update the tests to\n> check it in a more direct way, namely using \"git rev-parse --verify\".\n>\n> Also lose tests based on \"test -f .git/logs/refs/heads/\" from\n> nearby, to avoid exposing this particular implementation detail\n> unnecessarily.\n\nQuite pretty. Clean and optimized solution. Better this way.\n\nRegards\n"},{"id":"146005","messageId":"8myTzOiYB3ju-PTbbc8uT5rsI-P8hyc8F7UbDwbNzIh4Pa3k2_Uh2A@cipher.nrlssc.navy.mil","threadId":"24444","inReplyTo":"7v1vawk50n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-07-22T18:21:10Z","receivedAt":"2010-07-22T18:21:10Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"\nbtw, this works and is a definite improvement.\n\n-Brandon\n\nJunio C Hamano wrote:\n> Brandon Casey writes:\n> \n>> No time to investigate, but here is an example patch and the\n>> results of running the affected tests. Looks like reflog may\n>> be creating a reflog when it is not supposed to.\n> \n> Your later analysis is correct; \"git reflog show \" does not\n> complain when there is no reflog for , which might or might not be\n> a bug.\n> \n> Because these tests are not about behaviour of \"git reflog show\" command,\n> let's do this for now.\n> \n> Thanks.\n> \n> -- >8 --\n> Subject: tests: correct \"does reflog exist\" tests\n> \n> These two tests were not about how \"git reflog show \" exits when\n> there is no reflog, but were about whether \"checkout\" and \"branch\" create\n> or not create reflog when creating a new , update the tests to\n> check it in a more direct way, namely using \"git rev-parse --verify\".\n> \n> Also lose tests based on \"test -f .git/logs/refs/heads/\" from\n> nearby, to avoid exposing this particular implementation detail\n> unnecessarily.\n> \n> ---\n> t/t2017-checkout-orphan.sh | 47 +++++++------------------------------------\n> t/t3200-branch.sh | 13 ++---------\n> 2 files changed, 11 insertions(+), 49 deletions(-)\n> \n> diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh\n> index 81cb393..2d2f63f 100755\n> --- a/t/t2017-checkout-orphan.sh\n> +++ b/t/t2017-checkout-orphan.sh\n> @@ -68,65 +68,34 @@ test_expect_success '--orphan makes reflog by default' '\n> \tgit checkout master &&\n> \tgit config --unset core.logAllRefUpdates &&\n> \tgit checkout --orphan delta &&\n> -\t! test -f .git/logs/refs/heads/delta &&\n> -\t(\n> -\t\tPAGER= &&\n> -\t\texport PAGER &&\n> -\t\ttest_must_fail git reflog show delta\n> -\t) &&\n> +\ttest_must_fail git rev-parse --verify delta@{0} &&\n> \tgit commit -m Delta &&\n> -\ttest -f .git/logs/refs/heads/delta &&\n> -\tPAGER= git reflog show delta\n> +\tgit rev-parse --verify delta@{0}\n> '\n> \n> test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '\n> \tgit checkout master &&\n> \tgit config core.logAllRefUpdates false &&\n> \tgit checkout --orphan epsilon &&\n> -\t! test -f .git/logs/refs/heads/epsilon &&\n> -\t(\n> -\t\tPAGER= &&\n> -\t\texport PAGER &&\n> -\t\ttest_must_fail git reflog show epsilon\n> -\t) &&\n> +\ttest_must_fail git rev-parse --verify epsilon@{0} &&\n> \tgit commit -m Epsilon &&\n> -\t! test -f .git/logs/refs/heads/epsilon &&\n> -\t(\n> -\t\tPAGER= &&\n> -\t\texport PAGER &&\n> -\t\ttest_must_fail git reflog show epsilon\n> -\t)\n> +\ttest_must_fail git rev-parse --verify epsilon@{0}\n> '\n> \n> test_expect_success '--orphan with -l makes reflog when core.logAllRefUpdates = false' '\n> \tgit checkout master &&\n> \tgit checkout -l --orphan zeta &&\n> -\ttest -f .git/logs/refs/heads/zeta &&\n> -\t(\n> -\t\tPAGER= &&\n> -\t\texport PAGER &&\n> -\t\ttest_must_fail git reflog show zeta\n> -\t) &&\n> +\ttest_must_fail git rev-parse --verify zeta@{0} &&\n> \tgit commit -m Zeta &&\n> -\tPAGER= git reflog show zeta\n> +\tgit rev-parse --verify zeta@{0}\n> '\n> \n> test_expect_success 'giving up --orphan not committed when -l and core.logAllRefUpdates = false deletes reflog' '\n> \tgit checkout master &&\n> \tgit checkout -l --orphan eta &&\n> -\ttest -f .git/logs/refs/heads/eta &&\n> -\t(\n> -\t\tPAGER= &&\n> -\t\texport PAGER &&\n> -\t\ttest_must_fail git reflog show eta\n> -\t) &&\n> +\ttest_must_fail git rev-parse --verify eta@{0} &&\n> \tgit checkout master &&\n> -\t! test -f .git/logs/refs/heads/eta &&\n> -\t(\n> -\t\tPAGER= &&\n> -\t\texport PAGER &&\n> -\t\ttest_must_fail git reflog show eta\n> -\t)\n> +\ttest_must_fail git rev-parse --verify eta@{0}\n> '\n> \n> test_expect_success '--orphan is rejected with an existing name' '\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index bf7747d..f54a533 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -228,28 +228,21 @@ test_expect_success 'checkout -b makes reflog by default' '\n> \tgit checkout master &&\n> \tgit config --unset core.logAllRefUpdates &&\n> \tgit checkout -b alpha &&\n> -\ttest -f .git/logs/refs/heads/alpha &&\n> -\tPAGER= git reflog show alpha\n> +\tgit rev-parse --verify alpha@{0}\n> '\n> \n> test_expect_success 'checkout -b does not make reflog when core.logAllRefUpdates = false' '\n> \tgit checkout master &&\n> \tgit config core.logAllRefUpdates false &&\n> \tgit checkout -b beta &&\n> -\t! test -f .git/logs/refs/heads/beta &&\n> -\t(\n> -\t\tPAGER= &&\n> -\t\texport PAGER &&\n> -\t\ttest_must_fail git reflog show beta\n> -\t)\n> +\ttest_must_fail git rev-parse --verify beta@{0}\n> '\n> \n> test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates = false' '\n> \tgit checkout master &&\n> \tgit checkout -lb gamma &&\n> \tgit config --unset core.logAllRefUpdates &&\n> -\ttest -f .git/logs/refs/heads/gamma &&\n> -\tPAGER= git reflog show gamma\n> +\tgit rev-parse --verify gamma@{0}\n> '\n> \n> test_expect_success 'avoid ambiguous track' '\n"}]}