{"thread":{"id":"33772","subject":"[PATCH 1/4] t3400 (rebase): don't set GIT_AUTHOR_{NAME,EMAIL}","startedAt":"2013-05-10T14:29:34Z","lastAt":"2013-05-13T23:05:57Z","messageCount":21,"participants":["Ramkumar Ramachandra","Junio C Hamano","Eric Sunshine","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"216885","messageId":"1368196178-5807-1-git-send-email-artagnon@gmail.com","threadId":"33772","inReplyTo":null,"subject":"[PATCH 0/4] Trivial cleanups to t3400 (rebase)","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T14:29:34Z","receivedAt":"2013-05-10T14:29:34Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nI was just going through t3400 when wondering where to put the tests\nfor rebase.autostash. Here's a set of trivial patches.\n\nThanks.\n\nRamkumar Ramachandra (4):\n t3400 (rebase): don't set GIT_AUTHOR_{NAME,EMAIL}\n t3400 (rebase): downcase a couple of test titles\n t3400 (rebase): move lone statement into a test\n t4300 (rebase): don't unnecessarily set GIT_TRACE\n\n t/t3400-rebase.sh | 14 +++++---------\n 1 file changed, 5 insertions(+), 9 deletions(-)\n\n-- \n1.8.3.rc1.52.gc14258d\n"},{"id":"216881","messageId":"1368196178-5807-2-git-send-email-artagnon@gmail.com","threadId":"33772","inReplyTo":"1368196178-5807-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/4] t3400 (rebase): don't set GIT_AUTHOR_{NAME,EMAIL}","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T14:29:35Z","receivedAt":"2013-05-10T14:29:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"test-lib.sh already sets a sane GIT_AUTHOR_{NAME,EMAIL} for all test\nscripts to use. Don't unnecessarily duplicate the work.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3400-rebase.sh | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex b58fa1a..a7ca2f1 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -10,10 +10,6 @@ among other things.\n '\n . ./test-lib.sh\n \n-GIT_AUTHOR_NAME=author@name\n-GIT_AUTHOR_EMAIL=bogus@email@address\n-export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL\n-\n test_expect_success 'prepare repository with topic branches' '\n \tgit config core.logAllRefUpdates true &&\n \techo First >A &&\n-- \n1.8.3.rc1.52.gc14258d\n"},{"id":"216884","messageId":"1368196178-5807-3-git-send-email-artagnon@gmail.com","threadId":"33772","inReplyTo":"1368196178-5807-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/4] t3400 (rebase): downcase a couple of test titles","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T14:29:36Z","receivedAt":"2013-05-10T14:29:36Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Otherwise they stick out like sore thumbs in the test output, where\nall the other titles begin with a lowercase letter.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3400-rebase.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex a7ca2f1..cb4234a 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -152,7 +152,7 @@ test_expect_success 'setup: recover' '\n \tgit checkout modechange\n '\n \n-test_expect_success 'Show verbose error when HEAD could not be detached' '\n+test_expect_success 'show verbose error when HEAD could not be detached' '\n \t>B &&\n \ttest_must_fail git rebase topic 2>output.err >output.out &&\n \tgrep \"The following untracked working tree files would be overwritten by checkout:\" output.err &&\n@@ -184,7 +184,7 @@ test_expect_success 'rebase -q is quiet' '\n \ttest ! -s output.out\n '\n \n-test_expect_success 'Rebase a commit that sprinkles CRs in' '\n+test_expect_success 'rebase a commit that sprinkles CRs in' '\n \t(\n \t\techo \"One\"\n \t\techo \"TwoQ\"\n-- \n1.8.3.rc1.52.gc14258d\n"},{"id":"216882","messageId":"1368196178-5807-4-git-send-email-artagnon@gmail.com","threadId":"33772","inReplyTo":"1368196178-5807-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/4] t3400 (rebase): move lone statement into a test","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T14:29:37Z","receivedAt":"2013-05-10T14:29:37Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"'rm -f B' is a lone statement that isn't contained inside any\ntest_expect_* block. Use test_when_finished to execute it after the\npreceding test has finished.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3400-rebase.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex cb4234a..0841a12 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -154,11 +154,11 @@ test_expect_success 'setup: recover' '\n \n test_expect_success 'show verbose error when HEAD could not be detached' '\n \t>B &&\n+\ttest_when_finished rm -f B &&\n \ttest_must_fail git rebase topic 2>output.err >output.out &&\n \tgrep \"The following untracked working tree files would be overwritten by checkout:\" output.err &&\n \tgrep B output.err\n '\n-rm -f B\n \n test_expect_success 'fail when upstream arg is missing and not on branch' '\n \tgit checkout topic &&\n-- \n1.8.3.rc1.52.gc14258d\n"},{"id":"216883","messageId":"1368196178-5807-5-git-send-email-artagnon@gmail.com","threadId":"33772","inReplyTo":"1368196178-5807-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T14:29:38Z","receivedAt":"2013-05-10T14:29:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"A couple of tests execute 'git rebase' with GIT_TRACE set to 1, but\nthis trace output is not used anywhere. Remove it, since it is not\nrelevant to what we are testing.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t3400-rebase.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 0841a12..d0d9442 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -137,13 +137,13 @@ test_expect_success 'rebase a single mode change' '\n \ttest_chmod +x A &&\n \ttest_tick &&\n \tgit commit -m modechange &&\n-\tGIT_TRACE=1 git rebase master\n+\tgit rebase master\n '\n \n test_expect_success 'rebase is not broken by diff.renames' '\n \ttest_config diff.renames copies &&\n \tgit checkout filemove &&\n-\tGIT_TRACE=1 git rebase force-3way\n+\tgit rebase force-3way\n '\n \n test_expect_success 'setup: recover' '\n-- \n1.8.3.rc1.52.gc14258d\n"},{"id":"216888","messageId":"7vmws2529j.fsf@alter.siamese.dyndns.org","threadId":"33772","inReplyTo":"1368196178-5807-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-10T14:38:00Z","receivedAt":"2013-05-10T14:38:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> A couple of tests execute 'git rebase' with GIT_TRACE set to 1, but\n> this trace output is not used anywhere.\n\nIsn't it shown in \"t4300-*.sh -v\" output to help the debugger?\n\n> relevant to what we are testing.\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> t/t3400-rebase.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n> index 0841a12..d0d9442 100755\n> --- a/t/t3400-rebase.sh\n> +++ b/t/t3400-rebase.sh\n> @@ -137,13 +137,13 @@ test_expect_success 'rebase a single mode change' '\n> \ttest_chmod +x A &&\n> \ttest_tick &&\n> \tgit commit -m modechange &&\n> -\tGIT_TRACE=1 git rebase master\n> +\tgit rebase master\n> '\n> \n> test_expect_success 'rebase is not broken by diff.renames' '\n> \ttest_config diff.renames copies &&\n> \tgit checkout filemove &&\n> -\tGIT_TRACE=1 git rebase force-3way\n> +\tgit rebase force-3way\n> '\n> \n> test_expect_success 'setup: recover' '\n"},{"id":"216891","messageId":"CALkWK0k_ArM9EpY0S7=mbgFpHsoauZCr9e=ESX_CEA5-vRHrHQ@mail.gmail.com","threadId":"33772","inReplyTo":"7vmws2529j.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T14:46:42Z","receivedAt":"2013-05-10T14:46:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Ramkumar Ramachandra writes:\n>\n>> A couple of tests execute 'git rebase' with GIT_TRACE set to 1, but\n>> this trace output is not used anywhere.\n>\n> Isn't it shown in \"t4300-*.sh -v\" output to help the debugger?\n\nUm, but why the GIT_TRACE in just these two places? Can't I set\nGIT_TRACE=1 when executing the shell script if I really wanted that?\n"},{"id":"216892","messageId":"CAPig+cSW39bnmKOeNDz0H5ZyNzPt3oCLnCUttaMJU9fGuKvJeQ@mail.gmail.com","threadId":"33772","inReplyTo":"1368196178-5807-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/4] t3400 (rebase): don't set GIT_AUTHOR_{NAME,EMAIL}","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-05-10T14:53:16Z","receivedAt":"2013-05-10T14:53:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 10, 2013 at 10:29 AM, Ramkumar Ramachandra\n wrote:\n> test-lib.sh already sets a sane GIT_AUTHOR_{NAME,EMAIL} for all test\n> scripts to use. Don't unnecessarily duplicate the work.\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> t/t3400-rebase.sh | 4 ----\n> 1 file changed, 4 deletions(-)\n>\n> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n> index b58fa1a..a7ca2f1 100755\n> --- a/t/t3400-rebase.sh\n> +++ b/t/t3400-rebase.sh\n> @@ -10,10 +10,6 @@ among other things.\n> '\n> . ./test-lib.sh\n>\n> -GIT_AUTHOR_NAME=author@name\n> -GIT_AUTHOR_EMAIL=bogus@email@address\n\nThese values are intentionally bogus. Doesn't this change defeat the\npurpose of 43c2325 (am: use get_author_ident_from_commit instead of\nmailinfo when rebasing; 2010-06-16)?\n\n> -export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL\n> -\n> test_expect_success 'prepare repository with topic branches' '\n> git config core.logAllRefUpdates true &&\n> echo First >A &&\n"},{"id":"216893","messageId":"CALkWK0nnEg=nyqVE2fGH57+b6h_2FAOZKGWkdL8ZN+H3s-BV9w@mail.gmail.com","threadId":"33772","inReplyTo":"CAPig+cSW39bnmKOeNDz0H5ZyNzPt3oCLnCUttaMJU9fGuKvJeQ@mail.gmail.com","subject":"Re: [PATCH 1/4] t3400 (rebase): don't set GIT_AUTHOR_{NAME,EMAIL}","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T14:56:20Z","receivedAt":"2013-05-10T14:56:20Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Eric Sunshine wrote:\n> These values are intentionally bogus. Doesn't this change defeat the\n> purpose of 43c2325 (am: use get_author_ident_from_commit instead of\n> mailinfo when rebasing; 2010-06-16)?\n\nOh, oops. I paid too little attention to this series.\n\nThanks for catching.\n"},{"id":"216907","messageId":"7vr4he3kpz.fsf@alter.siamese.dyndns.org","threadId":"33772","inReplyTo":"CALkWK0k_ArM9EpY0S7=mbgFpHsoauZCr9e=ESX_CEA5-vRHrHQ@mail.gmail.com","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-10T15:42:16Z","receivedAt":"2013-05-10T15:42:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Junio C Hamano wrote:\n>> Ramkumar Ramachandra writes:\n>>\n>>> A couple of tests execute 'git rebase' with GIT_TRACE set to 1, but\n>>> this trace output is not used anywhere.\n>>\n>> Isn't it shown in \"t4300-*.sh -v\" output to help the debugger?\n>\n> Um, but why the GIT_TRACE in just these two places? Can't I set\n> GIT_TRACE=1 when executing the shell script if I really wanted that?\n\nPerhaps because this is a test about \"rebase\" and a typical debugger\ndoes not want to trace other \"git\" things while debugging this?\n"},{"id":"216913","messageId":"CALkWK0kLjf=m-_9PkXzxOpnpqfh-n004E0f7_mi-VL3SbYWFAg@mail.gmail.com","threadId":"33772","inReplyTo":"7vr4he3kpz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T16:42:00Z","receivedAt":"2013-05-10T16:42:00Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Perhaps because this is a test about \"rebase\" and a typical debugger\n> does not want to trace other \"git\" things while debugging this?\n\nOkay, let's drop this 4-part series: it's too minor.\n"},{"id":"216916","messageId":"7vr4he20yu.fsf@alter.siamese.dyndns.org","threadId":"33772","inReplyTo":"CAPig+cSW39bnmKOeNDz0H5ZyNzPt3oCLnCUttaMJU9fGuKvJeQ@mail.gmail.com","subject":"Re: [PATCH 1/4] t3400 (rebase): don't set GIT_AUTHOR_{NAME,EMAIL}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-10T17:34:17Z","receivedAt":"2013-05-10T17:34:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Fri, May 10, 2013 at 10:29 AM, Ramkumar Ramachandra\n> wrote:\n>> test-lib.sh already sets a sane GIT_AUTHOR_{NAME,EMAIL} for all test\n>> scripts to use. Don't unnecessarily duplicate the work.\n>>\n>> Signed-off-by: Ramkumar Ramachandra \n>> ---\n>> t/t3400-rebase.sh | 4 ----\n>> 1 file changed, 4 deletions(-)\n>>\n>> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n>> index b58fa1a..a7ca2f1 100755\n>> --- a/t/t3400-rebase.sh\n>> +++ b/t/t3400-rebase.sh\n>> @@ -10,10 +10,6 @@ among other things.\n>> '\n>> . ./test-lib.sh\n>>\n>> -GIT_AUTHOR_NAME=author@name\n>> -GIT_AUTHOR_EMAIL=bogus@email@address\n>\n> These values are intentionally bogus. Doesn't this change defeat the\n> purpose of 43c2325 (am: use get_author_ident_from_commit instead of\n> mailinfo when rebasing; 2010-06-16)?\n>\n>> -export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL\n>> -\n\nThen perhaps there needs an in-code comment before these three lines\nto explain why they are deliberately set to bogus values, no?\n"},{"id":"216918","messageId":"7vk3n6206h.fsf@alter.siamese.dyndns.org","threadId":"33772","inReplyTo":"CALkWK0kLjf=m-_9PkXzxOpnpqfh-n004E0f7_mi-VL3SbYWFAg@mail.gmail.com","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-10T17:51:18Z","receivedAt":"2013-05-10T17:51:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Junio C Hamano wrote:\n>> Perhaps because this is a test about \"rebase\" and a typical debugger\n>> does not want to trace other \"git\" things while debugging this?\n>\n> Okay, let's drop this 4-part series: it's too minor.\n\nWhy throw the baby with bathwater?\n\nTo me, most of them look like responses to valid issues, and that\nholds true even for [PATCH 1/4]. Even though your response may have\nbeen an incorrect one, the issue that triggered the response is\nstill valid---the setting of these variables without explanation\ninvites curiosities and a mistake similar to what you made in that\npatch.\n\nIf the patch were to consistently remove \"GIT_TRACE=1\" placed on\n\"git rebase\" from all test scripts that do not check the trace\noutput consistently (i.e. 3400, 3402, and 7402), with a different\njustification, e.g. \"whoever wants to debug a specific part of the\ntest can add GIT_TRACE=1 before running the test with -v, but it\nshould not be committed\", the change would be very reasonable, I\nwould think.\n"},{"id":"216920","messageId":"CALkWK0kTvwxMFGUqH0wKNVpLg-qQQbN+vOcAxiEwmOeDe=87jA@mail.gmail.com","threadId":"33772","inReplyTo":"7vk3n6206h.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T18:06:24Z","receivedAt":"2013-05-10T18:06:24Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> To me, most of them look like responses to valid issues, and that\n> holds true even for [PATCH 1/4]. Even though your response may have\n> been an incorrect one, the issue that triggered the response is\n> still valid---the setting of these variables without explanation\n> invites curiosities and a mistake similar to what you made in that\n> patch.\n\nI like comments, but lately I've seen nothing but reviews stripping my comments.\n\n> If the patch were to consistently remove \"GIT_TRACE=1\" placed on\n> \"git rebase\" from all test scripts that do not check the trace\n> output consistently (i.e. 3400, 3402, and 7402), with a different\n> justification, e.g. \"whoever wants to debug a specific part of the\n> test can add GIT_TRACE=1 before running the test with -v, but it\n> should not be committed\", the change would be very reasonable, I\n> would think.\n\nQuite frankly, I think -v is completely useless; who likes to scroll\nthrough pages of terminal output? If there are really users of this\nthing, why haven't they got it to auto-pager yet?\n\nI just comment out the test_expect_success and close-quote, and put a\ntest_done after it. I would never advocate this GIT_TRACE thing\nanywhere, because I want to put GIT_TRACE=1 (and possibly other\nmodifications) where I want it. Locally.\n"},{"id":"216921","messageId":"CALkWK0kh_wKqCJff7225JyS==5okGPdPPVW2v6Dd65oe0pyVpA@mail.gmail.com","threadId":"33772","inReplyTo":"CALkWK0kTvwxMFGUqH0wKNVpLg-qQQbN+vOcAxiEwmOeDe=87jA@mail.gmail.com","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T18:08:18Z","receivedAt":"2013-05-10T18:08:18Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> I just comment out the test_expect_success and close-quote, and put a\n> test_done after it. I would never advocate this GIT_TRACE thing\n> anywhere, because I want to put GIT_TRACE=1 (and possibly other\n> modifications) where I want it. Locally.\n\nOn that note, I'd really like a switch that runs until the first\nfailing test and re-runs just that failing test with -v.\n"},{"id":"216927","messageId":"20130510190705.GA3478@elie","threadId":"33772","inReplyTo":"CALkWK0kTvwxMFGUqH0wKNVpLg-qQQbN+vOcAxiEwmOeDe=87jA@mail.gmail.com","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-10T19:07:05Z","receivedAt":"2013-05-10T19:07:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Quite frankly, I think -v is completely useless; who likes to scroll\n> through pages of terminal output?\n\nI use \"-v -i\" together quite frequently when debugging. I also use\n\"-v\" automatically to debug test failures when tests are invoked\nautomatically on machines I do not have access to.\n\nPlease don't break it.\n\nNo particular opinion about the use of GIT_TRACE here: I haven't\nlooked at it closely (and in particular I didn't look up the purpose\nof the commit that added it, so I can't really judge).\n\nThanks,\nJonathan\n"},{"id":"216930","messageId":"CALkWK0kZWrY5_B3mxuVV1cA-AqvkqJpjRAuSovXLi6OCA+3ybQ@mail.gmail.com","threadId":"33772","inReplyTo":"20130510190705.GA3478@elie","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-10T19:14:41Z","receivedAt":"2013-05-10T19:14:41Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jonathan Nieder wrote:\n> I use \"-v -i\" together quite frequently when debugging. I also use\n> \"-v\" automatically to debug test failures when tests are invoked\n> automatically on machines I do not have access to.\n\nYeah, it makes sense on remote machines. I just found out about -i,\nand the -v -i combination is useful (and the autopager makes no sense\nin this case); no doubt. Can we do better by not printing the -v\noutput of the passing tests though?\n"},{"id":"216931","messageId":"20130510191639.GB3478@elie","threadId":"33772","inReplyTo":"CALkWK0kZWrY5_B3mxuVV1cA-AqvkqJpjRAuSovXLi6OCA+3ybQ@mail.gmail.com","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-10T19:16:39Z","receivedAt":"2013-05-10T19:16:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Can we do better by not printing the -v\n> output of the passing tests though?\n\nNot for my use. The output from comprable tests before is often\nuseful for comparison. I wouldn't be against such an option for\npeople who want it, though.\n\nThanks,\nJonathan\n"},{"id":"216948","messageId":"7vk3n6zgr3.fsf@alter.siamese.dyndns.org","threadId":"33772","inReplyTo":"20130510191639.GB3478@elie","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-10T21:06:56Z","receivedAt":"2013-05-10T21:06:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Ramkumar Ramachandra wrote:\n>\n>> Can we do better by not printing the -v\n>> output of the passing tests though?\n>\n> Not for my use. The output from comprable tests before is often\n> useful for comparison.\n\nPerhaps.\n\nBut the output from passing \"-v\" before the test that breaks is not\nvery useful for two reasons.\n\n - Often they are identical between passing and failing runs because\n the resulting states in their trash directories are truly the\n same, in which case the output is just being too noisy.\n\n - Some other times, the output from passing and failing runs look\n identical but only because earlier tests are sloppy and not\n showing differences that happens to matter as precondition to the\n failing test, in which case the output is too sketchy to be\n useful.\n\nIn the latter case, where a breakage needs deeper inspection, I\nhaven't found a way better than to stop the test _before_ the broken\none (by temporarily inserting \"exit\" just before it), run it once\nagain so that the trash directory just before the failing test is\nleft behind, and go there to manually inspect the states in the\ntrash directories of passing and failing ones to compare where in\nthe earlier tests that passed left a subtle differences that were\nnot detected but still caused to make a difference to the broken\ntest.\n\nI am not saying \"it is not very useful, so remove it\" but \"it is not\nvery useful, and I wish somebody clever thinks of a way to make it\nmore useful\", though ;-)\n"},{"id":"216990","messageId":"CALkWK0=rqjduiWsj2-iqCoW9tsv4G8gvpf7fCZwg5=TKuEo+bA@mail.gmail.com","threadId":"33772","inReplyTo":"7vk3n6zgr3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-11T11:37:14Z","receivedAt":"2013-05-11T11:37:14Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> But the output from passing \"-v\" before the test that breaks is not\n> very useful for two reasons.\n\nI sometimes checkout the Good branch in a different worktree, compare\nthe output/ state of the passing test with the failing one. I've\nnever really found the outputs from earlier tests enlightening. From\nmy experience, the failure is often due to an earlier test not\nimposing tighter passing conditions: but because it's shell, the\ndebugging time is very small. I always just patch-locally and run.\n\nI'm not sure how to make the testing framework more useful.\n"},{"id":"217245","messageId":"20130513230557.GJ3657@google.com","threadId":"33772","inReplyTo":"CALkWK0=rqjduiWsj2-iqCoW9tsv4G8gvpf7fCZwg5=TKuEo+bA@mail.gmail.com","subject":"Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-13T23:05:57Z","receivedAt":"2013-05-13T23:05:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> I've\n> never really found the outputs from earlier tests enlightening.\n\nIf the test suite would automatically use \"set -x\" when appropriate\nso output for each command was preceded by the command being run,\nthat would presumably make the verbose output more useful.\n\nIf you are not in a situation where it's difficult to debug\ninteractively, I doubt you'll find anything better than reproducing\nthe bug by hand and exploring. I suppose an option \"Run up to this\ntest, stop, and tell me the $TRASH_DIRECTORY and next test so I can\ntry it manually\" could be a way to simplify that workflow.\n\nRegards,\nJonathan\n"}]}