{"thread":{"id":"11217","subject":"[PATCH] Add more checkout tests","startedAt":"2007-12-10T03:05:34Z","lastAt":"2007-12-10T05:14:03Z","messageCount":4,"participants":["Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"62533","messageId":"Pine.LNX.4.64.0712092204200.5349@iabervon.org","threadId":"11217","inReplyTo":null,"subject":"[PATCH] Add more checkout tests","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-12-10T03:05:34Z","receivedAt":"2007-12-10T03:05:34Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"If you have local changes that don't conflict with the\nbranch-switching changes, these should be kept, not cause errors even\nwithout -m, and be reported afterwards in name-status format.\n\nWith -m, the changes carried across should be listed as well. And, for \nnow, include the merge-recursive output from this process.\n\nAlso test the detatched head message in at least one case.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n t/t7201-co.sh |   46 +++++++++++++++++++++++++++++++++++++++++++---\n 1 files changed, 43 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 55558ab..3cf0cf1 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -20,6 +20,8 @@ Test switching across them.\n \n . ./test-lib.sh\n \n+test_tick\n+\n fill () {\n \tfor i\n \tdo\n@@ -30,9 +32,10 @@ fill () {\n \n test_expect_success setup '\n \n+\tfill x y z > same &&\n \tfill 1 2 3 4 5 6 7 8 >one &&\n \tfill a b c d e >two &&\n-\tgit add one two &&\n+\tgit add same one two &&\n \tgit commit -m \"Initial A one, A two\" &&\n \n \tgit checkout -b renamer &&\n@@ -74,16 +77,44 @@ test_expect_success \"checkout with dirty tree without -m\" '\n \n '\n \n+test_expect_success \"checkout with unrelated dirty tree without -m\" '\n+\n+\tgit checkout -f master &&\n+\tfill 0 1 2 3 4 5 6 7 8 >same &&\n+\tcp same kept\n+\tgit checkout side >messages && \n+\tgit diff same kept\n+\t(cat > messages.expect <<EOF\n+M\tsame\n+EOF\n+) &&\n+\ttouch messages.expect &&\n+\tgit diff messages.expect messages\n+'\n+\n test_expect_success \"checkout -m with dirty tree\" '\n \n \tgit checkout -f master &&\n \tgit clean -f &&\n \n \tfill 0 1 2 3 4 5 6 7 8 >one &&\n-\tgit checkout -m side &&\n+\tgit checkout -m side > messages &&\n \n \ttest \"$(git symbolic-ref HEAD)\" = \"refs/heads/side\" &&\n \n+\t(cat >expect.messages <<EOF\n+Merging side with local\n+Merging:\n+ab76817 Side M one, D two, A three\n+virtual local\n+found 1 common ancestor(s):\n+7329388 Initial A one, A two\n+Auto-merged one\n+M\tone\n+EOF\n+) &&\n+\tgit diff expect.messages messages &&\n+\n \tfill \"M\tone\" \"A\tthree\" \"D\ttwo\" >expect.master &&\n \tgit diff --name-status master >current.master &&\n \tdiff expect.master current.master &&\n@@ -145,7 +176,16 @@ test_expect_success 'checkout -m with merge conflict' '\n test_expect_success 'checkout to detach HEAD' '\n \n \tgit checkout -f renamer && git clean -f &&\n-\tgit checkout renamer^ &&\n+\tgit checkout renamer^ 2>messages &&\n+\t(cat >messages.expect <<EOF\n+Note: moving to \"renamer^\" which isn'\"'\"'t a local branch\n+If you want to create a new branch from this checkout, you may do so\n+(now or later) by using -b with the checkout command again. Example:\n+  git checkout -b <new_branch_name>\n+HEAD is now at 7329388... Initial A one, A two\n+EOF\n+) &&\n+\tgit diff messages.expect messages &&\n \tH=$(git rev-parse --verify HEAD) &&\n \tM=$(git show-ref -s --verify refs/heads/master) &&\n \ttest \"z$H\" = \"z$M\" &&\n-- \n1.5.3.6.886.gb204\n"},{"id":"62535","messageId":"7vprxfmczi.fsf@gitster.siamese.dyndns.org","threadId":"11217","inReplyTo":"Pine.LNX.4.64.0712092204200.5349@iabervon.org","subject":"Re: [PATCH] Add more checkout tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-10T03:42:57Z","receivedAt":"2007-12-10T03:42:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> +test_expect_success \"checkout with unrelated dirty tree without -m\" '\n> +\n> +\tgit checkout -f master &&\n> +\tfill 0 1 2 3 4 5 6 7 8 >same &&\n> +\tcp same kept\n> +\tgit checkout side >messages && \n> +\tgit diff same kept\n> +\t(cat > messages.expect <<EOF\n> +M\tsame\n> +EOF\n> +) &&\n> +\ttouch messages.expect &&\n> +\tgit diff messages.expect messages\n> +'\n\nWhat is this \"touch\" about?\n\nI do not recall the details, but we had problem reports that some shells\ndo not handle here-documents (i.e. cmd <<EOF) inside test_expect_success\nwell, and generally tried to keep them outside.  I however see some of\nthe newer tests do have here-doc inside expect-success, and do not\nrecall hearing breakage reports on them.  Maybe it was a false alarm and\nwe were being overly cautious, or maybe not enough people (especially on\nmore exotic systems) are running tests these days.  Let's keep the\nhere-doc as-is in your tests and see what happens.\n\n>  test_expect_success \"checkout -m with dirty tree\" '\n>  \n>  \tgit checkout -f master &&\n>  \tgit clean -f &&\n>  \n>  \tfill 0 1 2 3 4 5 6 7 8 >one &&\n> -\tgit checkout -m side &&\n> +\tgit checkout -m side > messages &&\n>  \n>  \ttest \"$(git symbolic-ref HEAD)\" = \"refs/heads/side\" &&\n>  \n> +\t(cat >expect.messages <<EOF\n> +Merging side with local\n> +Merging:\n> +ab76817 Side M one, D two, A three\n> +virtual local\n> +found 1 common ancestor(s):\n> +7329388 Initial A one, A two\n> +Auto-merged one\n> +M\tone\n> +EOF\n> +) &&\n> +\tgit diff expect.messages messages &&\n\nI do not like the idea of testing the exact wording of messages this\nway.\n\nI do not think we care about the exact wording of these messages, and I\nthink our tests should check what we do care about without casting the\nUI in stone.  Otherwise, it will make it harder to clean-up the user\nexperience later.  Perhaps it would be sufficient to make sure that (1)\nthis checkout succeeds with exit 0 status, and that (2) the contents of\nthe merged 'one' is a reasonable merge result, i.e. \"git diff HEAD one\"\ngets the same patch-id as \"git diff HEAD one\" taken before switching the\nbranches.\n\n> @@ -145,7 +176,16 @@ test_expect_success 'checkout -m with merge conflict' '\n>  test_expect_success 'checkout to detach HEAD' '\n>  \n>  \tgit checkout -f renamer && git clean -f &&\n> -\tgit checkout renamer^ &&\n> +\tgit checkout renamer^ 2>messages &&\n> +\t(cat >messages.expect <<EOF\n> +Note: moving to \"renamer^\" which isn'\"'\"'t a local branch\n> +If you want to create a new branch from this checkout, you may do so\n> +(now or later) by using -b with the checkout command again. Example:\n> +  git checkout -b <new_branch_name>\n> +HEAD is now at 7329388... Initial A one, A two\n> +EOF\n> +) &&\n> +\tgit diff messages.expect messages &&\n\nSame here.  If we want to make sure the head is detached at the intended\ncommit, make sure \"rev-parse HEAD\" gives the expected result, and make\nsure \"symbolic-ref HEAD\" says it is not symbolic.\n\n>  \tH=$(git rev-parse --verify HEAD) &&\n>  \tM=$(git show-ref -s --verify refs/heads/master) &&\n>  \ttest \"z$H\" = \"z$M\" &&\n> -- \n> 1.5.3.6.886.gb204\n"},{"id":"62537","messageId":"Pine.LNX.4.64.0712092244570.5349@iabervon.org","threadId":"11217","inReplyTo":"7vprxfmczi.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add more checkout tests","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-12-10T04:03:28Z","receivedAt":"2007-12-10T04:03:28Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 9 Dec 2007, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > +test_expect_success \"checkout with unrelated dirty tree without -m\" '\n> > +\n> > +\tgit checkout -f master &&\n> > +\tfill 0 1 2 3 4 5 6 7 8 >same &&\n> > +\tcp same kept\n> > +\tgit checkout side >messages && \n> > +\tgit diff same kept\n> > +\t(cat > messages.expect <<EOF\n> > +M\tsame\n> > +EOF\n> > +) &&\n> > +\ttouch messages.expect &&\n> > +\tgit diff messages.expect messages\n> > +'\n> \n> What is this \"touch\" about?\n\nLeft over from before I'd added the here document, so there'd be a file to \ndiff against, and it would be wrong, and I could find the actual contents. \nI just forgot to take it out when I was creating the real thing.\n\n> I do not recall the details, but we had problem reports that some shells\n> do not handle here-documents (i.e. cmd <<EOF) inside test_expect_success\n> well, and generally tried to keep them outside.  I however see some of\n> the newer tests do have here-doc inside expect-success, and do not\n> recall hearing breakage reports on them.  Maybe it was a false alarm and\n> we were being overly cautious, or maybe not enough people (especially on\n> more exotic systems) are running tests these days.  Let's keep the\n> here-doc as-is in your tests and see what happens.\n\nSure. Possibly we should instead just be testing for the presence of the \ncorrect line, and absence of incorrect lines, in any case?\n\n> >  test_expect_success \"checkout -m with dirty tree\" '\n> >  \n> >  \tgit checkout -f master &&\n> >  \tgit clean -f &&\n> >  \n> >  \tfill 0 1 2 3 4 5 6 7 8 >one &&\n> > -\tgit checkout -m side &&\n> > +\tgit checkout -m side > messages &&\n> >  \n> >  \ttest \"$(git symbolic-ref HEAD)\" = \"refs/heads/side\" &&\n> >  \n> > +\t(cat >expect.messages <<EOF\n> > +Merging side with local\n> > +Merging:\n> > +ab76817 Side M one, D two, A three\n> > +virtual local\n> > +found 1 common ancestor(s):\n> > +7329388 Initial A one, A two\n> > +Auto-merged one\n> > +M\tone\n> > +EOF\n> > +) &&\n> > +\tgit diff expect.messages messages &&\n> \n> I do not like the idea of testing the exact wording of messages this\n> way.\n> \n> I do not think we care about the exact wording of these messages, and I\n> think our tests should check what we do care about without casting the\n> UI in stone.  Otherwise, it will make it harder to clean-up the user\n> experience later.  Perhaps it would be sufficient to make sure that (1)\n> this checkout succeeds with exit 0 status, and that (2) the contents of\n> the merged 'one' is a reasonable merge result, i.e. \"git diff HEAD one\"\n> gets the same patch-id as \"git diff HEAD one\" taken before switching the\n> branches.\n\nWhat I'm actually testing for is the \"M<tab>one\" line, and that the \nprevious line isn't name-status stuff; that is, that the name-status stuff \nis right.\n\nOf course, a patch to clean up the user experience could have a hunk that \nmakes the test expect that the UI is cleaned up. It's not like we can't \nchange our tests to accompany improvements in behavior, and I'd argue that \nthose hunks give a useful example of the improvement.\n\n> > @@ -145,7 +176,16 @@ test_expect_success 'checkout -m with merge conflict' '\n> >  test_expect_success 'checkout to detach HEAD' '\n> >  \n> >  \tgit checkout -f renamer && git clean -f &&\n> > -\tgit checkout renamer^ &&\n> > +\tgit checkout renamer^ 2>messages &&\n> > +\t(cat >messages.expect <<EOF\n> > +Note: moving to \"renamer^\" which isn'\"'\"'t a local branch\n> > +If you want to create a new branch from this checkout, you may do so\n> > +(now or later) by using -b with the checkout command again. Example:\n> > +  git checkout -b <new_branch_name>\n> > +HEAD is now at 7329388... Initial A one, A two\n> > +EOF\n> > +) &&\n> > +\tgit diff messages.expect messages &&\n> \n> Same here.  If we want to make sure the head is detached at the intended\n> commit, make sure \"rev-parse HEAD\" gives the expected result, and make\n> sure \"symbolic-ref HEAD\" says it is not symbolic.\n\nI think we're already testing for that. I really want the \"HEAD is now...\" \nline, which ought to give the right info.\n\nThe point of adding these tests (and parts of tests) is that I'd forgotten \nto maintain some of the important information while writing \nbuiltin-checkout, and there wasn't a test that it was provided. While some \nof the output is arbitrary informative text, there's a certain amount of \ngenerated information there that shouldn't get lost or be incorrect.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"62540","messageId":"7vlk83m8ro.fsf@gitster.siamese.dyndns.org","threadId":"11217","inReplyTo":"Pine.LNX.4.64.0712092244570.5349@iabervon.org","subject":"Re: [PATCH] Add more checkout tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-10T05:14:03Z","receivedAt":"2007-12-10T05:14:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> Of course, a patch to clean up the user experience could have a hunk that \n> makes the test expect that the UI is cleaned up. It's not like we can't \n> change our tests to accompany improvements in behavior, and I'd argue that \n> those hunks give a useful example of the improvement.\n\nOk.\n\n> The point of adding these tests (and parts of tests) is that I'd forgotten \n> to maintain some of the important information while writing \n> builtin-checkout, and there wasn't a test that it was provided. While some \n> of the output is arbitrary informative text, there's a certain amount of \n> generated information there that shouldn't get lost or be incorrect.\n\nFair enough.\n"}]}