{"thread":{"id":"27200","subject":"[PATCH] t/test-lib.sh: minor readability improvements","startedAt":"2011-04-27T12:49:37Z","lastAt":"2011-04-29T16:28:21Z","messageCount":5,"participants":["Mathias Lafeldt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"166516","messageId":"4DB810E1.3080102@debugon.org","threadId":"27200","inReplyTo":null,"subject":"[PATCH] t/test-lib.sh: minor readability improvements","fromName":"Mathias Lafeldt","fromEmail":"misfire@debugon.org","sentAt":"2011-04-27T12:49:37Z","receivedAt":"2011-04-27T12:49:37Z","isPatch":true,"sender":{"key":"misfire@debugon.org","avatar":"https://avatars.githubusercontent.com/u/158074?v=4"},"body":"Tweak/apply parameter expansion. Also use here document to save\ntest results instead of appending each line with \">>\".\n\nSigned-off-by: Mathias Lafeldt \n---\n t/test-lib.sh | 18 ++++++++++--------\n 1 files changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex abc47f3..b30725f 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -24,7 +24,7 @@ done,*)\n *' --tee '*|*' --va'*)\n \tmkdir -p test-results\n \tBASE=test-results/$(basename \"$0\" .sh)\n-\t(GIT_TEST_TEE_STARTED=done ${SHELL-sh} \"$0\" \"$@\" 2>&1;\n+\t(GIT_TEST_TEE_STARTED=done ${SHELL-\"sh\"} \"$0\" \"$@\" 2>&1;\n \t echo $? > $BASE.exit) | tee $BASE.out\n \ttest \"$(cat $BASE.exit)\" = 0\n \texit\n@@ -575,7 +575,7 @@ test_external () {\n test_external_without_stderr () {\n \t# The temporary file has no (and must have no) security\n \t# implications.\n-\ttmp=\"$TMPDIR\"; if [ -z \"$tmp\" ]; then tmp=/tmp; fi\n+\ttmp=${TMPDIR:-\"/tmp\"}\n \tstderr=\"$tmp/git-external-stderr.$$.tmp\"\n \ttest_external \"$@\" 4> \"$stderr\"\n \t[ -f \"$stderr\" ] || error \"Internal error: $stderr disappeared.\"\n@@ -801,12 +801,14 @@ test_done () {\n \t\tmkdir -p \"$test_results_dir\"\n \t\ttest_results_path=\"$test_results_dir/${0%.sh}-$$.counts\"\n \n-\t\techo \"total $test_count\" >> $test_results_path\n-\t\techo \"success $test_success\" >> $test_results_path\n-\t\techo \"fixed $test_fixed\" >> $test_results_path\n-\t\techo \"broken $test_broken\" >> $test_results_path\n-\t\techo \"failed $test_failure\" >> $test_results_path\n-\t\techo \"\" >> $test_results_path\n+\t\tcat >> \"$test_results_path\" < writes:\n\n> Tweak/apply parameter expansion. Also use here document to save\n> test results instead of appending each line with \">>\".\n\nThanks. A few minor nits.\n\n> Signed-off-by: Mathias Lafeldt \n> ---\n> t/test-lib.sh | 18 ++++++++++--------\n> 1 files changed, 10 insertions(+), 8 deletions(-)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index abc47f3..b30725f 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -24,7 +24,7 @@ done,*)\n> *' --tee '*|*' --va'*)\n> \tmkdir -p test-results\n> \tBASE=test-results/$(basename \"$0\" .sh)\n> -\t(GIT_TEST_TEE_STARTED=done ${SHELL-sh} \"$0\" \"$@\" 2>&1;\n> +\t(GIT_TEST_TEE_STARTED=done ${SHELL-\"sh\"} \"$0\" \"$@\" 2>&1;\n\nLooks unnecessary. Superstition?\n\n> \t echo $? > $BASE.exit) | tee $BASE.out\n> \ttest \"$(cat $BASE.exit)\" = 0\n> \texit\n> @@ -575,7 +575,7 @@ test_external () {\n> test_external_without_stderr () {\n> \t# The temporary file has no (and must have no) security\n> \t# implications.\n> -\ttmp=\"$TMPDIR\"; if [ -z \"$tmp\" ]; then tmp=/tmp; fi\n> +\ttmp=${TMPDIR:-\"/tmp\"}\n> \tstderr=\"$tmp/git-external-stderr.$$.tmp\"\n> \ttest_external \"$@\" 4> \"$stderr\"\n> \t[ -f \"$stderr\" ] || error \"Internal error: $stderr disappeared.\"\n> @@ -801,12 +801,14 @@ test_done () {\n> \t\tmkdir -p \"$test_results_dir\"\n> \t\ttest_results_path=\"$test_results_dir/${0%.sh}-$$.counts\"\n> \n> -\t\techo \"total $test_count\" >> $test_results_path\n> -\t\techo \"success $test_success\" >> $test_results_path\n> -\t\techo \"fixed $test_fixed\" >> $test_results_path\n> -\t\techo \"broken $test_broken\" >> $test_results_path\n> -\t\techo \"failed $test_failure\" >> $test_results_path\n> -\t\techo \"\" >> $test_results_path\n> +\t\tcat >> \"$test_results_path\" < +total $test_count\n> +success $test_success\n> +fixed $test_fixed\n> +broken $test_broken\n> +failed $test_failure\n> +\n> +EOF\n\nIt may probably be even easier to read if you indented the whole thing,\nusing the dash before the here-doc marker, like so:\n\n\tcat >>\"$test_results_path\" <<-EOF\n\ttotal $test_count\n success $test_success\n ...\n EOF\n"},{"id":"166727","messageId":"4DBA8C43.4040804@debugon.org","threadId":"27200","inReplyTo":"7vmxjb8uyu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t/test-lib.sh: minor readability improvements","fromName":"Mathias Lafeldt","fromEmail":"misfire@debugon.org","sentAt":"2011-04-29T10:00:35Z","receivedAt":"2011-04-29T10:00:35Z","isPatch":true,"sender":{"key":"misfire@debugon.org","avatar":"https://avatars.githubusercontent.com/u/158074?v=4"},"body":"On 04/27/2011 07:28 PM, Junio C Hamano wrote:\n> Mathias Lafeldt writes:\n> \n>> Tweak/apply parameter expansion. Also use here document to save\n>> test results instead of appending each line with \">>\".\n> \n> Thanks. A few minor nits.\n> \n>> Signed-off-by: Mathias Lafeldt \n>> ---\n>> t/test-lib.sh | 18 ++++++++++--------\n>> 1 files changed, 10 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/t/test-lib.sh b/t/test-lib.sh\n>> index abc47f3..b30725f 100644\n>> --- a/t/test-lib.sh\n>> +++ b/t/test-lib.sh\n>> @@ -24,7 +24,7 @@ done,*)\n>> *' --tee '*|*' --va'*)\n>> \tmkdir -p test-results\n>> \tBASE=test-results/$(basename \"$0\" .sh)\n>> -\t(GIT_TEST_TEE_STARTED=done ${SHELL-sh} \"$0\" \"$@\" 2>&1;\n>> +\t(GIT_TEST_TEE_STARTED=done ${SHELL-\"sh\"} \"$0\" \"$@\" 2>&1;\n> \n> Looks unnecessary. Superstition?\n\nIMHO, ${SHELL-sh} is kind of hard to read, especially when you\ndon't know about the ${parameter-word} format. In hindsight, I\nunderstand that the change isn't really necessary.\n \n>> \t echo $? > $BASE.exit) | tee $BASE.out\n>> \ttest \"$(cat $BASE.exit)\" = 0\n>> \texit\n>> @@ -575,7 +575,7 @@ test_external () {\n>> test_external_without_stderr () {\n>> \t# The temporary file has no (and must have no) security\n>> \t# implications.\n>> -\ttmp=\"$TMPDIR\"; if [ -z \"$tmp\" ]; then tmp=/tmp; fi\n>> +\ttmp=${TMPDIR:-\"/tmp\"}\n\nI guess, you'd prefer ${TMPDIR:-/tmp} here too.\n\n>> \tstderr=\"$tmp/git-external-stderr.$$.tmp\"\n>> \ttest_external \"$@\" 4> \"$stderr\"\n>> \t[ -f \"$stderr\" ] || error \"Internal error: $stderr disappeared.\"\n>> @@ -801,12 +801,14 @@ test_done () {\n>> \t\tmkdir -p \"$test_results_dir\"\n>> \t\ttest_results_path=\"$test_results_dir/${0%.sh}-$$.counts\"\n>> \n>> -\t\techo \"total $test_count\" >> $test_results_path\n>> -\t\techo \"success $test_success\" >> $test_results_path\n>> -\t\techo \"fixed $test_fixed\" >> $test_results_path\n>> -\t\techo \"broken $test_broken\" >> $test_results_path\n>> -\t\techo \"failed $test_failure\" >> $test_results_path\n>> -\t\techo \"\" >> $test_results_path\n>> +\t\tcat >> \"$test_results_path\" <> +total $test_count\n>> +success $test_success\n>> +fixed $test_fixed\n>> +broken $test_broken\n>> +failed $test_failure\n>> +\n>> +EOF\n> \n> It may probably be even easier to read if you indented the whole thing,\n> using the dash before the here-doc marker, like so:\n> \n> \tcat >>\"$test_results_path\" <<-EOF\n> \ttotal $test_count\n> success $test_success\n> ...\n> EOF\n\nOK, I'll be re-rolling the patch using \"<<-EOF\".\n\n-Mathias\n"},{"id":"166732","messageId":"1304080230-11670-1-git-send-email-misfire@debugon.org","threadId":"27200","inReplyTo":"4DBA8C43.4040804@debugon.org","subject":"[PATCH v2] t/test-lib.sh: minor readability improvements","fromName":"Mathias Lafeldt","fromEmail":"misfire@debugon.org","sentAt":"2011-04-29T12:30:30Z","receivedAt":"2011-04-29T12:30:30Z","isPatch":true,"sender":{"key":"misfire@debugon.org","avatar":"https://avatars.githubusercontent.com/u/158074?v=4"},"body":"Apply parameter expansion. Also use here document to save\ntest results instead of appending each line with \">>\".\n\nSigned-off-by: Mathias Lafeldt \n---\n t/test-lib.sh | 16 +++++++++-------\n 1 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex abc47f3..aca03d2 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -575,7 +575,7 @@ test_external () {\n test_external_without_stderr () {\n \t# The temporary file has no (and must have no) security\n \t# implications.\n-\ttmp=\"$TMPDIR\"; if [ -z \"$tmp\" ]; then tmp=/tmp; fi\n+\ttmp=${TMPDIR:-/tmp}\n \tstderr=\"$tmp/git-external-stderr.$$.tmp\"\n \ttest_external \"$@\" 4> \"$stderr\"\n \t[ -f \"$stderr\" ] || error \"Internal error: $stderr disappeared.\"\n@@ -801,12 +801,14 @@ test_done () {\n \t\tmkdir -p \"$test_results_dir\"\n \t\ttest_results_path=\"$test_results_dir/${0%.sh}-$$.counts\"\n \n-\t\techo \"total $test_count\" >> $test_results_path\n-\t\techo \"success $test_success\" >> $test_results_path\n-\t\techo \"fixed $test_fixed\" >> $test_results_path\n-\t\techo \"broken $test_broken\" >> $test_results_path\n-\t\techo \"failed $test_failure\" >> $test_results_path\n-\t\techo \"\" >> $test_results_path\n+\t\tcat >>\"$test_results_path\" <<-EOF\n+\t\ttotal $test_count\n+\t\tsuccess $test_success\n+\t\tfixed $test_fixed\n+\t\tbroken $test_broken\n+\t\tfailed $test_failure\n+\n+\t\tEOF\n \tfi\n \n \tif test \"$test_fixed\" != 0\n-- \n1.7.5\n"},{"id":"166748","messageId":"7vpqo5xbru.fsf@alter.siamese.dyndns.org","threadId":"27200","inReplyTo":"1304080230-11670-1-git-send-email-misfire@debugon.org","subject":"Re: [PATCH v2] t/test-lib.sh: minor readability improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-29T16:28:21Z","receivedAt":"2011-04-29T16:28:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"}]}