{"thread":{"id":"23548","subject":"[PATCH] t1304,t2007: quell output to stdout and stderr","startedAt":"2010-04-21T13:55:40Z","lastAt":"2010-05-26T09:08:15Z","messageCount":6,"participants":["Michael J Gruber","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"140029","messageId":"00e68c9727f8dd3426db6f78d6b583a0dcec4d13.1271858119.git.git@drmicha.warpmail.net","threadId":"23548","inReplyTo":null,"subject":"[PATCH] t1304,t2007: quell output to stdout and stderr","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-04-21T13:55:40Z","receivedAt":"2010-04-21T13:55:40Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"These tests send output to stdout or stderr even without -v. This is\ndistracting because unexpected output flashing by during make test\nusually indicates problems\n\nShut them up uncoditionally: In both cases, the output was due to\nintermediate commands in between the actual test cases.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n t/t1304-default-acl.sh      |    2 +-\n t/t2007-checkout-symlink.sh |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex 055ad00..b26d2e8 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -15,7 +15,7 @@ umask 077\n # is a good candidate: exists on all unices, and it has permission\n # anyway, so we don't create a security hole running the testsuite.\n \n-if ! setfacl -m u:root:rwx .; then\n+if ! setfacl -m u:root:rwx . 2>/dev/null; then\n     say \"Skipping ACL tests: unable to use setfacl\"\n     test_done\n fi\ndiff --git a/t/t2007-checkout-symlink.sh b/t/t2007-checkout-symlink.sh\nindex 20f3343..fc5db05 100755\n--- a/t/t2007-checkout-symlink.sh\n+++ b/t/t2007-checkout-symlink.sh\n@@ -45,7 +45,7 @@ test_expect_success 'switch from symlink to dir' '\n '\n \n rm -fr frotz xyzzy nitfol &&\n-git checkout -f master || exit\n+git checkout -q -f master || exit\n \n test_expect_success 'switch from dir to symlink' '\n \n-- \n1.7.1.rc1.248.gcefbb\n"},{"id":"140033","messageId":"20100421144535.GA8481@progeny.tock","threadId":"23548","inReplyTo":"00e68c9727f8dd3426db6f78d6b583a0dcec4d13.1271858119.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] t1304,t2007: quell output to stdout and stderr","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-21T14:45:35Z","receivedAt":"2010-04-21T14:45:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Michael,\n\nMichael J Gruber wrote:\n\n> --- a/t/t1304-default-acl.sh\n> +++ b/t/t1304-default-acl.sh\n> @@ -15,7 +15,7 @@ umask 077\n>  # is a good candidate: exists on all unices, and it has permission\n>  # anyway, so we don't create a security hole running the testsuite.\n>  \n> -if ! setfacl -m u:root:rwx .; then\n> +if ! setfacl -m u:root:rwx . 2>/dev/null; then\n>      say \"Skipping ACL tests: unable to use setfacl\"\n>      test_done\n>  fi\n[and a similar suppression of ‘git checkout’ output]\n\nIn the spirit of commit 4a45f7d (Use test_expect_success for test\nsetups, 2010-03-20), might it make sense to turn these into tests?\n\nI am imagining something like this.\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex b26d2e8..8b3ff7a 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -15,7 +15,15 @@ umask 077\n # is a good candidate: exists on all unices, and it has permission\n # anyway, so we don't create a security hole running the testsuite.\n \n-if ! setfacl -m u:root:rwx . 2>/dev/null; then\n+test_expect_success 'Setup: try to set an ACL' '\n+\tif setfacl -m u:root:rwx .\n+\tthen\n+\t\ttest_set_prereq ACL\n+\tfi\n+'\n+\n+if ! test_have_prereq ACL\n+then\n     say \"Skipping ACL tests: unable to use setfacl\"\n     test_done\n fi\ndiff --git a/t/t2007-checkout-symlink.sh b/t/t2007-checkout-symlink.sh\nindex fc5db05..f8f40e5 100755\n--- a/t/t2007-checkout-symlink.sh\n+++ b/t/t2007-checkout-symlink.sh\n@@ -44,11 +44,12 @@ test_expect_success 'switch from symlink to dir' '\n \n '\n \n-rm -fr frotz xyzzy nitfol &&\n-git checkout -q -f master || exit\n-\n test_expect_success 'switch from dir to symlink' '\n \n+\trm -fr frotz xyzzy nitfol &&\n+\tgit rm -fr . &&\n+\tgit checkout -f master &&\n+\n \tgit checkout side\n \n '\n"},{"id":"140035","messageId":"4BCF1160.6020304@drmicha.warpmail.net","threadId":"23548","inReplyTo":"20100421144535.GA8481@progeny.tock","subject":"Re: [PATCH] t1304,t2007: quell output to stdout and stderr","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-04-21T14:53:20Z","receivedAt":"2010-04-21T14:53:20Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jonathan Nieder venit, vidit, dixit 21.04.2010 16:45:\n> Hi Michael,\n> \n> Michael J Gruber wrote:\n> \n>> --- a/t/t1304-default-acl.sh\n>> +++ b/t/t1304-default-acl.sh\n>> @@ -15,7 +15,7 @@ umask 077\n>>  # is a good candidate: exists on all unices, and it has permission\n>>  # anyway, so we don't create a security hole running the testsuite.\n>>  \n>> -if ! setfacl -m u:root:rwx .; then\n>> +if ! setfacl -m u:root:rwx . 2>/dev/null; then\n>>      say \"Skipping ACL tests: unable to use setfacl\"\n>>      test_done\n>>  fi\n> [and a similar suppression of ‘git checkout’ output]\n> \n> In the spirit of commit 4a45f7d (Use test_expect_success for test\n> setups, 2010-03-20), might it make sense to turn these into tests?\n\nIn principle yes, of course, I've done this in other cases. But note\nthat here, on both occasions, the test script wants to exit if\nprerequisites are not met. I don't think it's OK to exit or test_done\nfrom within test_expect_something, is it?\n\n> I am imagining something like this.\n> \n> diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\n> index b26d2e8..8b3ff7a 100755\n> --- a/t/t1304-default-acl.sh\n> +++ b/t/t1304-default-acl.sh\n> @@ -15,7 +15,15 @@ umask 077\n>  # is a good candidate: exists on all unices, and it has permission\n>  # anyway, so we don't create a security hole running the testsuite.\n>  \n> -if ! setfacl -m u:root:rwx . 2>/dev/null; then\n> +test_expect_success 'Setup: try to set an ACL' '\n> +\tif setfacl -m u:root:rwx .\n> +\tthen\n> +\t\ttest_set_prereq ACL\n> +\tfi\n> +'\n> +\n> +if ! test_have_prereq ACL\n> +then\n>      say \"Skipping ACL tests: unable to use setfacl\"\n>      test_done\n>  fi\n> diff --git a/t/t2007-checkout-symlink.sh b/t/t2007-checkout-symlink.sh\n> index fc5db05..f8f40e5 100755\n> --- a/t/t2007-checkout-symlink.sh\n> +++ b/t/t2007-checkout-symlink.sh\n> @@ -44,11 +44,12 @@ test_expect_success 'switch from symlink to dir' '\n>  \n>  '\n>  \n> -rm -fr frotz xyzzy nitfol &&\n> -git checkout -q -f master || exit\n> -\n>  test_expect_success 'switch from dir to symlink' '\n>  \n> +\trm -fr frotz xyzzy nitfol &&\n> +\tgit rm -fr . &&\n> +\tgit checkout -f master &&\n> +\n>  \tgit checkout side\n>  \n>  '\n"},{"id":"140037","messageId":"20100421151236.GB8726@progeny.tock","threadId":"23548","inReplyTo":"4BCF1160.6020304@drmicha.warpmail.net","subject":"Re: [PATCH] t1304,t2007: quell output to stdout and stderr","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-21T15:12:36Z","receivedAt":"2010-04-21T15:12:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael J Gruber wrote:\n\n> I don't think it's OK to exit or test_done\n> from within test_expect_something, is it?\n\nexit: the consequences are the same whether you’re in a test or not.\nEither way, it’s bad.  I think the intent of that ‘exit’ was to make a\ngood effort to set up for the next test and skip the test if that\ndidn’t work, which is a bit old-fashioned.  Emptying the index before\nthe checkout would make it like that the setup should not fail even if\nsome related bug resurfaces.\n\ntest_done: not sure what would happen.  Indeed, it doesn’t sound like\na very sane thing to do. :)\n\nJonathan\n"},{"id":"140159","messageId":"5ef48a01f29730a24b74d599bb9b89e77591decf.1271969046.git.git@drmicha.warpmail.net","threadId":"23548","inReplyTo":"20100421151236.GB8726@progeny.tock","subject":"[PATCH v2] t1304,t2007: quell output to stdout and stderr","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-04-22T20:45:23Z","receivedAt":"2010-04-22T20:45:23Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"These tests send output to stdout or stderr even without -v. This is\ndistracting because unexpected output flashing by during make test\nusually indicates problems.\n\nShut them up unconditionally by integrating them in test code: In both\ncases, the output was due to intermediate commands in between the actual\ntest cases.\n\nHelped-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n t/t1304-default-acl.sh      |   10 +++++++++-\n t/t2007-checkout-symlink.sh |    5 ++---\n 2 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex 055ad00..8b3ff7a 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -15,7 +15,15 @@ umask 077\n # is a good candidate: exists on all unices, and it has permission\n # anyway, so we don't create a security hole running the testsuite.\n \n-if ! setfacl -m u:root:rwx .; then\n+test_expect_success 'Setup: try to set an ACL' '\n+\tif setfacl -m u:root:rwx .\n+\tthen\n+\t\ttest_set_prereq ACL\n+\tfi\n+'\n+\n+if ! test_have_prereq ACL\n+then\n     say \"Skipping ACL tests: unable to use setfacl\"\n     test_done\n fi\ndiff --git a/t/t2007-checkout-symlink.sh b/t/t2007-checkout-symlink.sh\nindex 20f3343..e98ec4c 100755\n--- a/t/t2007-checkout-symlink.sh\n+++ b/t/t2007-checkout-symlink.sh\n@@ -44,11 +44,10 @@ test_expect_success 'switch from symlink to dir' '\n \n '\n \n-rm -fr frotz xyzzy nitfol &&\n-git checkout -f master || exit\n-\n test_expect_success 'switch from dir to symlink' '\n \n+\trm -fr frotz xyzzy nitfol &&\n+\tgit checkout -f master &&\n \tgit checkout side\n \n '\n-- \n1.7.1.rc1.248.gcefbb\n"},{"id":"142357","messageId":"4BFCE4FF.60500@drmicha.warpmail.net","threadId":"23548","inReplyTo":"5ef48a01f29730a24b74d599bb9b89e77591decf.1271969046.git.git@drmicha.warpmail.net","subject":"Re: [PATCH v2] t1304,t2007: quell output to stdout and stderr","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-05-26T09:08:15Z","receivedAt":"2010-05-26T09:08:15Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Michael J Gruber venit, vidit, dixit 22.04.2010 22:45:\n> These tests send output to stdout or stderr even without -v. This is\n> distracting because unexpected output flashing by during make test\n> usually indicates problems.\n> \n> Shut them up unconditionally by integrating them in test code: In both\n> cases, the output was due to intermediate commands in between the actual\n> test cases.\n> \n> Helped-by: Jonathan Nieder <jrnieder@gmail.com>\n> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n> ---\n\nping...\n"}]}