{"thread":{"id":"42114","subject":"make test Unexpected passes","startedAt":"2016-04-22T20:05:24Z","lastAt":"2016-04-27T22:39:42Z","messageCount":6,"participants":["Ramsay Jones","Elijah Newren","Ben Woosley","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"284186","messageId":"571A8404.5030200@ramsayjones.plus.com","threadId":"42114","inReplyTo":null,"subject":"make test Unexpected passes","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-04-22T20:05:24Z","receivedAt":"2016-04-22T20:05:24Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Hi Ben, Junio,\n\nTonight, the testsuite passed with a couple of 'unexpected passes', viz:\n\n$ tail -17 ptest-out\n[13:24:29]\nAll tests successful.\n\nTest Summary Report\n-------------------\nt3421-rebase-topology-linear.sh (Wstat: 0 Tests: 76 Failed: 0)\n TODO passed: 50, 54\nt6036-recursive-corner-cases.sh (Wstat: 0 Tests: 22 Failed: 0)\n TODO passed: 11\nFiles=746, Tests=13515, 445 wallclock secs ( 3.83 usr 0.61 sys + 52.78 cusr 27.89 csys = 85.11 CPU)\nResult: PASS\nmake clean-except-prove-cache\nmake[2]: Entering directory `/home/ramsay/git/t'\nrm -f -r 'trash directory'.* 'test-results'\nrm -f -r valgrind/bin\nmake[2]: Leaving directory `/home/ramsay/git/t'\nmake[1]: Leaving directory `/home/ramsay/git/t'\n$ \n\nIn the first case, t3421-*.sh, git bisect fingered commit f32ec670\n(\"git-rebase--merge: don't include absent parent as a base\", 20-04-2016).\n\nIn the second case, t6036-*.sh, git bisect fingered commit b61f9d6e\n(\"ll-merge: use a longer conflict marker for internal merge\", 14-04-2016).\n\nI won't have any time tonight to look into this any further (are these\nfalse positives?), so I thought I would just make sure you were aware\nof these 'unexpected passes'.\n\nATB,\nRamsay Jones\n"},{"id":"284208","messageId":"CABPp-BFdzu4stEfbGAiqDwRAXt7EcvYBEVz1kChJaR4udC5SXA@mail.gmail.com","threadId":"42114","inReplyTo":"571A8404.5030200@ramsayjones.plus.com","subject":"Re: make test Unexpected passes","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2016-04-22T22:19:51Z","receivedAt":"2016-04-22T22:19:51Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Apr 22, 2016 at 1:05 PM, Ramsay Jones\n wrote:\n> Hi Ben, Junio,\n>\n> In the second case, t6036-*.sh, git bisect fingered commit b61f9d6e\n> (\"ll-merge: use a longer conflict marker for internal merge\", 14-04-2016).\n\nYeah, the t6036 testcase 'git detects conflict w/\ncriss-cross+contrived resolution' could be made to pass by tweaking\nthe conflict markers. In fact, any tweak would make it appear to\npass, but the test could be updated to still fail by updating the\ncontrived resolution of a previous merge to use the newer conflict\nmarker style as well.\n\nThe test is kind of fragile in this way, and is really there just to\ndocument this slightly weird and never seen in practice corner case.\nI don't think we'll ever fix the underlying \"problem\"; not even sure\nif it's possible without fundamentally re-thinking our merge strategy\naltogether. I just thought that when I was writing all those tests\nthat documenting this particular behavior as a testcase had some\nvalue, but if the conflict markers are going to be updated\nperiodically, then the cost of the test may outweigh its value and we\nshould just toss this one test from that file.\n\nElijah\n"},{"id":"284215","messageId":"loom.20160423T011428-888@post.gmane.org","threadId":"42114","inReplyTo":"571A8404.5030200@ramsayjones.plus.com","subject":"Re: make test Unexpected passes","fromName":"Ben Woosley","fromEmail":"ben.woosley@gmail.com","sentAt":"2016-04-22T23:16:16Z","receivedAt":"2016-04-22T23:16:16Z","isPatch":false,"sender":{"key":"ben.woosley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5470?v=4"},"body":"Ramsay Jones ramsayjones.plus.com> writes:\n\n> \n> Hi Ben, Junio,\n> \n> Tonight, the testsuite passed with a couple of 'unexpected passes', viz:\n>\n> In the first case, t3421-*.sh, git bisect fingered commit f32ec670\n> (\"git-rebase--merge: don't include absent parent as a base\", 20-04-2016).\n>\n> ATB,\n> Ramsay Jones\n> \n \nYep,\n\nThese know breakages:\n\nok 50 - rebase -m --onto --root\nok 54 - rebase -m without --onto --root with disjoint history\n\nHave to do with rebasing a root/orphan branch with the -m flag,\nwhich defaults to -- merge=recursive, which is the case the patch fixed.\n\nHere are the necessary changes:\n\n--- a/t/t3421-rebase-topology-linear.sh\n+++ b/t/t3421-rebase-topology-linear.sh\n@@ -253,7 +253,7 @@ test_run_rebase () {\n \t\"\n }\n test_run_rebase success ''\n-test_run_rebase failure -m\n+test_run_rebase success -m\n test_run_rebase success -i\n test_run_rebase success -p\n \n@@ -268,7 +268,7 @@ test_run_rebase () {\n \t\"\n }\n test_run_rebase success ''\n-test_run_rebase failure -m\n+test_run_rebase success -m\n test_run_rebase success -i\n test_run_rebase failure -p\n \n-- \n2.8.1.211.g8e54d77\n"},{"id":"284352","messageId":"xmqqtwiqn7sj.fsf@gitster.mtv.corp.google.com","threadId":"42114","inReplyTo":"loom.20160423T011428-888@post.gmane.org","subject":"Re: make test Unexpected passes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-24T19:09:00Z","receivedAt":"2016-04-24T19:09:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Woosley writes:\n\n> These know breakages:\n>\n> ok 50 - rebase -m --onto --root\n> ok 54 - rebase -m without --onto --root with disjoint history\n>\n> Have to do with rebasing a root/orphan branch with the -m flag,\n> which defaults to -- merge=recursive, which is the case the patch fixed.\n>\n> Here are the necessary changes:\n> ...\n\nThanks, will squash them in and reword the log message accordingly.\n\n git-rebase--merge: don't include absent parent as a base\n\n Absent this fix, attempts to rebase an orphan branch using \"rebase -m\"\n fails with:\n\n $ git rebase -m ORPHAN_TARGET_BASE\n First, rewinding head to replay your work on top of it...\n ...\n Note the default rebase behavior does not fail:\n\n $ git rebase ORPHAN_TARGET_BASE\n First, rewinding head to replay your work on top of it...\n Applying: ORPHAN_ROOT_COMMIT_MSG\n Using index info to reconstruct a base tree...\n\n A few tests were expecting the old behaviour to forbid rebasing such\n a history with \"rebase -m\", which now need to expect them to succeed.\n\n Signed-off-by: Ben Woosley \n Signed-off-by: Junio C Hamano \n"},{"id":"284755","messageId":"xmqqoa8u7lmi.fsf@gitster.mtv.corp.google.com","threadId":"42114","inReplyTo":"CABPp-BFdzu4stEfbGAiqDwRAXt7EcvYBEVz1kChJaR4udC5SXA@mail.gmail.com","subject":"Re: make test Unexpected passes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-27T22:05:57Z","receivedAt":"2016-04-27T22:05:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> Yeah, the t6036 testcase 'git detects conflict w/\n> criss-cross+contrived resolution' could be made to pass by tweaking\n> the conflict markers. In fact, any tweak would make it appear to\n> pass, but the test could be updated to still fail by updating the\n> contrived resolution of a previous merge to use the newer conflict\n> marker style as well.\n\nIsn't what the test expects bogus in the first place? I'd suggest\nremoving the test as \"pointless waste of resource\".\n\nComments?\n\n-- >8 --\nSubject: [PATCH] t6036: remove pointless test that expects failure\n\nOne test in t6036 prepares a file whose contents contain these\nlines:\n\n\t<<<<<<< Temporary merge branch 1\n\tC\n\t=======\n\tB\n\t>>>>>>> Temporary merge branch 2\n\nand uses recursive merge strategy to run criss-cross merge with it.\n\nManual merge resolution by users fundamentally depends on being able\nto tell what is the tracked contents and what is the separator lines\nadded by \"git merge\" to tell users which block of lines came from\nwhere. You can deliberately craft a file with lines that resemble\nconflict marker lines to make it impossible for the user (here, the\nouter merge of merge-recursive also counts as a user of the internal\nmerge) to tell which part is which, and write a test to demonstrate\nthat with such a file that \"git merge\" fundamentally cannot work\nwith and has to fail on. It however is pointless and waste of time\nand resource to run such a test that asserts the obvious.\n\nIn real life, people who do need to track files with such lines that\nhave <<<< ==== >>>> as their prefixes set the conflict-marker-size\nattribute to make sure they will be able to tell between the tracked\nlines that happen to begin with these (confusing) prefixes and the\nmarker lines that are added by \"git merge\".\n\nRemove the test as pointless waste of resource.\n\nSigned-off-by: Junio C Hamano \n---\n t/t6036-recursive-corner-cases.sh | 83 ---------------------------------------\n 1 file changed, 83 deletions(-)\n\ndiff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh\nindex cc1ee6a..c7b0ae6 100755\n--- a/t/t6036-recursive-corner-cases.sh\n+++ b/t/t6036-recursive-corner-cases.sh\n@@ -304,89 +304,6 @@ test_expect_success 'git detects conflict merging criss-cross+modify/delete, rev\n \ttest $(git rev-parse :3:file) = $(git rev-parse B:file)\n '\n \n-#\n-# criss-cross + modify/modify with very contrived file contents:\n-#\n-# B D\n-# o---o\n-# / \\ / \\\n-# A o X ? F\n-# \\ / \\ /\n-# o---o\n-# C E\n-#\n-# Commit A: file with contents 'A\\n'\n-# Commit B: file with contents 'B\\n'\n-# Commit C: file with contents 'C\\n'\n-# Commit D: file with contents 'D\\n'\n-# Commit E: file with contents:\n-# <<<<<<< Temporary merge branch 1\n-# C\n-# =======\n-# B\n-# >>>>>>> Temporary merge branch 2\n-#\n-# Now, when we merge commits D & E, does git detect the conflict?\n-\n-test_expect_success 'setup differently handled merges of content conflict' '\n-\tgit clean -fdqx &&\n-\trm -rf .git &&\n-\tgit init &&\n-\n-\techo A >file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m A &&\n-\n-\tgit branch B &&\n-\tgit checkout -b C &&\n-\techo C >file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m C &&\n-\n-\tgit checkout B &&\n-\techo B >file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m B &&\n-\n-\tgit checkout B^0 &&\n-\ttest_must_fail git merge C &&\n-\techo D >file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m D &&\n-\tgit tag D &&\n-\n-\tgit checkout C^0 &&\n-\ttest_must_fail git merge B &&\n-\tcat <file &&\n-<<<<<<< Temporary merge branch 1\n-C\n-=======\n-B\n->>>>>>> Temporary merge branch 2\n-EOF\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m E &&\n-\tgit tag E\n-'\n-\n-test_expect_failure 'git detects conflict w/ criss-cross+contrived resolution' '\n-\tgit checkout D^0 &&\n-\n-\ttest_must_fail git merge -s recursive E^0 &&\n-\n-\ttest 3 -eq $(git ls-files -s | wc -l) &&\n-\ttest 3 -eq $(git ls-files -u | wc -l) &&\n-\ttest 0 -eq $(git ls-files -o | wc -l) &&\n-\n-\ttest $(git rev-parse :2:file) = $(git rev-parse D:file) &&\n-\ttest $(git rev-parse :3:file) = $(git rev-parse E:file)\n-'\n-\n #\n # criss-cross + d/f conflict via add/add:\n # Commit A: Neither file 'a' nor directory 'a/' exists.\n"},{"id":"284758","messageId":"CABPp-BHFQCmu_xGNXaKAC=PjQ5+QCS9etbNSoQ3PCDHptzRWQA@mail.gmail.com","threadId":"42114","inReplyTo":"xmqqoa8u7lmi.fsf@gitster.mtv.corp.google.com","subject":"Re: make test Unexpected passes","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2016-04-27T22:39:42Z","receivedAt":"2016-04-27T22:39:42Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Apr 27, 2016 at 3:05 PM, Junio C Hamano wrote:\n> Isn't what the test expects bogus in the first place? I'd suggest\n> removing the test as \"pointless waste of resource\".\n>\n> Comments?\n>\n> -- >8 --\n\nYes, toss it; I find your arguments below compelling.\n\n> Manual merge resolution by users fundamentally depends on being able\n> to tell what is the tracked contents and what is the separator lines\n> added by \"git merge\" to tell users which block of lines came from\n> where. You can deliberately craft a file with lines that resemble\n> conflict marker lines to make it impossible for the user (here, the\n> outer merge of merge-recursive also counts as a user of the internal\n> merge) to tell which part is which, and write a test to demonstrate\n> that with such a file that \"git merge\" fundamentally cannot work\n> with and has to fail on. It however is pointless and waste of time\n> and resource to run such a test that asserts the obvious.\n>\n> In real life, people who do need to track files with such lines that\n> have <<<< ==== >>>> as their prefixes set the conflict-marker-size\n> attribute to make sure they will be able to tell between the tracked\n> lines that happen to begin with these (confusing) prefixes and the\n> marker lines that are added by \"git merge\".\n"}]}