# [PATCH] New test for preserve merges and squash 8 messages from 2008-03-20 to 2008-03-21. Participants: Jörg Sommer, Junio C Hamano, Johannes Schindelin. Thread: https://gitlist.dev/t/12767 ## Jörg Sommer, 2008-03-20 00:03 Subject: [PATCH] New test for preserve merges and squash Message-ID: <1205971432-12641-1-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1205971432-12641-1-git-send-email-joerg%40alea.gnuu.de ``` Signed-off-by: Jörg Sommer --- t/t3404-rebase-interactive.sh | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) The current version of git fails this test. I think it's a bug, because the patch is what I expect to happen. Or am I wrong? diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 62e65d7..05f3828 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -213,6 +213,26 @@ test_expect_success 'preserve merges with -p' ' test $(git show HEAD~2:file1) = B ' +# This test uses to-be-preserved from the test 'preserve merges with -p' +test_expect_success 'squash and preserve merges' ' + test_tick && + git checkout -b squash-and-preserve-merges master && + echo A > file1 && + git commit -m SaPM-1 file1 && + echo B > file1 && + git commit -m SaPM-2 file1 && + git merge to-be-preserved && + echo C > file1 && + git commit -m SaPM-3 file1 && + + EXPECT_COUNT=4 FAKE_LINES="1 2 squash 4 3" \ + git rebase -i -p --onto branch1 master && + test $(git rev-parse HEAD^2) = $(git rev-parse to-be-preserved) && + test $(git rev-parse HEAD~3) = $(git rev-parse branch1) && + test $(git show HEAD:file1) = C && + test $(git show HEAD~2:file1) = A +' + test_expect_success '--continue tries to commit' ' test_tick && ! git rebase -i --onto new-branch1 HEAD^ && -- 1.5.4.4 ``` ## Junio C Hamano, 2008-03-20 00:28 Subject: Re: [PATCH] New test for preserve merges and squash Message-ID: <7v1w661bnx.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v1w661bnx.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1205971432-12641-1-git-send-email-joerg@alea.gnuu.de> ``` Jörg Sommer writes: > Signed-off-by: Jörg Sommer > --- > t/t3404-rebase-interactive.sh | 20 ++++++++++++++++++++ > 1 files changed, 20 insertions(+), 0 deletions(-) > > The current version of git fails this test. I think it's a bug, because > the patch is what I expect to happen. Or am I wrong? Please mark such tests with test_expect_fail. And also Cc: the guilty parties. $ git-blame master -- git-rebase--interactive.sh $ git shortlog -n -s master -- git-rebase--interactive.sh would tell you who they are ;-). > +test_expect_success 'squash and preserve merges' ' > + test_tick && > + git checkout -b squash-and-preserve-merges master && > + echo A > file1 && > + git commit -m SaPM-1 file1 && > + echo B > file1 && > + git commit -m SaPM-2 file1 && > + git merge to-be-preserved && > + echo C > file1 && > + git commit -m SaPM-3 file1 && > + > + EXPECT_COUNT=4 FAKE_LINES="1 2 squash 4 3" \ > + git rebase -i -p --onto branch1 master && > + test $(git rev-parse HEAD^2) = $(git rev-parse to-be-preserved) && > + test $(git rev-parse HEAD~3) = $(git rev-parse branch1) && > + test $(git show HEAD:file1) = C && > + test $(git show HEAD~2:file1) = A > +' Please split such a test into two parts, a part that sets things up (which would succeed), and the part that runs the command it tries to test ("rebase -i" followed by the validation of the result, which you are expecting to fail). That way, somebody who is trying to fix the breakage can stop the test script by inserting "exit" between the two, run the rebase command by hand to see what is going on, etc. to diagnose the issue. ``` ## Jörg Sommer, 2008-03-20 10:18 Subject: Re: [PATCH] New test for preserve merges and squash Message-ID: <20080320101809.GA23108@alea.gnuu.de> URL: https://gitlist.dev/e/20080320101809.GA23108%40alea.gnuu.de In-Reply-To: <7v1w661bnx.fsf@gitster.siamese.dyndns.org> ``` Hi, Junio C Hamano schrieb am Wed 19. Mar, 17:28 (-0700): > Jörg Sommer writes: > > > Signed-off-by: Jörg Sommer > > --- > > t/t3404-rebase-interactive.sh | 20 ++++++++++++++++++++ > > 1 files changed, 20 insertions(+), 0 deletions(-) > > > > The current version of git fails this test. I think it's a bug, because > > the patch is what I expect to happen. Or am I wrong? > > Please mark such tests with test_expect_fail. No, I expect the test succeeds. Currently, it fails. This might be, because my expectations are wrong or there's a bug. > And also Cc: the guilty parties. Okay, I've added Johannes the author of f09c9b8c5ff to the Cc: list. Johannes, can you explain why this test fails? Bye, Jörg. -- > Definiere ‚Demokratie‘ … … eine Mehrheit beweist einer Minderheit, dass Widerstand zwecklos ist. ``` ## Johannes Schindelin, 2008-03-20 11:09 Subject: Re: [PATCH] New test for preserve merges and squash Message-ID: URL: https://gitlist.dev/e/alpine.LSU.1.00.0803201208080.3983%40racer.site In-Reply-To: <20080320101809.GA23108@alea.gnuu.de> ``` Hi, On Thu, 20 Mar 2008, Jörg Sommer wrote: > Junio C Hamano schrieb am Wed 19. Mar, 17:28 (-0700): > > Jörg Sommer writes: > > > > > Signed-off-by: Jörg Sommer > > > --- > > > t/t3404-rebase-interactive.sh | 20 ++++++++++++++++++++ > > > 1 files changed, 20 insertions(+), 0 deletions(-) > > > > > > The current version of git fails this test. I think it's a bug, > > > because the patch is what I expect to happen. Or am I wrong? > > > > Please mark such tests with test_expect_fail. > > No, I expect the test succeeds. Currently, it fails. This might be, > because my expectations are wrong or there's a bug. The definition of "test_expect_fail" is: this should succeed, but needs fixin'. > > And also Cc: the guilty parties. > > Okay, I've added Johannes the author of f09c9b8c5ff to the Cc: list. > > Johannes, can you explain why this test fails? At the moment, I can't, because I indeed missed your original mail. Maybe I'll find time this afternoon to dive into gmane to find your original mail. Ciao, Dscho ``` ## Jörg Sommer, 2008-03-20 13:01 Subject: [PATCH] New test for rebase with preserve merges and squash Message-ID: <1206018070-3402-1-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206018070-3402-1-git-send-email-joerg%40alea.gnuu.de In-Reply-To: ``` Signed-off-by: Jörg Sommer --- t/t3404-rebase-interactive.sh | 52 +++++++++++++++++++++++++++++++++++++++++ 1 files changed, 52 insertions(+), 0 deletions(-) > > > > > > Please mark such tests with test_expect_fail. > > > > No, I expect the test succeeds. Currently, it fails. This might be, > > because my expectations are wrong or there's a bug. > > The definition of "test_expect_fail" is: this should succeed, but needs > fixin'. Ahh, I didn't know this. Here's a new one: diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 62e65d7..c849415 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -362,4 +362,56 @@ test_expect_success 'rebase with a file named HEAD in worktree' ' ' +test_expect_success 'squash and preserve merges' ' + test_tick && + git checkout -b squash-and-preserve-merges master && + echo A > file1 && + git commit -m SaPM-1 file1 && + echo B > file1 && + git commit -m SaPM-2 file1 && + git merge to-be-preserved && + echo C > file1 && + git commit -m SaPM-3 file1 +' + +# This test should fail, because the prompt includes the commit from the +# merge not only the merge: +# -> pick 9604163 unrelated +# pick 5ef0364 SaPM-1 +# pick 22aadcf SaPM-2 +# pick 828f7d8 Merge branch 'to-be-preserved' into squash-and-preserve-merges +# pick 2a15a54 SaPM-3 +test_expect_failure 'expect preserve merges shown not commits from merge' ' + EXPECT_COUNT=4 FAKE_LINES="1 2 squash 4 3" \ + git rebase -i -p --onto branch1 master || + { git rebase --abort; + EXPECT_COUNT=5 FAKE_LINES="1 2 3 squash 5 4" \ + git rebase -i -p --onto branch1 master; + false; } +' + +# The rebase changes nothing: SaPM-3 is still the last commit while it +# should have been merged with SaMP-2 and the branch is not rebased on +# branch1 +# +# % git forest squash-and-preserve-merges +# ╓─[squash-and-preserve-merges]──SaPM-3 +# ╟ Merge branch 'to-be-preserved' into squash-and-preserve-merges +# ╠═╗ +# ╟ ║ SaPM-2 +# ╟ ║ SaPM-1 +# ║ ╟─[to-be-preserved]──unrelated +# ╟─║─[master]──E +# ╠═╝ +# ╟ D +# ╟ C +# ╟ B +# ╙─[A]──A +test_expect_failure 'squash and preserve merges' ' + test $(git rev-parse HEAD^2) = $(git rev-parse to-be-preserved) && + test $(git rev-parse HEAD~3) = $(git rev-parse branch1) && + test $(git show HEAD:file1) = C && + test $(git show HEAD~2:file1) = A +' + test_done -- 1.5.4.4 ``` ## Johannes Schindelin, 2008-03-20 23:46 Subject: Re: [PATCH] New test for rebase with preserve merges and squash Message-ID: URL: https://gitlist.dev/e/alpine.LSU.1.00.0803210040270.4124%40racer.site In-Reply-To: <1206018070-3402-1-git-send-email-joerg@alea.gnuu.de> ``` Hi, On Thu, 20 Mar 2008, Jörg Sommer wrote: > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > index 62e65d7..c849415 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -362,4 +362,56 @@ test_expect_success 'rebase with a file named HEAD in worktree' ' > > ' > > +test_expect_success 'squash and preserve merges' ' > + test_tick && > + git checkout -b squash-and-preserve-merges master && > + echo A > file1 && > + git commit -m SaPM-1 file1 && > + echo B > file1 && > + git commit -m SaPM-2 file1 && > + git merge to-be-preserved && > + echo C > file1 && > + git commit -m SaPM-3 file1 > +' > + > +# This test should fail, because the prompt includes the commit from the > +# merge not only the merge: > +# -> pick 9604163 unrelated > +# pick 5ef0364 SaPM-1 > +# pick 22aadcf SaPM-2 > +# pick 828f7d8 Merge branch 'to-be-preserved' into squash-and-preserve-merges > +# pick 2a15a54 SaPM-3 > +test_expect_failure 'expect preserve merges shown not commits from merge' ' > + EXPECT_COUNT=4 FAKE_LINES="1 2 squash 4 3" \ > + git rebase -i -p --onto branch1 master || > + { git rebase --abort; > + EXPECT_COUNT=5 FAKE_LINES="1 2 3 squash 5 4" \ > + git rebase -i -p --onto branch1 master; > + false; } > +' I'm sorry, but I have to tell you: I do not like that style at all (for one, your expect_failure can succeed for all kind of reasons, the exit value of git rebase --abort is not even checked). And I do not understand the message "expect preserve merges shown not commits from merge". Which makes things kinda awkwardly hard for me. So I have to guess that you think that in this case: B---D-E-F-G / ,---' A---C a revision range B..G excludes C? Not so. "^B ^C G" would, but there is no way to specify this with a call to rebase. So I find it quite logical that your test case fails. Hth, Dscho ``` ## Jörg Sommer, 2008-03-21 12:56 Subject: Why rebase with preserve merges asks for merged commits Message-ID: <20080321125650.GA4587@alea.gnuu.de> URL: https://gitlist.dev/e/20080321125650.GA4587%40alea.gnuu.de In-Reply-To: ``` Hallo Johannes, Johannes Schindelin schrieb am Fri 21. Mar, 00:46 (+0100): > On Thu, 20 Mar 2008, Jörg Sommer wrote: > > > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > > index 62e65d7..c849415 100755 > > --- a/t/t3404-rebase-interactive.sh > > +++ b/t/t3404-rebase-interactive.sh > > @@ -362,4 +362,56 @@ test_expect_success 'rebase with a file named HEAD in worktree' ' > > > > ' > > > > +test_expect_success 'squash and preserve merges' ' > > + test_tick && > > + git checkout -b squash-and-preserve-merges master && > > + echo A > file1 && > > + git commit -m SaPM-1 file1 && > > + echo B > file1 && > > + git commit -m SaPM-2 file1 && > > + git merge to-be-preserved && > > + echo C > file1 && > > + git commit -m SaPM-3 file1 > > +' > > + > > +# This test should fail, because the prompt includes the commit from the > > +# merge not only the merge: > > +# -> pick 9604163 unrelated > > +# pick 5ef0364 SaPM-1 > > +# pick 22aadcf SaPM-2 > > +# pick 828f7d8 Merge branch 'to-be-preserved' into squash-and-preserve-merges > > +# pick 2a15a54 SaPM-3 > > +test_expect_failure 'expect preserve merges shown not commits from merge' ' > > + EXPECT_COUNT=4 FAKE_LINES="1 2 squash 4 3" \ > > + git rebase -i -p --onto branch1 master || > > + { git rebase --abort; > > + EXPECT_COUNT=5 FAKE_LINES="1 2 3 squash 5 4" \ > > + git rebase -i -p --onto branch1 master; > > + false; } > > +' > > I'm sorry, but I have to tell you: I do not like that style at all (for > one, your expect_failure can succeed for all kind of reasons, the exit > value of git rebase --abort is not even checked). Yes, it's ugly. So, let me step back. Think of the following situation: M----------U to-be-preserved \ \ `--A---B---+---C squash-and-preserve-merges When I do a rebase M..C with preserve merges I can decide about all commits including U which came in with the merge. U pick 9604163 unrelated A pick 5ef0364 SaPM-1 B pick 22aadcf SaPM-2 + pick 828f7d8 Merge branch 'to-be-preserved' into squash-and-preserve-merges C pick 2a15a54 SaPM-3 Why I can decide about the commit U from the branch to-be-preserved? I expect to see not the commit from the merged branches, because they aren't related to the branch to be rebased. Let's take a different situation: M--U1--U2--U3 \ \ `---A---B---+---C A rebase M..C with preserve merges asks you about the commits M..U3, too. Why? I preserve the merge that pulls in these commmits. Bye, Jörg. -- Die meisten Menschen wollen lieber durch Lob ruiniert als durch Kritik gerettet werden. ``` ## Johannes Schindelin, 2008-03-21 13:14 Subject: Re: Why rebase with preserve merges asks for merged commits Message-ID: URL: https://gitlist.dev/e/alpine.LSU.1.00.0803211410310.4124%40racer.site In-Reply-To: <20080321125650.GA4587@alea.gnuu.de> ``` Hi, On Fri, 21 Mar 2008, Jörg Sommer wrote: > Think of the following situation: > > M----------U to-be-preserved > \ \ > `--A---B---+---C squash-and-preserve-merges > > When I do a rebase M..C with preserve merges I can decide about all commits > including U which came in with the merge. > > U pick 9604163 unrelated > A pick 5ef0364 SaPM-1 > B pick 22aadcf SaPM-2 > + pick 828f7d8 Merge branch 'to-be-preserved' into squash-and-preserve-merges > C pick 2a15a54 SaPM-3 > > Why I can decide about the commit U from the branch to-be-preserved? Well, -p obviously does not take the commits (it takes a superset in your case) you would expect it to take. I am not sure if that is always the correct thing to assume, though. In the short run, you can do it manually by $ git checkout B $ git rebase --onto branch1 M $ git merge U $ git cherry-pick C Ciao, Dscho P.S.: Oh, BTW, just as in German, you have to put the verb before a subject in an English question. ```