# [PATCH 0/2] git-p4: New test cases for branch detection 8 messages from 2012-05-22 to 2012-05-25. Participants: Vitor Antunes, Pete Wyckoff, Junio C Hamano. Thread: https://gitlist.dev/t/30596 ## Vitor Antunes, 2012-05-22 23:38 Subject: [PATCH 0/2] git-p4: New test cases for branch detection Message-ID: <1337729891-27648-1-git-send-email-vitor.hda@gmail.com> URL: https://gitlist.dev/e/1337729891-27648-1-git-send-email-vitor.hda%40gmail.com ``` Two new test cases to verify branch detection functionality. The first is working properly and I think it is a good idea to make sure this is not broken in the future. The second test case shows a limitation in the current algorithm for new branch detection. Vitor Antunes (2): git-p4: Test changelists touching two branches git-p4: Verify detection of "empty" branch creation t/t9801-git-p4-branch.sh | 110 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 110 insertions(+), 0 deletions(-) -- 1.7.7.rc2.14.g5e044.dirty ``` ## Vitor Antunes, 2012-05-22 23:38 Subject: [PATCH 1/2] git-p4: Test changelists touching two branches Message-ID: <1337729891-27648-2-git-send-email-vitor.hda@gmail.com> URL: https://gitlist.dev/e/1337729891-27648-2-git-send-email-vitor.hda%40gmail.com In-Reply-To: <1337729891-27648-1-git-send-email-vitor.hda@gmail.com> ``` It is possible to modify two different branches in P4 in a single changelist. git-p4 correctly detects this and commits the relevant changes to the different branches separately. This test proves that and avoid future regressions in this behavior. Signed-off-by: Vitor Antunes --- t/t9801-git-p4-branch.sh | 54 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 54 insertions(+), 0 deletions(-) diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh index 2859256..89d8c59 100755 --- a/t/t9801-git-p4-branch.sh +++ b/t/t9801-git-p4-branch.sh @@ -306,6 +306,60 @@ test_expect_success 'git p4 clone complex branches' ' ) ' +# Move branch3/file3 to branch4/file3 in a single changelist +test_expect_success 'git p4 submit to two branches in a single changelist' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$cli" && + p4 integrate //depot/branch3/file3 //depot/branch4/file3 && + p4 delete //depot/branch3/file3 && + p4 submit -d "Move branch3/file3 to branch4/file3" + ) +' + +# Confirm that changes to two branches done in a single changelist +# are correctly imported by git p4 +test_expect_success 'git p4 sync changes to two branches in the same changelist' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList branch1:branch2 && + git config --add git-p4.branchList branch1:branch3 && + git config --add git-p4.branchList branch1:branch4 && + git config --add git-p4.branchList branch1:branch5 && + git p4 clone --dest=. --detect-branches //depot@all && + git log --all --graph --decorate --stat && + git reset --hard p4/depot/branch1 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_file file3 && + grep update file2 && + git reset --hard p4/depot/branch2 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_missing file3 && + ! grep update file2 && + git reset --hard p4/depot/branch3 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_missing file3 && + grep update file2 && + git reset --hard p4/depot/branch4 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_file file3 && + ! grep update file2 && + git reset --hard p4/depot/branch5 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_file file3 && + ! grep update file2 && + test_path_is_missing .git/git-p4-tmp + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' -- 1.7.7.rc2.14.g5e044.dirty ``` ## Vitor Antunes, 2012-05-22 23:38 Subject: [PATCH 2/2] git-p4: Verify detection of "empty" branch creation Message-ID: <1337729891-27648-3-git-send-email-vitor.hda@gmail.com> URL: https://gitlist.dev/e/1337729891-27648-3-git-send-email-vitor.hda%40gmail.com In-Reply-To: <1337729891-27648-1-git-send-email-vitor.hda@gmail.com> ``` Current implementation of new branch parent detection works on the principle that the new branch is a complete integration, with no changes, of the original files. This test shows this deficiency in the particular case when the new branch is created from a subset of the original files. Signed-off-by: Vitor Antunes --- t/t9801-git-p4-branch.sh | 56 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 56 insertions(+), 0 deletions(-) diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh index 89d8c59..c8e4d86 100755 --- a/t/t9801-git-p4-branch.sh +++ b/t/t9801-git-p4-branch.sh @@ -360,6 +360,62 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist' ) ' +# Create a branch by integrating a single file +test_expect_success 'git p4 file subset branch' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$cli" && + p4 integrate //depot/branch1/file1 //depot/branch6/file1 && + p4 submit -d "Integrate file1 alone from branch1 to branch6" + ) +' + +# Check if git -p4 creates a new branch containing a single file, +# instead of keeping the old files from the original branch +test_expect_failure 'git p4 clone file subset branch' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList branch1:branch2 && + git config --add git-p4.branchList branch1:branch3 && + git config --add git-p4.branchList branch1:branch4 && + git config --add git-p4.branchList branch1:branch5 && + git config --add git-p4.branchList branch1:branch6 && + "$GITP4" clone --dest=. --detect-branches //depot@all && + git log --all --graph --decorate --stat && + git reset --hard p4/depot/branch1 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_file file3 && + grep update file2 && + git reset --hard p4/depot/branch2 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_missing file3 && + ! grep update file2 && + git reset --hard p4/depot/branch3 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_missing file3 && + grep update file2 && + git reset --hard p4/depot/branch4 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_file file3 && + ! grep update file2 && + git reset --hard p4/depot/branch5 && + test_path_is_file file1 && + test_path_is_file file2 && + test_path_is_file file3 && + ! grep update file2 && + git reset --hard p4/depot/branch6 && + test_path_is_file file1 && + ! test_path_is_file file2 && + ! test_path_is_file file3 + ) +' test_expect_success 'kill p4d' ' kill_p4d ' -- 1.7.7.rc2.14.g5e044.dirty ``` ## Pete Wyckoff, 2012-05-24 01:08 Subject: Re: [PATCH 1/2] git-p4: Test changelists touching two branches Message-ID: <20120524010805.GA26443@padd.com> URL: https://gitlist.dev/e/20120524010805.GA26443%40padd.com In-Reply-To: <1337729891-27648-2-git-send-email-vitor.hda@gmail.com> ``` vitor.hda@gmail.com wrote on Wed, 23 May 2012 00:38 +0100: > It is possible to modify two different branches in P4 in a single > changelist. git-p4 correctly detects this and commits the relevant > changes to the different branches separately. This test proves that and > avoid future regressions in this behavior. > > Signed-off-by: Vitor Antunes > --- > t/t9801-git-p4-branch.sh | 54 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 54 insertions(+), 0 deletions(-) > > diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh > index 2859256..89d8c59 100755 > --- a/t/t9801-git-p4-branch.sh > +++ b/t/t9801-git-p4-branch.sh > @@ -306,6 +306,60 @@ test_expect_success 'git p4 clone complex branches' ' > ) > ' > > +# Move branch3/file3 to branch4/file3 in a single changelist > +test_expect_success 'git p4 submit to two branches in a single changelist' ' > + test_when_finished cleanup_git && > + test_create_repo "$git" && You can skip the git bits here. This is just setup for the next test. Ack everything else. Nice to have the extra tests. -- Pete > + ( > + cd "$cli" && > + p4 integrate //depot/branch3/file3 //depot/branch4/file3 && > + p4 delete //depot/branch3/file3 && > + p4 submit -d "Move branch3/file3 to branch4/file3" > + ) > +' > + > +# Confirm that changes to two branches done in a single changelist > +# are correctly imported by git p4 > +test_expect_success 'git p4 sync changes to two branches in the same changelist' ' > + test_when_finished cleanup_git && > + test_create_repo "$git" && > + ( > + cd "$git" && > + git config git-p4.branchList branch1:branch2 && > + git config --add git-p4.branchList branch1:branch3 && > + git config --add git-p4.branchList branch1:branch4 && > + git config --add git-p4.branchList branch1:branch5 && > + git p4 clone --dest=. --detect-branches //depot@all && > + git log --all --graph --decorate --stat && > + git reset --hard p4/depot/branch1 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_file file3 && > + grep update file2 && > + git reset --hard p4/depot/branch2 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_missing file3 && > + ! grep update file2 && > + git reset --hard p4/depot/branch3 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_missing file3 && > + grep update file2 && > + git reset --hard p4/depot/branch4 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_file file3 && > + ! grep update file2 && > + git reset --hard p4/depot/branch5 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_file file3 && > + ! grep update file2 && > + test_path_is_missing .git/git-p4-tmp > + ) > +' > + > test_expect_success 'kill p4d' ' > kill_p4d > ' > -- > 1.7.7.rc2.14.g5e044.dirty > ``` ## Pete Wyckoff, 2012-05-24 01:13 Subject: Re: [PATCH 2/2] git-p4: Verify detection of "empty" branch creation Message-ID: <20120524011338.GB26443@padd.com> URL: https://gitlist.dev/e/20120524011338.GB26443%40padd.com In-Reply-To: <1337729891-27648-3-git-send-email-vitor.hda@gmail.com> ``` vitor.hda@gmail.com wrote on Wed, 23 May 2012 00:38 +0100: > Current implementation of new branch parent detection works on the > principle that the new branch is a complete integration, with no > changes, of the original files. > This test shows this deficiency in the particular case when the new > branch is created from a subset of the original files. > > Signed-off-by: Vitor Antunes > --- > t/t9801-git-p4-branch.sh | 56 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 56 insertions(+), 0 deletions(-) > > diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh > index 89d8c59..c8e4d86 100755 > --- a/t/t9801-git-p4-branch.sh > +++ b/t/t9801-git-p4-branch.sh > @@ -360,6 +360,62 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist' > ) > ' > > +# Create a branch by integrating a single file > +test_expect_success 'git p4 file subset branch' ' > + test_when_finished cleanup_git && > + test_create_repo "$git" && Ditto; no need for git here. > + ( > + cd "$cli" && > + p4 integrate //depot/branch1/file1 //depot/branch6/file1 && > + p4 submit -d "Integrate file1 alone from branch1 to branch6" > + ) > +' > + > +# Check if git -p4 creates a new branch containing a single file, Stray "-" -------^ > +# instead of keeping the old files from the original branch > +test_expect_failure 'git p4 clone file subset branch' ' > + test_when_finished cleanup_git && > + test_create_repo "$git" && > + ( > + cd "$git" && > + git config git-p4.branchList branch1:branch2 && > + git config --add git-p4.branchList branch1:branch3 && > + git config --add git-p4.branchList branch1:branch4 && > + git config --add git-p4.branchList branch1:branch5 && > + git config --add git-p4.branchList branch1:branch6 && > + "$GITP4" clone --dest=. --detect-branches //depot@all && Rebase error? It fails because this should be "git p4". > + git log --all --graph --decorate --stat && > + git reset --hard p4/depot/branch1 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_file file3 && > + grep update file2 && > + git reset --hard p4/depot/branch2 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_missing file3 && > + ! grep update file2 && > + git reset --hard p4/depot/branch3 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_missing file3 && > + grep update file2 && > + git reset --hard p4/depot/branch4 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_file file3 && > + ! grep update file2 && > + git reset --hard p4/depot/branch5 && > + test_path_is_file file1 && > + test_path_is_file file2 && > + test_path_is_file file3 && > + ! grep update file2 && > + git reset --hard p4/depot/branch6 && > + test_path_is_file file1 && > + ! test_path_is_file file2 && But then it fails here too because the code is indeed buggy. Nice to have this test too, to document the problem and maybe prod someone to fix it someday. > + ! test_path_is_file file3 Do use "test_path_is_missing" for these non-existant file tests, though, please. -- Pete > + ) > +' > test_expect_success 'kill p4d' ' > kill_p4d > ' > -- > 1.7.7.rc2.14.g5e044.dirty > ``` ## Vitor Antunes, 2012-05-24 22:42 Subject: Re: [PATCH 1/2] git-p4: Test changelists touching two branches Message-ID: URL: https://gitlist.dev/e/CAOpHH-U%2BcG2DvTtWD25JNgLYKrNcwXgqysA0G-wr5DFhLG-DAg%40mail.gmail.com In-Reply-To: <20120524010805.GA26443@padd.com> ``` On Thu, May 24, 2012 at 2:08 AM, Pete Wyckoff wrote: > vitor.hda@gmail.com wrote on Wed, 23 May 2012 00:38 +0100: >> It is possible to modify two different branches in P4 in a single >> changelist. git-p4 correctly detects this and commits the relevant >> changes to the different branches separately. This test proves that and >> avoid future regressions in this behavior. >> >> Signed-off-by: Vitor Antunes >> --- >>  t/t9801-git-p4-branch.sh |   54 ++++++++++++++++++++++++++++++++++++++++++++++ >>  1 files changed, 54 insertions(+), 0 deletions(-) >> >> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh >> index 2859256..89d8c59 100755 >> --- a/t/t9801-git-p4-branch.sh >> +++ b/t/t9801-git-p4-branch.sh >> @@ -306,6 +306,60 @@ test_expect_success 'git p4 clone complex branches' ' >>       ) >>  ' >> >> +# Move branch3/file3 to branch4/file3 in a single changelist >> +test_expect_success 'git p4 submit to two branches in a single changelist' ' >> +     test_when_finished cleanup_git && >> +     test_create_repo "$git" && > > You can skip the git bits here.  This is just setup for the next > test. What about the parentheses in the next line, is it necessary? Or can I simply remove that pair of parentheses and unindent the code in between? Thanks, Vitor > Ack everything else.  Nice to have the extra tests. > >                -- Pete > >> +     ( >> +             cd "$cli" && >> +             p4 integrate //depot/branch3/file3 //depot/branch4/file3 && >> +             p4 delete //depot/branch3/file3 && >> +             p4 submit -d "Move branch3/file3 to branch4/file3" >> +     ) >> +' >> + >> +# Confirm that changes to two branches done in a single changelist >> +# are correctly imported by git p4 >> +test_expect_success 'git p4 sync changes to two branches in the same changelist' ' >> +     test_when_finished cleanup_git && >> +     test_create_repo "$git" && >> +     ( >> +             cd "$git" && >> +             git config git-p4.branchList branch1:branch2 && >> +             git config --add git-p4.branchList branch1:branch3 && >> +             git config --add git-p4.branchList branch1:branch4 && >> +             git config --add git-p4.branchList branch1:branch5 && >> +             git p4 clone --dest=. --detect-branches //depot@all && >> +             git log --all --graph --decorate --stat && >> +             git reset --hard p4/depot/branch1 && >> +             test_path_is_file file1 && >> +             test_path_is_file file2 && >> +             test_path_is_file file3 && >> +             grep update file2 && >> +             git reset --hard p4/depot/branch2 && >> +             test_path_is_file file1 && >> +             test_path_is_file file2 && >> +             test_path_is_missing file3 && >> +             ! grep update file2 && >> +             git reset --hard p4/depot/branch3 && >> +             test_path_is_file file1 && >> +             test_path_is_file file2 && >> +             test_path_is_missing file3 && >> +             grep update file2 && >> +             git reset --hard p4/depot/branch4 && >> +             test_path_is_file file1 && >> +             test_path_is_file file2 && >> +             test_path_is_file file3 && >> +             ! grep update file2 && >> +             git reset --hard p4/depot/branch5 && >> +             test_path_is_file file1 && >> +             test_path_is_file file2 && >> +             test_path_is_file file3 && >> +             ! grep update file2 && >> +             test_path_is_missing .git/git-p4-tmp >> +     ) >> +' >> + >>  test_expect_success 'kill p4d' ' >>       kill_p4d >>  ' >> -- >> 1.7.7.rc2.14.g5e044.dirty >> ``` ## Junio C Hamano, 2012-05-25 00:02 Subject: Re: [PATCH 1/2] git-p4: Test changelists touching two branches Message-ID: <7vmx4xunzr.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vmx4xunzr.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Vitor Antunes writes: > What about the parentheses in the next line, is it necessary? Or can I > simply remove that pair of parentheses and unindent the code in > between? > >> ... >>> + ( >>> + cd "$cli" && >>> + p4 integrate //depot/branch3/file3 //depot/branch4/file3 && >>> + p4 delete //depot/branch3/file3 && >>> + p4 submit -d "Move branch3/file3 to branch4/file3" >>> + ) >>> +' If you mean this part, the parentheses to throw you into a subprocess are required. Otherwise, a failure in any of these three p4 commands will leave you in $cli directory, causing the next test to start in a directory that it does not expect. ``` ## Vitor Antunes, 2012-05-25 08:32 Subject: Re: [PATCH 1/2] git-p4: Test changelists touching two branches Message-ID: URL: https://gitlist.dev/e/CAOpHH-U5dRaMsMPn3X6KujSb%3Dpnw%2BvoaOLaEntGhi%2B4x1uboGA%40mail.gmail.com In-Reply-To: <7vmx4xunzr.fsf@alter.siamese.dyndns.org> ``` On Fri, May 25, 2012 at 1:02 AM, Junio C Hamano wrote: > Vitor Antunes writes: > >> What about the parentheses in the next line, is it necessary? Or can I >> simply remove that pair of parentheses and unindent the code in >> between? >> >>> ... >>>> +     ( >>>> +             cd "$cli" && >>>> +             p4 integrate //depot/branch3/file3 //depot/branch4/file3 && >>>> +             p4 delete //depot/branch3/file3 && >>>> +             p4 submit -d "Move branch3/file3 to branch4/file3" >>>> +     ) >>>> +' > > If you mean this part, the parentheses to throw you into a subprocess are > required.  Otherwise, a failure in any of these three p4 commands will > leave you in $cli directory, causing the next test to start in a directory > that it does not expect. That makes sense and is quite obvious now that I _see_ it. Thanks for your help Junio. Vitor ```