{"thread":{"id":"29121","subject":"[PATCH 2/2] t3401: use test_commit in setup","startedAt":"2011-12-09T16:59:11Z","lastAt":"2011-12-10T08:14:43Z","messageCount":7,"participants":["Martin von Zweigbergk","Ramkumar Ramachandra","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"180673","messageId":"1323449952-14161-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"29121","inReplyTo":null,"subject":"[PATCH 1/2] t3401: modernize style","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-12-09T16:59:11Z","receivedAt":"2011-12-09T16:59:11Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Put the opening quote starting each test on the same line as the\ntest_expect_* invocation. Also make sure to use tabs for indentation.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n t/t3401-rebase-partial.sh |   67 ++++++++++++++++++++++-----------------------\n 1 files changed, 33 insertions(+), 34 deletions(-)\n\ndiff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh\nindex aea6685..d7c874c 100755\n--- a/t/t3401-rebase-partial.sh\n+++ b/t/t3401-rebase-partial.sh\n@@ -11,51 +11,50 @@ local branch.\n '\n . ./test-lib.sh\n \n-test_expect_success \\\n-    'prepare repository with topic branch' \\\n-    'echo First > A &&\n-     git update-index --add A &&\n-     git commit -m \"Add A.\" &&\n+test_expect_success 'prepare repository with topic branch' '\n+\techo First > A &&\n+\tgit update-index --add A &&\n+\tgit commit -m \"Add A.\" &&\n \n-     git checkout -b my-topic-branch &&\n+\tgit checkout -b my-topic-branch &&\n \n-     echo Second > B &&\n-     git update-index --add B &&\n-     git commit -m \"Add B.\" &&\n+\techo Second > B &&\n+\tgit update-index --add B &&\n+\tgit commit -m \"Add B.\" &&\n \n-     echo AnotherSecond > C &&\n-     git update-index --add C &&\n-     git commit -m \"Add C.\" &&\n+\techo AnotherSecond > C &&\n+\tgit update-index --add C &&\n+\tgit commit -m \"Add C.\" &&\n \n-     git checkout -f master &&\n+\tgit checkout -f master &&\n \n-     echo Third >> A &&\n-     git update-index A &&\n-     git commit -m \"Modify A.\"\n+\techo Third >> A &&\n+\tgit update-index A &&\n+\tgit commit -m \"Modify A.\"\n '\n \n-test_expect_success \\\n-    'pick top patch from topic branch into master' \\\n-    'git cherry-pick my-topic-branch^0 &&\n-     git checkout -f my-topic-branch &&\n-     git branch master-merge master &&\n-     git branch my-topic-branch-merge my-topic-branch\n+test_expect_success 'pick top patch from topic branch into master' '\n+\tgit cherry-pick my-topic-branch^0 &&\n+\tgit checkout -f my-topic-branch &&\n+\tgit branch master-merge master &&\n+\tgit branch my-topic-branch-merge my-topic-branch\n '\n \n-test_debug \\\n-    'git cherry master &&\n-     git format-patch -k --stdout --full-index master >/dev/null &&\n-     gitk --all & sleep 1\n+test_debug '\n+\tgit cherry master &&\n+\tgit format-patch -k --stdout --full-index master >/dev/null &&\n+\tgitk --all & sleep 1\n '\n \n-test_expect_success \\\n-    'rebase topic branch against new master and check git am did not get halted' \\\n-    'git rebase master && test ! -d .git/rebase-apply'\n+test_expect_success 'rebase topic branch against new master and check git am did not get halted' '\n+\tgit rebase master &&\n+\ttest ! -d .git/rebase-apply\n+'\n \n-test_expect_success \\\n-\t'rebase --merge topic branch that was partially merged upstream' \\\n-\t'git checkout -f my-topic-branch-merge &&\n-\t git rebase --merge master-merge &&\n-\t test ! -d .git/rebase-merge'\n+test_expect_success 'rebase --merge topic branch that was partially merged upstream' '\n+\tgit checkout -f my-topic-branch-merge &&\n+\tgit rebase --merge master-merge &&\n+\ttest ! -d .git/rebase-merge\n+'\n \n test_done\n-- \n1.7.8.237.gcc4e3\n"},{"id":"180672","messageId":"1323449952-14161-2-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"29121","inReplyTo":"1323449952-14161-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH 2/2] t3401: use test_commit in setup","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-12-09T16:59:12Z","receivedAt":"2011-12-09T16:59:12Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Simplify t3401 by using test_commit in the setup. This lets us refer\nto commits using their tags and there is no longer a need to create\nthe branch my-topic-branch-merge. Also, the branch master-merge points\nto the same commit as master (even before this change), so that branch\ndoes not need to be created either.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n t/t3401-rebase-partial.sh |   31 ++++++++-----------------------\n 1 files changed, 8 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh\nindex d7c874c..1aac22c 100755\n--- a/t/t3401-rebase-partial.sh\n+++ b/t/t3401-rebase-partial.sh\n@@ -12,32 +12,17 @@ local branch.\n . ./test-lib.sh\n \n test_expect_success 'prepare repository with topic branch' '\n-\techo First > A &&\n-\tgit update-index --add A &&\n-\tgit commit -m \"Add A.\" &&\n-\n+\ttest_commit A &&\n \tgit checkout -b my-topic-branch &&\n-\n-\techo Second > B &&\n-\tgit update-index --add B &&\n-\tgit commit -m \"Add B.\" &&\n-\n-\techo AnotherSecond > C &&\n-\tgit update-index --add C &&\n-\tgit commit -m \"Add C.\" &&\n-\n+\ttest_commit B &&\n+\ttest_commit C &&\n \tgit checkout -f master &&\n-\n-\techo Third >> A &&\n-\tgit update-index A &&\n-\tgit commit -m \"Modify A.\"\n+\ttest_commit A2 A.t\n '\n \n test_expect_success 'pick top patch from topic branch into master' '\n-\tgit cherry-pick my-topic-branch^0 &&\n-\tgit checkout -f my-topic-branch &&\n-\tgit branch master-merge master &&\n-\tgit branch my-topic-branch-merge my-topic-branch\n+\tgit cherry-pick C &&\n+\tgit checkout -f my-topic-branch\n '\n \n test_debug '\n@@ -52,8 +37,8 @@ test_expect_success 'rebase topic branch against new master and check git am did\n '\n \n test_expect_success 'rebase --merge topic branch that was partially merged upstream' '\n-\tgit checkout -f my-topic-branch-merge &&\n-\tgit rebase --merge master-merge &&\n+\tgit reset --hard C &&\n+\tgit rebase --merge master &&\n \ttest ! -d .git/rebase-merge\n '\n \n-- \n1.7.8.237.gcc4e3\n"},{"id":"180684","messageId":"CALkWK0mip_pzUDQO=YLxaVwsWEAUdrk_EKcNG94Xr5+N+kzBtw@mail.gmail.com","threadId":"29121","inReplyTo":"1323449952-14161-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH 1/2] t3401: modernize style","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T18:51:58Z","receivedAt":"2011-12-09T18:51:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Martin,\n\nNo cover letter, so I'm assuming these are just two random one-off\npatches. The motivation is unclear: lazy afternoon? :P\n\nMartin von Zweigbergk wrote:\n> [...]\n> diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh\n> index aea6685..d7c874c 100755\n> --- a/t/t3401-rebase-partial.sh\n> +++ b/t/t3401-rebase-partial.sh\n> @@ -11,51 +11,50 @@ local branch.\n>  '\n>  . ./test-lib.sh\n>\n> -test_expect_success \\\n> -    'prepare repository with topic branch' \\\n> -    'echo First > A &&\n> -     git update-index --add A &&\n> -     git commit -m \"Add A.\" &&\n> +test_expect_success 'prepare repository with topic branch' '\n> +       echo First > A &&\n> +       git update-index --add A &&\n> +       git commit -m \"Add A.\" &&\n\nStyle nit: >[^ ] is prevalent FWIW.\n\n$ git grep '> [^ ]' t/ | wc -l\n3091\n$ git grep '>[^ ]' t/ | wc -l\n9271\n\nSure, the regular expressions aren't tailored to make sure that only\nredirections are caught, but I suppose it's safe to assume that one\nnumber is significantly larger than the other.\n\n> [...]\n> -test_expect_success \\\n> -    'rebase topic branch against new master and check git am did not get halted' \\\n> -    'git rebase master && test ! -d .git/rebase-apply'\n> +test_expect_success 'rebase topic branch against new master and check git am did not get halted' '\n> +       git rebase master &&\n> +       test ! -d .git/rebase-apply\n> +'\n\nWhile at it, why not change this \"test ! -d\" to\n\"test_path_is_missing\"?  My rationale is that you're touching the file\nanyway to do a generic cleanup; might as well finish it.\n\nThanks.\n\n-- Ram\n"},{"id":"180685","messageId":"CALkWK0k-dL3xZ+dyqdACj7man-Q2QrAPZCCMhXiX0WNGZHv6Fw@mail.gmail.com","threadId":"29121","inReplyTo":"1323449952-14161-2-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH 2/2] t3401: use test_commit in setup","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-09T19:02:13Z","receivedAt":"2011-12-09T19:02:13Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi again,\n\nMartin von Zweigbergk wrote:\n> Simplify t3401 by using test_commit in the setup. This lets us refer\n> to commits using their tags and there is no longer a need to create\n> the branch my-topic-branch-merge. Also, the branch master-merge points\n> to the same commit as master (even before this change), so that branch\n> does not need to be created either.\n\nThe terms \"tag\" and \"branch\" here have no significance, so\nde-emphasizing them to \"ref\" is probably a good idea.  Isn't the truth\nmore like: instead of creating commits and creating refs to track\nthose commits by hand, use test_commit to achieve the same result in a\nsingle step?\n\nCheers.\n\n-- Ram\n"},{"id":"180697","messageId":"20111209195752.GA21260@elie.hsd1.il.comcast.net","threadId":"29121","inReplyTo":"CALkWK0k-dL3xZ+dyqdACj7man-Q2QrAPZCCMhXiX0WNGZHv6Fw@mail.gmail.com","subject":"Re: [PATCH 2/2] t3401: use test_commit in setup","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-09T19:57:52Z","receivedAt":"2011-12-09T19:57:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Martin von Zweigbergk wrote:\n\n>> Simplify t3401 by using test_commit in the setup. This lets us refer\n>> to commits using their tags and there is no longer a need to create\n>> the branch my-topic-branch-merge. Also, the branch master-merge points\n>> to the same commit as master (even before this change), so that branch\n>> does not need to be created either.\n>\n> The terms \"tag\" and \"branch\" here have no significance, so\n> de-emphasizing them to \"ref\" is probably a good idea.\n\nWha?  No, \"tag\" refers to refs under refs/tags/, and \"branch\" refers to\nrefs/heads/, just like usual.\n\nI like the patch, for what it's worth.\n"},{"id":"180699","messageId":"20111209200703.GA21280@elie.hsd1.il.comcast.net","threadId":"29121","inReplyTo":"CALkWK0mip_pzUDQO=YLxaVwsWEAUdrk_EKcNG94Xr5+N+kzBtw@mail.gmail.com","subject":"Re: [PATCH 1/2] t3401: modernize style","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-09T20:07:03Z","receivedAt":"2011-12-09T20:07:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> The motivation is unclear: lazy afternoon? :P\n\nPerhaps he was reading the list and after noticing a few patches in\nthe same vein, realized that this test script could be made easier to\nread, too.\n\n[...]\n> Martin von Zweigbergk wrote:\n\n>> +       echo First > A &&\n>> +       git update-index --add A &&\n>> +       git commit -m \"Add A.\" &&\n>\n> Style nit: >[^ ] is prevalent FWIW.\n\nAt first it wasn't clear to me what you meant here.  Was it that\nquoted text in an email should start with a non-space character, like\na tab?\n\nFinally I caught on that you mean that redirection operators tend to\nbe flush against the filename they are redirecting to.\n\n[...]\n>> +       test ! -d .git/rebase-apply\n>> +'\n>\n> While at it, why not change this \"test ! -d\" to\n> \"test_path_is_missing\"?\n\nSounds like a useful hint.  The benefits are that it would catch\nfailures that make .git/rebase-apply into an ordinary file, and more\nuseful output from \"sh t3401-* -v -i\" when the test fails.  The\nmain downside I can think of is that the test script would not run\nagainst versions of the test harness before v1.7.3.3~5^2~1 (test-lib:\nuser-friendly alternatives to test [-d|-f|-e], 2010-08-10).\n\nThe patch looks good to me, too.  Thanks, both.\n\nSincerely,\nJonathan\n"},{"id":"180748","messageId":"CAOeW2eGbwQZRDXn0crMD2eMTkw3eqWeCS3x2p4VhT65q45cs0Q@mail.gmail.com","threadId":"29121","inReplyTo":"20111209200703.GA21280@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 1/2] t3401: modernize style","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-12-10T08:14:43Z","receivedAt":"2011-12-10T08:14:43Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Fri, Dec 9, 2011 at 12:07 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> The motivation is unclear: lazy afternoon? :P\n>\n> Perhaps he was reading the list and after noticing a few patches in\n> the same vein, realized that this test script could be made easier to\n> read, too.\n\nSort of. These patches have been sitting in my repo since late Sept\nand the patches you mention made me decide to send them out now. The\nreason I did this back then was while trying to fix rebase to pick the\nright patches when used with --onto. See this old discussion:\nhttp://thread.gmane.org/gmane.comp.version-control.git/161917/focus=162041.\nAlso in the same series are patches teach rebase to only feed the\ncommit names to git-am (wrapped in a silly \"From $sha1 Mon\nSep 17 00:00:00 2001\" to please git-mailsplit). These patches have\nbeen taking way too long, which is why I'm sending these little\ncleanups separately.\n\n>> Martin von Zweigbergk wrote:\n>\n>>> +       echo First > A &&\n>>> +       git update-index --add A &&\n>>> +       git commit -m \"Add A.\" &&\n>>\n>> Style nit: >[^ ] is prevalent FWIW.\n>\n> Finally I caught on that you mean that redirection operators tend to\n> be flush against the filename they are redirecting to.\n\nSo did I. I think I'll leave the code unchanged, though, because the\nend result, after patch 2/2 is unaffected anyways (it removes\nredirections).\n\n>> While at it, why not change this \"test ! -d\" to\n>> \"test_path_is_missing\"?\n\nWill bake it into patch 2/2 if you don't mind. Unless there are other\ncomments, that would mean this patch can be applies as is, Junio.\n\n> The patch looks good to me, too.  Thanks, both.\n\nThanks to you both, too.\n"}]}