{"thread":{"id":"34052","subject":"[PATCH 0/3] Trivial patches","startedAt":"2013-06-07T20:32:20Z","lastAt":"2013-06-10T20:41:24Z","messageCount":24,"participants":["Felipe Contreras","Junio C Hamano","Duy Nguyen","Philip Oakley","SZEDER Gábor","Johannes Sixt","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"219699","messageId":"1370637143-21336-1-git-send-email-felipe.contreras@gmail.com","threadId":"34052","inReplyTo":null,"subject":"[PATCH 0/3] Trivial patches","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-07T20:32:20Z","receivedAt":"2013-06-07T20:32:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Felipe Contreras (3):\n sequencer: trivial fix\n test: improve rebase -q test\n submodule: remove unnecessary check\n\n sequencer.c | 7 +++++--\n submodule.c | 5 ++---\n t/t3400-rebase.sh | 1 +\n 3 files changed, 8 insertions(+), 5 deletions(-)\n\n-- \n1.8.3.698.g079b096\n"},{"id":"219700","messageId":"1370637143-21336-2-git-send-email-felipe.contreras@gmail.com","threadId":"34052","inReplyTo":"1370637143-21336-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/3] sequencer: trivial fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-07T20:32:21Z","receivedAt":"2013-06-07T20:32:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We should free objects before leaving.\n\nSigned-off-by: Felipe Contreras \n---\n sequencer.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex ab6f8a7..7eeae2f 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -626,12 +626,15 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\trerere(opts->allow_rerere_auto);\n \t} else {\n \t\tint allow = allow_empty(opts, commit);\n-\t\tif (allow < 0)\n-\t\t\treturn allow;\n+\t\tif (allow < 0) {\n+\t\t\tres = allow;\n+\t\t\tgoto leave;\n+\t\t}\n \t\tif (!opts->no_commit)\n \t\t\tres = run_git_commit(defmsg, opts, allow);\n \t}\n \n+leave:\n \tfree_message(&msg);\n \tfree(defmsg);\n \n-- \n1.8.3.698.g079b096\n"},{"id":"219702","messageId":"1370637143-21336-3-git-send-email-felipe.contreras@gmail.com","threadId":"34052","inReplyTo":"1370637143-21336-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/3] test: improve rebase -q test","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-07T20:32:22Z","receivedAt":"2013-06-07T20:32:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Let's show the output so it's clear why it failed.\n\nSigned-off-by: Felipe Contreras \n---\n t/t3400-rebase.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex b58fa1a..fb39531 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -185,6 +185,7 @@ test_expect_success 'default to @{upstream} when upstream arg is missing' '\n test_expect_success 'rebase -q is quiet' '\n \tgit checkout -b quiet topic &&\n \tgit rebase -q master >output.out 2>&1 &&\n+\tcat output.out &&\n \ttest ! -s output.out\n '\n \n-- \n1.8.3.698.g079b096\n"},{"id":"219701","messageId":"1370637143-21336-4-git-send-email-felipe.contreras@gmail.com","threadId":"34052","inReplyTo":"1370637143-21336-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 3/3] submodule: remove unnecessary check","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-07T20:32:23Z","receivedAt":"2013-06-07T20:32:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"read_cache() already does that check.\n\nSigned-off-by: Felipe Contreras \n---\n submodule.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex ad476ce..8685424 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -603,9 +603,8 @@ int fetch_populated_submodules(const struct argv_array *options,\n \tif (!work_tree)\n \t\tgoto out;\n \n-\tif (!the_index.initialized)\n-\t\tif (read_cache() < 0)\n-\t\t\tdie(\"index file corrupt\");\n+\tif (read_cache() < 0)\n+\t\tdie(\"index file corrupt\");\n \n \targv_array_push(&argv, \"fetch\");\n \tfor (i = 0; i < options->argc; i++)\n-- \n1.8.3.698.g079b096\n"},{"id":"219762","messageId":"7vsj0tr0y7.fsf@alter.siamese.dyndns.org","threadId":"34052","inReplyTo":"1370637143-21336-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/3] sequencer: trivial fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-07T22:49:20Z","receivedAt":"2013-06-07T22:49:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n\nI thought the conclusion was that combination of c8d1351 and 706728a\nwe already queued was the right change. Is this meant to replace\nthem?\n"},{"id":"219763","messageId":"CAMP44s12BjNTeoei-+PZQcd0uBRCLPsfMyA79zuBwcxTpAnRFQ@mail.gmail.com","threadId":"34052","inReplyTo":"7vsj0tr0y7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] sequencer: trivial fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-07T22:59:00Z","receivedAt":"2013-06-07T22:59:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Jun 7, 2013 at 5:49 PM, Junio C Hamano wrote:\n> I thought the conclusion was that combination of c8d1351 and 706728a\n> we already queued was the right change. Is this meant to replace\n> them?\n\nYes, those would do, but I'm not going to work on that series any more.\n\n-- \nFelipe Contreras\n"},{"id":"219780","messageId":"CACsJy8DHeqOz=WbxurCvPiDq73k4eftwrEEZzWBbifS51PDbLQ@mail.gmail.com","threadId":"34052","inReplyTo":"1370637143-21336-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-06-08T02:44:45Z","receivedAt":"2013-06-08T02:44:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Jun 8, 2013 at 3:32 AM, Felipe Contreras\n wrote:\n> Let's show the output so it's clear why it failed.\n\nI think you can always look into trash-directory.t3400 and figure why.\nBut if you show this, I think you should use test_cmp to make it clear\nwhat is not wanted. Something like\n\n: >expected &&\ntest_cmp expected output.out\n\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> t/t3400-rebase.sh | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n> index b58fa1a..fb39531 100755\n> --- a/t/t3400-rebase.sh\n> +++ b/t/t3400-rebase.sh\n> @@ -185,6 +185,7 @@ test_expect_success 'default to @{upstream} when upstream arg is missing' '\n> test_expect_success 'rebase -q is quiet' '\n> git checkout -b quiet topic &&\n> git rebase -q master >output.out 2>&1 &&\n> + cat output.out &&\n> test ! -s output.out\n> '\n>\n> --\n> 1.8.3.698.g079b096\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\n\n--\nDuy\n"},{"id":"219796","messageId":"CAMP44s012kdYH5tRBnH4ijsGuVVDUe1Zbo4XkFx+wQNwOt_trg@mail.gmail.com","threadId":"34052","inReplyTo":"CACsJy8DHeqOz=WbxurCvPiDq73k4eftwrEEZzWBbifS51PDbLQ@mail.gmail.com","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-08T10:15:52Z","receivedAt":"2013-06-08T10:15:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Jun 7, 2013 at 9:44 PM, Duy Nguyen wrote:\n> On Sat, Jun 8, 2013 at 3:32 AM, Felipe Contreras\n> wrote:\n>> Let's show the output so it's clear why it failed.\n>\n> I think you can always look into trash-directory.t3400 and figure why.\n> But if you show this, I think you should use test_cmp to make it clear\n> what is not wanted. Something like\n>\n> : >expected &&\n> test_cmp expected output.out\n\nFeel free to send that patch. I'm done with this one.\n\n-- \nFelipe Contreras\n"},{"id":"220087","messageId":"7vd2rvqgra.fsf@alter.siamese.dyndns.org","threadId":"34052","inReplyTo":"CACsJy8DHeqOz=WbxurCvPiDq73k4eftwrEEZzWBbifS51PDbLQ@mail.gmail.com","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-09T18:30:01Z","receivedAt":"2013-06-09T18:30:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Sat, Jun 8, 2013 at 3:32 AM, Felipe Contreras\n> wrote:\n>> Let's show the output so it's clear why it failed.\n>\n> I think you can always look into trash-directory.t3400 and figure why.\n> But if you show this, I think you should use test_cmp to make it clear\n> what is not wanted. Something like\n>\n> : >expected &&\n> test_cmp expected output.out\n\nThere are quite many places that do this \"output must be empty\" in\nthe test suite, so it may deserve a special case perhaps like this\none.\n\n-- >8 --\nSubject: [PATCH] test: test_output_must_be_empty helper\n\nThere are quite a lot places where an output file is expected to be\nempty, and we fail the test when it is not. The output from running\nthe test script with -i -v can be helped if we showed the unexpected\ncontents at that point.\n\nWe could of course do\n\n >expected.empty && test_cmp expected.empty actual\n\nbut this is commmon enough to be done with a dedicated helper.\n\nSigned-off-by: Junio C Hamano \n---\n t/t0040-parse-options.sh | 46 +++++++++++++++++++++----------------------\n t/t3400-rebase.sh | 2 +-\n t/t3903-stash.sh | 10 +++++-----\n t/t5521-pull-options.sh | 26 ++++++++++++------------\n t/t5702-clone-options.sh | 2 +-\n t/t7102-reset.sh | 2 +-\n t/t7400-submodule-basic.sh | 6 +++---\n t/t9402-git-cvsserver-refs.sh | 12 +++++------\n t/test-lib-functions.sh | 12 +++++++++++\n 9 files changed, 65 insertions(+), 53 deletions(-)\n\ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex 244a43c..e2f5be0 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -50,7 +50,7 @@ EOF\n \n test_expect_success 'test help' '\n \ttest_must_fail test-parse-options -h > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_i18ncmp expect output\n '\n \n@@ -75,7 +75,7 @@ check() {\n \tshift &&\n \tsed \"s/^$what .*/$what $expect/\" expect &&\n \ttest-parse-options $* >output 2>output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n }\n \n@@ -86,7 +86,7 @@ check_i18n() {\n \tshift &&\n \tsed \"s/^$what .*/$what $expect/\" expect &&\n \ttest-parse-options $* >output 2>output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_i18ncmp expect output\n }\n \n@@ -99,7 +99,7 @@ check_unknown() {\n \tesac &&\n \tcat expect.err >>expect &&\n \ttest_must_fail test-parse-options $* >output 2>output.err &&\n-\ttest ! -s output &&\n+\ttest_output_must_be_empty output &&\n \ttest_cmp expect output.err\n }\n \n@@ -112,7 +112,7 @@ check_unknown_i18n() {\n \tesac &&\n \tcat expect.err >>expect &&\n \ttest_must_fail test-parse-options $* >output 2>output.err &&\n-\ttest ! -s output &&\n+\ttest_output_must_be_empty output &&\n \ttest_i18ncmp expect output.err\n }\n \n@@ -149,7 +149,7 @@ test_expect_success 'short options' '\n \ttest-parse-options -s123 -b -i 1729 -b -vv -n -F my.file \\\n \t> output 2> output.err &&\n \ttest_cmp expect output &&\n-\ttest ! -s output.err\n+\ttest_output_must_be_empty output.err\n '\n \n cat > expect << EOF\n@@ -168,7 +168,7 @@ test_expect_success 'long options' '\n \ttest-parse-options --boolean --integer 1729 --boolean --string2=321 \\\n \t\t--verbose --verbose --no-dry-run --abbrev=10 --file fi.le\\\n \t\t--obsolete > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n@@ -199,7 +199,7 @@ EOF\n test_expect_success 'intermingled arguments' '\n \ttest-parse-options a1 --string 123 b1 --boolean -j 13 -- --boolean \\\n \t\t> output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n@@ -217,13 +217,13 @@ EOF\n \n test_expect_success 'unambiguously abbreviated option' '\n \ttest-parse-options --int 2 --boolean --no-bo > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'unambiguously abbreviated option with \"=\"' '\n \ttest-parse-options --int=2 > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n@@ -246,7 +246,7 @@ EOF\n \n test_expect_success 'non ambiguous option (after two options it abbreviates)' '\n \ttest-parse-options --st 123 > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n@@ -256,7 +256,7 @@ EOF\n \n test_expect_success 'detect possible typos' '\n \ttest_must_fail test-parse-options -boolean > output 2> output.err &&\n-\ttest ! -s output &&\n+\ttest_output_must_be_empty output &&\n \ttest_cmp typo.err output.err\n '\n \n@@ -266,7 +266,7 @@ EOF\n \n test_expect_success 'detect possible typos' '\n \ttest_must_fail test-parse-options -ambiguous > output 2> output.err &&\n-\ttest ! -s output &&\n+\ttest_output_must_be_empty output &&\n \ttest_cmp typo.err output.err\n '\n \n@@ -285,7 +285,7 @@ EOF\n \n test_expect_success 'keep some options as arguments' '\n \ttest-parse-options --quux > output 2> output.err &&\n- test ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n test_cmp expect output\n '\n \n@@ -305,7 +305,7 @@ EOF\n test_expect_success 'OPT_DATE() and OPT_SET_PTR() work' '\n \ttest-parse-options -t \"1970-01-01 00:00:01 +0000\" --default-string \\\n \t\tfoo -q > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n@@ -324,7 +324,7 @@ EOF\n \n test_expect_success 'OPT_CALLBACK() and OPT_BIT() work' '\n \ttest-parse-options --length=four -b -4 > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n@@ -352,13 +352,13 @@ EOF\n \n test_expect_success 'OPT_BIT() and OPT_SET_INT() work' '\n \ttest-parse-options --set23 -bbbbb --no-or4 > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'OPT_NEGBIT() and OPT_SET_INT() work' '\n \ttest-parse-options --set23 -bbbbb --neg-or4 > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n@@ -376,19 +376,19 @@ EOF\n \n test_expect_success 'OPT_BIT() works' '\n \ttest-parse-options -bb --or4 > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'OPT_NEGBIT() works' '\n \ttest-parse-options -bb --no-neg-or4 > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'OPT_COUNTUP() with PARSE_OPT_NODASH works' '\n \ttest-parse-options + + + + + + > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n@@ -406,7 +406,7 @@ EOF\n \n test_expect_success 'OPT_NUMBER_CALLBACK() works' '\n \ttest-parse-options -12345 > output 2> output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \n@@ -424,7 +424,7 @@ EOF\n \n test_expect_success 'negation of OPT_NONEG flags is not ambiguous' '\n \ttest-parse-options --no-ambig >output 2>output.err &&\n-\ttest ! -s output.err &&\n+\ttest_output_must_be_empty output.err &&\n \ttest_cmp expect output\n '\n \ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 1de0ebd..e198419 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -179,7 +179,7 @@ test_expect_success 'default to @{upstream} when upstream arg is missing' '\n test_expect_success 'rebase -q is quiet' '\n \tgit checkout -b quiet topic &&\n \tgit rebase -q master >output.out 2>&1 &&\n-\ttest ! -s output.out\n+\ttest_output_must_be_empty output.out\n '\n \n test_expect_success 'Rebase a commit that sprinkles CRs in' '\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 5dfbda7..ebd838a 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -200,17 +200,17 @@ test_expect_success 'apply -q is quiet' '\n \techo foo > file &&\n \tgit stash &&\n \tgit stash apply -q > output.out 2>&1 &&\n-\ttest ! -s output.out\n+\ttest_output_must_be_empty output.out\n '\n \n test_expect_success 'save -q is quiet' '\n \tgit stash save --quiet > output.out 2>&1 &&\n-\ttest ! -s output.out\n+\ttest_output_must_be_empty output.out\n '\n \n test_expect_success 'pop -q is quiet' '\n \tgit stash pop -q > output.out 2>&1 &&\n-\ttest ! -s output.out\n+\ttest_output_must_be_empty output.out\n '\n \n test_expect_success 'pop -q --index works and is quiet' '\n@@ -219,13 +219,13 @@ test_expect_success 'pop -q --index works and is quiet' '\n \tgit stash save --quiet &&\n \tgit stash pop -q --index > output.out 2>&1 &&\n \ttest foo = \"$(git show :file)\" &&\n-\ttest ! -s output.out\n+\ttest_output_must_be_empty output.out\n '\n \n test_expect_success 'drop -q is quiet' '\n \tgit stash &&\n \tgit stash drop -q > output.out 2>&1 &&\n-\ttest ! -s output.out\n+\ttest_output_must_be_empty output.out\n '\n \n test_expect_success 'stash -k' '\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex aa31abe..f913166 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -15,19 +15,19 @@ test_expect_success 'git pull -q' '\n \tmkdir clonedq &&\n \t(cd clonedq && git init &&\n \tgit pull -q \"../parent\" >out 2>err &&\n-\ttest ! -s err &&\n-\ttest ! -s out)\n+\ttest_output_must_be_empty err &&\n+\ttest_output_must_be_empty out)\n '\n \n test_expect_success 'git pull -q --rebase' '\n \tmkdir clonedqrb &&\n \t(cd clonedqrb && git init &&\n \tgit pull -q --rebase \"../parent\" >out 2>err &&\n-\ttest ! -s err &&\n-\ttest ! -s out &&\n+\ttest_output_must_be_empty err &&\n+\ttest_output_must_be_empty out &&\n \tgit pull -q --rebase \"../parent\" >out 2>err &&\n-\ttest ! -s err &&\n-\ttest ! -s out)\n+\ttest_output_must_be_empty err &&\n+\ttest_output_must_be_empty out)\n '\n \n test_expect_success 'git pull' '\n@@ -35,7 +35,7 @@ test_expect_success 'git pull' '\n \t(cd cloned && git init &&\n \tgit pull \"../parent\" >out 2>err &&\n \ttest -s err &&\n-\ttest ! -s out)\n+\ttest_output_must_be_empty out)\n '\n \n test_expect_success 'git pull --rebase' '\n@@ -43,7 +43,7 @@ test_expect_success 'git pull --rebase' '\n \t(cd clonedrb && git init &&\n \tgit pull --rebase \"../parent\" >out 2>err &&\n \ttest -s err &&\n-\ttest ! -s out)\n+\ttest_output_must_be_empty out)\n '\n \n test_expect_success 'git pull -v' '\n@@ -51,7 +51,7 @@ test_expect_success 'git pull -v' '\n \t(cd clonedv && git init &&\n \tgit pull -v \"../parent\" >out 2>err &&\n \ttest -s err &&\n-\ttest ! -s out)\n+\ttest_output_must_be_empty out)\n '\n \n test_expect_success 'git pull -v --rebase' '\n@@ -59,22 +59,22 @@ test_expect_success 'git pull -v --rebase' '\n \t(cd clonedvrb && git init &&\n \tgit pull -v --rebase \"../parent\" >out 2>err &&\n \ttest -s err &&\n-\ttest ! -s out)\n+\ttest_output_must_be_empty out)\n '\n \n test_expect_success 'git pull -v -q' '\n \tmkdir clonedvq &&\n \t(cd clonedvq && git init &&\n \tgit pull -v -q \"../parent\" >out 2>err &&\n-\ttest ! -s out &&\n-\ttest ! -s err)\n+\ttest_output_must_be_empty out &&\n+\ttest_output_must_be_empty err)\n '\n \n test_expect_success 'git pull -q -v' '\n \tmkdir clonedqv &&\n \t(cd clonedqv && git init &&\n \tgit pull -q -v \"../parent\" >out 2>err &&\n-\ttest ! -s out &&\n+\ttest_output_must_be_empty out &&\n \ttest -s err)\n '\n \ndiff --git a/t/t5702-clone-options.sh b/t/t5702-clone-options.sh\nindex 02cb024..d486c9b 100755\n--- a/t/t5702-clone-options.sh\n+++ b/t/t5702-clone-options.sh\n@@ -22,7 +22,7 @@ test_expect_success 'clone -o' '\n test_expect_success 'redirected clone' '\n \n \tgit clone \"file://$(pwd)/parent\" clone-redirected >out 2>err &&\n-\ttest ! -s err\n+\ttest_output_must_be_empty err\n \n '\n test_expect_success 'redirected clone -v' '\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex df82ec9..622feff 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -457,7 +457,7 @@ test_expect_success 'disambiguation (1)' '\n \ttest_must_fail git diff --quiet -- secondfile &&\n \ttest -z \"$(git diff --cached --name-only)\" &&\n \ttest -f secondfile &&\n-\ttest ! -s secondfile\n+\ttest_output_must_be_empty secondfile\n \n '\n \ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 2683cba..2779b7e 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -78,7 +78,7 @@ test_expect_success 'submodule add' '\n \t(\n \t\tcd addtest &&\n \t\tgit submodule add -q \"$submodurl\" submod >actual &&\n-\t\ttest ! -s actual &&\n+\t\ttest_output_must_be_empty actual &&\n \t\techo \"gitdir: ../.git/modules/submod\" >expect &&\n \t\ttest_cmp expect submod/.git &&\n \t\t(\n@@ -308,7 +308,7 @@ test_expect_success 'update should work when path is an empty dir' '\n \n \tmkdir init &&\n \tgit submodule update -q >update.out &&\n-\ttest ! -s update.out &&\n+\ttest_output_must_be_empty update.out &&\n \n \tinspect init &&\n \ttest_cmp expect head-sha1\n@@ -696,7 +696,7 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano\n \t\trm -rf repo &&\n \t\tgit rm repo &&\n \t\tgit submodule add -q --name repo_new \"$submodurl/bare.git\" repo >actual &&\n-\t\ttest ! -s actual &&\n+\t\ttest_output_must_be_empty actual &&\n \t\techo \"gitdir: ../.git/modules/submod\" >expect &&\n \t\ttest_cmp expect submod/.git &&\n \t\t(\ndiff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh\nindex 735a018..84a4309 100755\n--- a/t/t9402-git-cvsserver-refs.sh\n+++ b/t/t9402-git-cvsserver-refs.sh\n@@ -330,7 +330,7 @@ test_expect_success 'validate result of edits [cvswork2]' '\n \n test_expect_success 'validate basic diffs saved during above cvswork2 edits' '\n \ttest $(grep Index: cvsEdit1.diff | wc -l) = 1 &&\n-\ttest ! -s cvsEdit2-empty.diff &&\n+\ttest_output_must_be_empty cvsEdit2-empty.diff &&\n \ttest $(grep Index: cvsEdit2-N.diff | wc -l) = 1 &&\n \ttest $(grep Index: cvsEdit3.diff | wc -l) = 3 &&\n \trm -rf diffSandbox &&\n@@ -456,20 +456,20 @@ test_expect_success 'cvs up -r $(git rev-parse v1)' '\n \n test_expect_success 'cvs diff -r v1 -u' '\n \t( cd cvswork && cvs -f diff -r v1 -u ) >cvsDiff.out 2>cvs.log &&\n-\ttest ! -s cvsDiff.out &&\n-\ttest ! -s cvs.log\n+\ttest_output_must_be_empty cvsDiff.out &&\n+\ttest_output_must_be_empty cvs.log\n '\n \n test_expect_success 'cvs diff -N -r v2 -u' '\n \t( cd cvswork && ! cvs -f diff -N -r v2 -u ) >cvsDiff.out 2>cvs.log &&\n-\ttest ! -s cvs.log &&\n+\ttest_output_must_be_empty cvs.log &&\n \ttest -s cvsDiff.out &&\n \tcheck_diff cvsDiff.out v2 v1 >check_diff.out 2>&1\n '\n \n test_expect_success 'cvs diff -N -r v2 -r v1.2' '\n \t( cd cvswork && ! cvs -f diff -N -r v2 -r v1.2 -u ) >cvsDiff.out 2>cvs.log &&\n-\ttest ! -s cvs.log &&\n+\ttest_output_must_be_empty cvs.log &&\n \ttest -s cvsDiff.out &&\n \tcheck_diff cvsDiff.out v2 v1.2 >check_diff.out 2>&1\n '\n@@ -488,7 +488,7 @@ test_expect_success 'apply early [cvswork3] diff to b3' '\n \n test_expect_success 'check [cvswork3] diff' '\n \t( cd cvswork3 && ! cvs -f diff -N -u ) >\"$WORKDIR/cvsDiff.out\" 2>cvs.log &&\n-\ttest ! -s cvs.log &&\n+\ttest_output_must_be_empty cvs.log &&\n \ttest -s cvsDiff.out &&\n \ttest $(grep Index: cvsDiff.out | wc -l) = 3 &&\n \ttest_cmp cvsDiff.out cvswork3edit.diff &&\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 3fc9cc9..a88e652 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -606,6 +606,18 @@ test_cmp() {\n \t$GIT_TEST_CMP \"$@\"\n }\n \n+# Check if the file expected to be empty is indeed empty, and barfs\n+# otherwise.\n+\n+test_output_must_be_empty () {\n+\tif test -s \"$1\"\n+\tthen\n+\t\techo \"Bad: test_output_must_be_empty '$1', but has the following.\"\n+\t\tcat \"$1\"\n+\t\treturn 1\n+\tfi\n+}\n+\n # Tests that its two parameters refer to the same revision\n test_cmp_rev () {\n \tgit rev-parse --verify \"$1\" >expect.rev &&\n-- \n1.8.3-477-gc2fede3\n"},{"id":"220090","messageId":"CAMP44s3Pny7JkyHbLZ3kUemNK70JhdYWdpELTjNLz0y3Z2V3+A@mail.gmail.com","threadId":"34052","inReplyTo":"7vd2rvqgra.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-09T18:35:21Z","receivedAt":"2013-06-09T18:35:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 9, 2013 at 1:30 PM, Junio C Hamano wrote:\n\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -606,6 +606,18 @@ test_cmp() {\n> $GIT_TEST_CMP \"$@\"\n> }\n>\n> +# Check if the file expected to be empty is indeed empty, and barfs\n> +# otherwise.\n> +\n> +test_output_must_be_empty () {\n\nWhy such a big name? test_empty() does the trick.\n\n> + if test -s \"$1\"\n> + then\n> + echo \"Bad: test_output_must_be_empty '$1', but has the following.\"\n\necho \"'$1' is not empty, it contains:\"\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"220105","messageId":"7vy5ajozuj.fsf@alter.siamese.dyndns.org","threadId":"34052","inReplyTo":"CAMP44s3Pny7JkyHbLZ3kUemNK70JhdYWdpELTjNLz0y3Z2V3+A@mail.gmail.com","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-09T19:20:36Z","receivedAt":"2013-06-09T19:20:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Sun, Jun 9, 2013 at 1:30 PM, Junio C Hamano wrote:\n>\n>> --- a/t/test-lib-functions.sh\n>> +++ b/t/test-lib-functions.sh\n>> @@ -606,6 +606,18 @@ test_cmp() {\n>> $GIT_TEST_CMP \"$@\"\n>> }\n>>\n>> +# Check if the file expected to be empty is indeed empty, and barfs\n>> +# otherwise.\n>> +\n>> +test_output_must_be_empty () {\n>\n> Why such a big name? test_empty() does the trick.\n\nPrimarily in order to avoid that exact name \"test_empty\" that others\nmay want to use for a helper to check that the contents of a string\nvariable is empty. test-file-must-be-empty is a bit shorter and\nalso good for that purpose; I do not think we want to go any shorter\nthan that.\n"},{"id":"220148","messageId":"CAMP44s38T9EUOe8EBKy1kxa-rEu7g0jb7+HB019AgCub+2SVnw@mail.gmail.com","threadId":"34052","inReplyTo":"7vy5ajozuj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-09T19:33:45Z","receivedAt":"2013-06-09T19:33:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 9, 2013 at 2:20 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Sun, Jun 9, 2013 at 1:30 PM, Junio C Hamano wrote:\n>>\n>>> --- a/t/test-lib-functions.sh\n>>> +++ b/t/test-lib-functions.sh\n>>> @@ -606,6 +606,18 @@ test_cmp() {\n>>> $GIT_TEST_CMP \"$@\"\n>>> }\n>>>\n>>> +# Check if the file expected to be empty is indeed empty, and barfs\n>>> +# otherwise.\n>>> +\n>>> +test_output_must_be_empty () {\n>>\n>> Why such a big name? test_empty() does the trick.\n>\n> Primarily in order to avoid that exact name \"test_empty\" that others\n> may want to use for a helper to check that the contents of a string\n> variable is empty.\n\nWhich is never going to happen.\n\n-- \nFelipe Contreras\n"},{"id":"220164","messageId":"7vfvwrowd0.fsf@alter.siamese.dyndns.org","threadId":"34052","inReplyTo":"CAMP44s38T9EUOe8EBKy1kxa-rEu7g0jb7+HB019AgCub+2SVnw@mail.gmail.com","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-09T20:35:55Z","receivedAt":"2013-06-09T20:35:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Sun, Jun 9, 2013 at 2:20 PM, Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>>\n>>> On Sun, Jun 9, 2013 at 1:30 PM, Junio C Hamano wrote:\n>>>\n>>>> --- a/t/test-lib-functions.sh\n>>>> +++ b/t/test-lib-functions.sh\n>>>> @@ -606,6 +606,18 @@ test_cmp() {\n>>>> $GIT_TEST_CMP \"$@\"\n>>>> }\n>>>>\n>>>> +# Check if the file expected to be empty is indeed empty, and barfs\n>>>> +# otherwise.\n>>>> +\n>>>> +test_output_must_be_empty () {\n>>>\n>>> Why such a big name? test_empty() does the trick.\n>>\n>> Primarily in order to avoid that exact name \"test_empty\" that others\n>> may want to use for a helper to check that the contents of a string\n>> variable is empty.\n>\n> Which is never going to happen.\n\nFor anything, a failure from\n\n\ttest -z \"$mustbeemptystring\"\n\nin the test suite is much harder to diagnose because there is\nnothing left in the trash directory to inspect, as opposed to\n\n\ttest ! -s \"$mustbeemptyfile\"\n\nwhere you can just go there and inspect yourself.\n"},{"id":"220165","messageId":"CAMP44s0Qr54+8hbkZ+jmXB628SCPwg1zZo80UBhFe2PCrgQP4Q@mail.gmail.com","threadId":"34052","inReplyTo":"7vfvwrowd0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-09T20:41:54Z","receivedAt":"2013-06-09T20:41:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 9, 2013 at 3:35 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Sun, Jun 9, 2013 at 2:20 PM, Junio C Hamano wrote:\n>>> Felipe Contreras writes:\n>>>\n>>>> On Sun, Jun 9, 2013 at 1:30 PM, Junio C Hamano wrote:\n>>>>\n>>>>> --- a/t/test-lib-functions.sh\n>>>>> +++ b/t/test-lib-functions.sh\n>>>>> @@ -606,6 +606,18 @@ test_cmp() {\n>>>>> $GIT_TEST_CMP \"$@\"\n>>>>> }\n>>>>>\n>>>>> +# Check if the file expected to be empty is indeed empty, and barfs\n>>>>> +# otherwise.\n>>>>> +\n>>>>> +test_output_must_be_empty () {\n>>>>\n>>>> Why such a big name? test_empty() does the trick.\n>>>\n>>> Primarily in order to avoid that exact name \"test_empty\" that others\n>>> may want to use for a helper to check that the contents of a string\n>>> variable is empty.\n>>\n>> Which is never going to happen.\n>\n> For anything, a failure from\n>\n> test -z \"$mustbeemptystring\"\n>\n> in the test suite is much harder to diagnose because there is\n> nothing left in the trash directory to inspect, as opposed to\n>\n> test ! -s \"$mustbeemptyfile\"\n>\n> where you can just go there and inspect yourself.\n\nExcept that it's usually gone. And I challenge you to find a instance\nwhere there's a test -z \"$mustbeemptystring\" that throws a test\nfailure. It will take you time to find it (if there's any).\n\nMoreover, by that rationale, we should call test_cmp, test_file_cmp,\nbut there's no need, because that's rarely needed (if at all). There\nwill not be a need for test_string_must_be_empty() just like there's\nno need for test_string_cmp().\n\n-- \nFelipe Contreras\n"},{"id":"220202","messageId":"C30D5684C80641EFAC0F89F00B48A74F@PhilipOakley","threadId":"34052","inReplyTo":"CAMP44s38T9EUOe8EBKy1kxa-rEu7g0jb7+HB019AgCub+2SVnw@mail.gmail.com","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-06-09T22:44:11Z","receivedAt":"2013-06-09T22:44:11Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Felipe Contreras\" \nSent: Sunday, June 09, 2013 8:33 PM\n> On Sun, Jun 9, 2013 at 2:20 PM, Junio C Hamano \n> wrote:\n>> Felipe Contreras writes:\n>>\n>>> On Sun, Jun 9, 2013 at 1:30 PM, Junio C Hamano \n>>> wrote:\n>>>\n>>>> --- a/t/test-lib-functions.sh\n>>>> +++ b/t/test-lib-functions.sh\n>>>> @@ -606,6 +606,18 @@ test_cmp() {\n>>>> $GIT_TEST_CMP \"$@\"\n>>>> }\n>>>>\n>>>> +# Check if the file expected to be empty is indeed empty, and \n>>>> barfs\n>>>> +# otherwise.\n>>>> +\n>>>> +test_output_must_be_empty () {\n>>>\n>>> Why such a big name? test_empty() does the trick.\n>>\n>> Primarily in order to avoid that exact name \"test_empty\" that others\n>> may want to use for a helper to check that the contents of a string\n>> variable is empty.\n>\n> Which is never going to happen.\n>\n\nWhile folks do use such simplistic names, given that the patch had many \ncall sites, I do think Filipe's short name would quickly become the \naccepted test name and not cause any great difficulties.\n\nregards\nPhilip \n"},{"id":"220215","messageId":"7v61xmn9av.fsf@alter.siamese.dyndns.org","threadId":"34052","inReplyTo":"C30D5684C80641EFAC0F89F00B48A74F@PhilipOakley","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-09T23:39:20Z","receivedAt":"2013-06-09T23:39:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> While folks do use such simplistic names, given that the patch had\n> many call sites, I do think Filipe's short name would quickly become\n> the accepted test name and not cause any great difficulties.\n\nOK.\n"},{"id":"220268","messageId":"20130610093904.GG2091@goldbirke","threadId":"34052","inReplyTo":"CAMP44s0Qr54+8hbkZ+jmXB628SCPwg1zZo80UBhFe2PCrgQP4Q@mail.gmail.com","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2013-06-10T09:39:04Z","receivedAt":"2013-06-10T09:39:04Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Jun 09, 2013 at 03:41:54PM -0500, Felipe Contreras wrote:\n> There\n> will not be a need for test_string_must_be_empty() just like there's\n> no need for test_string_cmp().\n\nActually, if there were a test_string_cmp(), that would be the test\nhelper function I used most often.\n"},{"id":"220305","messageId":"7vppvuj6wl.fsf@alter.siamese.dyndns.org","threadId":"34052","inReplyTo":"20130610093904.GG2091@goldbirke","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-10T15:56:58Z","receivedAt":"2013-06-10T15:56:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> On Sun, Jun 09, 2013 at 03:41:54PM -0500, Felipe Contreras wrote:\n>> There\n>> will not be a need for test_string_must_be_empty() just like there's\n>> no need for test_string_cmp().\n>\n> Actually, if there were a test_string_cmp(), that would be the test\n> helper function I used most often.\n\nHmm, there indeed are quite a many \"At this point, the variable's\nvalue must be this\" in the test scripts. With things like this:\n\n t/t0002-gitfile.sh: test \"$SHA\" = \"$(git rev-list HEAD)\"\n\nwe can go to the trash directory upon seeing a failure to run the\ncommand used on the RHS, but the value in $SHA is cumbersome to find\nout (either running it under sh -x or insert an extra echo before\nit), so such a helper function may be useful.\n\nDo you really need a general comparison (\"does A sort before B\") or\njust equality? If the latter, test_string_equal (or even\nstring_equal) might be a better name for it.\n\nBy the way, test_cmp() is a replacement for the \"cmp\" command and\nthat is why it does not have \"file\" in its name.\n"},{"id":"220323","messageId":"CAMP44s33khUaAWV+UrkS2CVRy2aCCk8kv3h5HXROySoocKrL8w@mail.gmail.com","threadId":"34052","inReplyTo":"7vppvuj6wl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-10T17:01:06Z","receivedAt":"2013-06-10T17:01:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jun 10, 2013 at 10:56 AM, Junio C Hamano wrote:\n\n> By the way, test_cmp() is a replacement for the \"cmp\" command and\n> that is why it does not have \"file\" in its name.\n\nThat's an irrelevant implementation detail. But if you want to be\ndriven the the implementation, call it test_zero().\n\n-- \nFelipe Contreras\n"},{"id":"220329","messageId":"20130610172708.GH2091@goldbirke","threadId":"34052","inReplyTo":"7vppvuj6wl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2013-06-10T17:27:08Z","receivedAt":"2013-06-10T17:27:08Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jun 10, 2013 at 08:56:58AM -0700, Junio C Hamano wrote:\n> SZEDER Gábor writes:\n> \n> > On Sun, Jun 09, 2013 at 03:41:54PM -0500, Felipe Contreras wrote:\n> >> There\n> >> will not be a need for test_string_must_be_empty() just like there's\n> >> no need for test_string_cmp().\n> >\n> > Actually, if there were a test_string_cmp(), that would be the test\n> > helper function I used most often.\n> \n> Hmm, there indeed are quite a many \"At this point, the variable's\n> value must be this\" in the test scripts. With things like this:\n> \n> t/t0002-gitfile.sh: test \"$SHA\" = \"$(git rev-list HEAD)\"\n> \n> we can go to the trash directory upon seeing a failure to run the\n> command used on the RHS, but the value in $SHA is cumbersome to find\n> out (either running it under sh -x or insert an extra echo before\n> it), so such a helper function may be useful.\n> \n> Do you really need a general comparison (\"does A sort before B\") or\n> just equality? If the latter, test_string_equal (or even\n> string_equal) might be a better name for it.\n\nYeah, I need only equality. Or at least it would be nice to have.\n\nMy main motivation is that, like in your example, in the bash prompt\ntests I only have to check a single line of output, but because of\ndebuggability I always did:\n\n echo \"(master)\" >expected\n __git_ps1 >actual\n test_cmp expected actual\n\nWith such a helper function this could be reduced to a single line:\n\n test_string_equal \"(master)\" \"$(__git_ps1)\"\n\nwithout loss of functionality or debuggability, because in case of a\nfailure it would output something like this (bikesheddable, of\ncourse):\n\n Error:\n expected: \"(master)\"\n got: \"((deadbeef...))\"\n\nAnd perhaps with a description as an optional third argument to help\nidentify the failed check if multiple such checks are done in a single\ntest, e.g. the test_rev_parse() helper in t/t1501-worktree.sh, 'setup:\nhelper for testing rev-parse', which could be shortened as:\n\n test_string_equal \"$1\" \"$(git rev-parse --is-bare-repository)\" \"bare\"\n test_string_equal \"$2\" \"$(git rev-parse --is-inside-git-dir)\" \"gitdir\"\n test_string_equal \"$3\" \"$(git rev-parse --is-inside-work-tree)\" \"worktree\"\n\nand if something goes wrong we'd get:\n\n Error: worktree\n expected: \"true\"\n got: \"false\"\n\nPerhaps I could find some time in the days ahead to give it a go.\n\n\nGábor\n"},{"id":"220360","messageId":"51B623DA.4050802@kdbg.org","threadId":"34052","inReplyTo":"20130610172708.GH2091@goldbirke","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-06-10T19:07:06Z","receivedAt":"2013-06-10T19:07:06Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10.06.2013 19:27, schrieb SZEDER Gábor:\n> My main motivation is that, like in your example, in the bash prompt\n> tests I only have to check a single line of output, but because of\n> debuggability I always did:\n> \n> echo \"(master)\" >expected\n> __git_ps1 >actual\n> test_cmp expected actual\n\nChained by &&, I presume.\n\n> With such a helper function this could be reduced to a single line:\n> \n> test_string_equal \"(master)\" \"$(__git_ps1)\"\n> \n> without loss of functionality\n\nNot quite: A non-zero exit code of the $(__git_ps1) is lost. (It\nprobably doesn't matter here, but it certainly does if the command is\n$(git rev-parse foo) or similar.)\n\n-- Hannes\n"},{"id":"220362","messageId":"7vbo7dhj20.fsf@alter.siamese.dyndns.org","threadId":"34052","inReplyTo":"20130610172708.GH2091@goldbirke","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-10T19:17:27Z","receivedAt":"2013-06-10T19:17:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> With such a helper function this could be reduced to a single line:\n>\n> test_string_equal \"(master)\" \"$(__git_ps1)\"\n>\n> without loss of functionality or debuggability, because in case of a\n> failure it would output something like this (bikesheddable, of\n> course):\n>\n> Error:\n> expected: \"(master)\"\n> got: \"((deadbeef...))\"\n\nYeah, that looks sensible.\n"},{"id":"220370","messageId":"20130610201933.GI2091@goldbirke","threadId":"34052","inReplyTo":"51B623DA.4050802@kdbg.org","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2013-06-10T20:19:33Z","receivedAt":"2013-06-10T20:19:33Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jun 10, 2013 at 09:07:06PM +0200, Johannes Sixt wrote:\n> Am 10.06.2013 19:27, schrieb SZEDER Gábor:\n> > My main motivation is that, like in your example, in the bash prompt\n> > tests I only have to check a single line of output, but because of\n> > debuggability I always did:\n> > \n> > echo \"(master)\" >expected\n> > __git_ps1 >actual\n> > test_cmp expected actual\n> \n> Chained by &&, I presume.\n\nSure.\n\n> > With such a helper function this could be reduced to a single line:\n> > \n> > test_string_equal \"(master)\" \"$(__git_ps1)\"\n> > \n> > without loss of functionality\n> \n> Not quite: A non-zero exit code of the $(__git_ps1) is lost. (It\n> probably doesn't matter here, but it certainly does if the command is\n> $(git rev-parse foo) or similar.)\n\nOuch, indeed. Yeah, the exit code doesn't matter for the prompt tests\n(I mean for __git_ps1() tests, but maybe it does matter for some\n__gitdir() tests), but I suppose it does matter everywhere else where\nthe same construct is used. We could still do\n\n actual=\"$(git foo)\" &&\n test_string_equal \"good\" \"$actual\"\n\nto preserve and check the exit code, and this is still one line\nshorter, but overall not that appealing anymore.\n\nHowever. The git command's exit code is lost the same way in 'test\ngood = $(git foo)' constructs, too, and plenty of such constructs are\nall over the test suite. Shouldn't we avoid using such constucts\nthen?\n\n\nGábor\n"},{"id":"220372","messageId":"20130610204123.GB13333@sigill.intra.peff.net","threadId":"34052","inReplyTo":"7vd2rvqgra.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] test: improve rebase -q test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-10T20:41:24Z","receivedAt":"2013-06-10T20:41:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 09, 2013 at 11:30:01AM -0700, Junio C Hamano wrote:\n\n> -- >8 --\n> Subject: [PATCH] test: test_output_must_be_empty helper\n> \n> There are quite a lot places where an output file is expected to be\n> empty, and we fail the test when it is not. The output from running\n> the test script with -i -v can be helped if we showed the unexpected\n> contents at that point.\n> \n> We could of course do\n> \n> >expected.empty && test_cmp expected.empty actual\n> \n> but this is commmon enough to be done with a dedicated helper.\n\nThanks, I think this improves the readability of the test suite (and its\noutput when there are failures).\n\nYou can also do:\n\n test_cmp /dev/null actual\n\nfor the same effect, but I guess the diff is not all that interesting\n(by definition, it would consist only of added lines, and you are\nalready showing them, so it would not be an improvement).\n\n-Peff\n"}]}