Subject: Re: [PATCH/RFC 2/2] git-p4: Add complex test case for branch import

From: Pete Wyckoff <[email-masked]>
Date: Sat, 27 Aug 2011 19:32:01 -0400

[email-masked] wrote on Wed, 24 Aug 2011 22:23 +0100:
> On Wed, Aug 24, 2011 at 11:46 AM, Vitor Antunes <[email-masked]> wrote:
> > 1. When doing a "checkpoint" it would make sense to have some kind of
> >   feedback loop to know when fast-import completes it. In fact, I've
> >   just looked at fast-import man page and the "progress" command seems
> >   to do exactly that! Myabe we should crease a specific function for
> >   "checkpoint" that would call also "progress" instead of calling
> >   os.sleep().
> 
>     # Force a checkpoint in fast-import and wait for it to finish
>     def checkpoint(self):
>         self.gitStream.write("checkpoint\n\n")
>         self.gitStream.write("progress checkpoint\n\n")
>         out = self.gitOutput.readline()
>         if self.verbose:
>             print "checkpoint finished: " + out
> 
> This seems to work! :)

That's brilliant.  And not even hacky.  :)

> > 2. In order to avoid needing to use "--force" it would be nice to have
> >   some form of "drop" command in fast-import that would allows us to
> >   actively drop an older commit after calling "reset" (this could even
> >   be an option of "reset"). This way fast-import would not find
> >   dangling commits in the end of the import.  Maybe there's already
> >   some sort of command to achieve this in fast-import...? We could
> >   probably ask to one of its maintainers.
> 
> Just missing point 2 now.

How about this.  Create the commit onto a fake branch name,
something involving the changeid would be okay.  Remember this
so we can delete it later.  For a parent, you can use anything,
so the top of the parent branch would be okay.  Checkpoint.

Use rev-parse like you do.  But go from the most recent commit on
back, hoping that the branch was created recently.  I suspect
this is the case for those using "git-p4 sync" with
detect-branches to discover new ones periodically.

For each ref, diff-tree like you do.  When it is found, generate
the commit again, with its real branch name and parent now.

When it exits, remember to delete the temp branches.  Would be
nice to do this even on error exit, but that's harder.  Maybe
the fake branch name should be something like
"refs/git-p4-temp/change-30214" instead of refs/heads/... to
indicate they are special.  GC will commit the commits
eventually.

The other thought I had was to use "blob" with a mark name on
each file.  Use --export-marks=...  Checkpoint.  Read the blob
ids corresponding to files on the branch.  Then iterate back the
revs and do "git ls-tree -r ref^{tree}".  Compare its blobs to
the new ones until a matching commit is found.  Finally assemble
the blobs into a commit; no need to write them again.  This seems
harder.

Here's a commit that refactors the tests and adds a bunch more
branch tests.  They all pass with your checkpoint/progress fix,
and still using --force.

		-- Pete

---------------------8<---------------
From f1e11b78cbb74fd4347df5fff64a373c3e449f68 Mon Sep 17 00:00:00 2001
From: Pete Wyckoff <[email-masked]>
Date: Mon, 22 Aug 2011 22:20:33 -0400
Subject: [PATCH] git-p4 tests: refactor and add branch tests

Separate the tests related to detecting p4 branches
into their own file, and add a few more.

Introduce a library for functions that are common to
both git-p4 test files.

Signed-off-by: Pete Wyckoff <[email-masked]>
---
 t/lib-git-p4.sh          |   55 ++++++++
 t/t9800-git-p4.sh        |  182 +-------------------------
 t/t9801-git-p4-branch.sh |  326 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 388 insertions(+), 175 deletions(-)
 create mode 100644 t/lib-git-p4.sh
 create mode 100755 t/t9801-git-p4-branch.sh

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
new file mode 100644
index 0000000..dbc1499
--- /dev/null
+++ b/t/lib-git-p4.sh
@@ -0,0 +1,55 @@
+#
+# Library code for git-p4 tests
+#
+
+. ./test-lib.sh
+
+( p4 -h && p4d -h ) >/dev/null 2>&1 || {
+	skip_all='skipping git-p4 tests; no p4 or p4d'
+	test_done
+}
+
+GITP4=$GIT_BUILD_DIR/contrib/fast-import/git-p4
+P4DPORT=10669
+
+export P4PORT=localhost:$P4DPORT
+export P4CLIENT=client
+
+db="$TRASH_DIRECTORY/db"
+cli="$TRASH_DIRECTORY/cli"
+git="$TRASH_DIRECTORY/git"
+
+start_p4d()
+{
+	mkdir -p "$db" &&
+	p4d -q -d -r "$db" -p $P4DPORT &&
+	mkdir -p "$cli" &&
+	mkdir -p "$git" &&
+	cd "$cli" &&
+	p4 client -i <<-EOF
+	Client: client
+	Description: client
+	Root: $cli
+	View: //depot/... //client/...
+	EOF
+}
+
+kill_p4d()
+{
+	pid=`pgrep -f p4d` &&
+	test -n "$pid" &&
+	for i in {1..5} ; do
+	    test_debug "ps wl `echo $pid`" &&
+	    kill $pid 2>/dev/null &&
+	    pgrep -f p4d >/dev/null || break &&
+	    sleep 0.2
+	done &&
+	rm -rf "$db" &&
+	rm -rf "$cli"
+}
+
+cleanup_git() {
+	cd "$TRASH_DIRECTORY" &&
+	rm -rf "$git" &&
+	mkdir "$git"
+}
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index cf3a70e..684aa53 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -2,40 +2,16 @@
 
 test_description='git-p4 tests'
 
-. ./test-lib.sh
+. ./lib-git-p4.sh
 
-( p4 -h && p4d -h ) >/dev/null 2>&1 || {
-	skip_all='skipping git-p4 tests; no p4 or p4d'
-	test_done
-}
-
-GITP4=$GIT_BUILD_DIR/contrib/fast-import/git-p4
-P4DPORT=10669
-
-export P4PORT=localhost:$P4DPORT
-
-db="$TRASH_DIRECTORY/db"
-cli="$TRASH_DIRECTORY/cli"
-git="$TRASH_DIRECTORY/git"
-
-test_debug 'echo p4d -q -d -r "$db" -p $P4DPORT'
-test_expect_success setup '
-	mkdir -p "$db" &&
-	p4d -q -d -r "$db" -p $P4DPORT &&
-	mkdir -p "$cli" &&
-	mkdir -p "$git" &&
-	export P4PORT=localhost:$P4DPORT
+test_expect_success 'start p4d' '
+	kill_p4d || : &&
+	start_p4d &&
+	cd "$TRASH_DIRECTORY"
 '
 
 test_expect_success 'add p4 files' '
 	cd "$cli" &&
-	p4 client -i <<-EOF &&
-	Client: client
-	Description: client
-	Root: $cli
-	View: //depot/... //client/...
-	EOF
-	export P4CLIENT=client &&
 	echo file1 >file1 &&
 	p4 add file1 &&
 	p4 submit -d "file1" &&
@@ -45,12 +21,6 @@ test_expect_success 'add p4 files' '
 	cd "$TRASH_DIRECTORY"
 '
 
-cleanup_git() {
-	cd "$TRASH_DIRECTORY" &&
-	rm -rf "$git" &&
-	mkdir "$git"
-}
-
 test_expect_success 'basic git-p4 clone' '
 	"$GITP4" clone --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
@@ -404,146 +374,8 @@ test_expect_success 'detect copies' '
 	p4 filelog //depot/file13 | grep -q "branch from //depot/file"
 '
 
-# Create a simple branch structure in P4 depot to check if it is correctly
-# cloned.
-test_expect_success 'add simple p4 branches' '
-	cd "$cli" &&
-	mkdir branch1 &&
-	cd branch1 &&
-	echo file1 >file1 &&
-	echo file2 >file2 &&
-	p4 add file1 file2 &&
-	p4 submit -d "branch1" &&
-	p4 integrate //depot/branch1/... //depot/branch2/... &&
-	p4 submit -d "branch2" &&
-	echo file3 >file3 &&
-	p4 add file3 &&
-	p4 submit -d "add file3 in branch1" &&
-	p4 open file2 &&
-	echo update >>file2 &&
-	p4 submit -d "update file2 in branch1" &&
-	p4 integrate //depot/branch1/... //depot/branch3/... &&
-	p4 submit -d "branch3" &&
-	cd "$TRASH_DIRECTORY"
-'
-
-# Configure branches through git-config and clone them.
-# All files are tested to make sure branches were cloned correctly.
-# Finally, make an update to branch1 on P4 side to check if it is imported
-# correctly by git-p4.
-test_expect_success 'git-p4 clone simple branches' '
-	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 &&
-	"$GITP4" clone --dest=. --detect-branches //depot@all &&
-	git log --all --graph --decorate --stat &&
-	git reset --hard p4/depot/branch1 &&
-	test -f file1 &&
-	test -f file2 &&
-	test -f file3 &&
-	grep -q update file2 &&
-	git reset --hard p4/depot/branch2 &&
-	test -f file1 &&
-	test -f file2 &&
-	test ! -f file3 &&
-	! grep -q update file2 &&
-	git reset --hard p4/depot/branch3 &&
-	test -f file1 &&
-	test -f file2 &&
-	test -f file3 &&
-	grep -q update file2 &&
-	cd "$cli" &&
-	cd branch1 &&
-	p4 edit file2 &&
-	echo file2_ >>file2 &&
-	p4 submit -d "update file2 in branch3" &&
-	cd "$git" &&
-	git reset --hard p4/depot/branch1 &&
-	"$GITP4" rebase &&
-	grep -q file2_ file2
-'
-
-# Create a complex branch structure in P4 depot to check if they are correctly
-# cloned. The branches are created from older changelists to check if git-p4 is
-# able to correctly detect them.
-# The final expected structure is:
-# `branch1
-# | `- file1
-# | `- file2 (updated)
-# | `- file3
-# `branch2
-# | `- file1
-# | `- file2
-# `branch3
-# | `- file1
-# | `- file2 (updated)
-# | `- file3
-# `branch4
-# | `- file1
-# | `- file2
-# `branch5
-#   `- file1
-#   `- file2
-#   `- file3
-test_expect_success 'add p4 complex branches' '
-	cd "$cli" &&
-	changelist=$(p4 changes -m1 //depot/... | cut -d" " -f2) &&
-	changelist=$((changelist - 5)) &&
-	p4 integrate //depot/branch1/...@$changelist //depot/branch4/... &&
-	p4 submit -d "branch4" &&
-	changelist=$((changelist + 2)) &&
-	p4 integrate //depot/branch1/...@$changelist //depot/branch5/... &&
-	p4 submit -d "branch5" &&
-	cd "$TRASH_DIRECTORY"
-'
-
-# Configure branches through git-config and clone them. git-p4 will only be able
-# to clone the original structure if it is able to detect the origin changelist
-# of each branch.
-test_expect_success 'git-p4 clone complex branches' '
-	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 &&
-	"$GITP4" clone --dest=. --detect-branches //depot@all &&
-	git log --all --graph --decorate --stat &&
-	git reset --hard p4/depot/branch1 &&
-	test -f file1 &&
-	test -f file2 &&
-	test -f file3 &&
-	grep -q update file2 &&
-	git reset --hard p4/depot/branch2 &&
-	test -f file1 &&
-	test -f file2 &&
-	test ! -f file3 &&
-	! grep -q update file2 &&
-	git reset --hard p4/depot/branch3 &&
-	test -f file1 &&
-	test -f file2 &&
-	test -f file3 &&
-	grep -q update file2 &&
-	git reset --hard p4/depot/branch4 &&
-	test -f file1 &&
-	test -f file2 &&
-	test ! -f file3 &&
-	! grep -q update file2 &&
-	git reset --hard p4/depot/branch5 &&
-	test -f file1 &&
-	test -f file2 &&
-	test -f file3 &&
-	! grep -q update file2
-'
-
-test_expect_success 'shutdown' '
-	pid=`pgrep -f p4d` &&
-	test -n "$pid" &&
-	test_debug "ps wl `echo $pid`" &&
-	kill $pid
+test_expect_success 'kill p4d' '
+	kill_p4d
 '
 
 test_done
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
new file mode 100755
index 0000000..c71d1a5
--- /dev/null
+++ b/t/t9801-git-p4-branch.sh
@@ -0,0 +1,326 @@
+#!/bin/sh
+
+test_description='git-p4 p4 branching tests'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	kill_p4d || : &&
+	start_p4d &&
+	cd "$TRASH_DIRECTORY"
+'
+
+#
+# 1: //depot/main/f1
+# 2: //depot/main/f2
+# 3: integrate //depot/main/... -> //depot/branch1/...
+# 4: //depot/main/f4
+# 5: //depot/branch1/f5
+# .: named branch branch2
+# 6: integrate -b branch2
+# 7: //depot/branch2/f7
+# 8: //depot/main/f8
+#
+test_expect_success 'basic p4 branches' '
+	cd "$cli" &&
+	mkdir -p main &&
+
+	echo f1 >main/f1 &&
+	p4 add main/f1 &&
+	p4 submit -d "main/f1" &&
+
+	echo f2 >main/f2 &&
+	p4 add main/f2 &&
+	p4 submit -d "main/f2" &&
+
+	p4 integrate //depot/main/... //depot/branch1/... &&
+	p4 submit -d "integrate main to branch1" &&
+
+	echo f4 >main/f4 &&
+	p4 add main/f4 &&
+	p4 submit -d "main/f4" &&
+
+	echo f5 >branch1/f5 &&
+	p4 add branch1/f5 &&
+	p4 submit -d "branch1/f5" &&
+
+	p4 branch -i <<-EOF &&
+	Branch: branch2
+	View: //depot/main/... //depot/branch2/...
+	EOF
+
+	p4 integrate -b branch2 &&
+	p4 submit -d "integrate main to branch2" &&
+
+	echo f7 >branch2/f7 &&
+	p4 add branch2/f7 &&
+	p4 submit -d "branch2/f7" &&
+
+	echo f8 >main/f8 &&
+	p4 add main/f8 &&
+	p4 submit -d "main/f8" &&
+
+	cd "$TRASH_DIRECTORY"
+'
+
+test_expect_success 'import main, no branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot/main@all &&
+	cd "$git" &&
+	git log --oneline --graph --decorate --all &&
+	git rev-list master >wc &&
+	test_line_count = 4 wc
+'
+
+test_expect_success 'import branch1, no branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot/branch1@all &&
+	cd "$git" &&
+	git log --oneline --graph --decorate --all &&
+	git rev-list master >wc &&
+	test_line_count = 2 wc
+'
+
+test_expect_success 'import branch2, no branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot/branch2@all &&
+	cd "$git" &&
+	git log --oneline --graph --decorate --all &&
+	git rev-list master >wc &&
+	test_line_count = 2 wc
+'
+
+test_expect_success 'import depot, no branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot@all &&
+	cd "$git" &&
+	git log --oneline --graph --decorate --all &&
+	git rev-list master >wc &&
+	test_line_count = 8 wc
+'
+
+test_expect_success 'import depot, branch detection' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" --detect-branches //depot@all &&
+	cd "$git" &&
+
+	git log --oneline --graph --decorate --all &&
+
+	# 4 main commits
+	git rev-list master >wc &&
+	test_line_count = 4 wc &&
+
+	# 3 main, 1 integrate, 1 on branch2
+	git rev-list p4/depot/branch2 >wc &&
+	test_line_count = 5 wc &&
+
+	# no branch1, since no p4 branch created for it
+	test_must_fail git show-ref p4/depot/branch1
+'
+
+test_expect_success 'import depot, branch detection, branchList branch definition' '
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	cd "$git" &&
+	git config git-p4.branchList main:branch1 &&
+	"$GITP4" clone --dest=. --detect-branches //depot@all &&
+
+	git log --oneline --graph --decorate --all &&
+
+	# 4 main commits
+	git rev-list master >wc &&
+	test_line_count = 4 wc &&
+
+	# 3 main, 1 integrate, 1 on branch2
+	git rev-list p4/depot/branch2 >wc &&
+	test_line_count = 5 wc &&
+
+	# 2 main, 1 integrate, 1 on branch1
+	git rev-list p4/depot/branch1 >wc &&
+	test_line_count = 4 wc
+'
+
+test_expect_success 'branch from non-head should find parent' '
+	cd "$cli" &&
+	p4 integrate //depot/main/...@1 //depot/branch3/... &&
+	p4 submit -d "integrate main@1 to branch3" &&
+
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	cd "$git" &&
+
+	git config git-p4.branchList main:branch1 &&
+	git config git-p4.branchList main:branch3 &&
+	"$GITP4" clone --dest=. --detect-branches //depot@all &&
+
+	git log --oneline --graph --decorate --all &&
+
+	branch3_parent=$(git rev-parse p4/depot/branch3^) &&
+	change1=$(git rev-parse :/"change = 1") &&
+	echo test $branch3_parent = $change1 &&
+	test $branch3_parent = $change1
+'
+
+test_expect_success 'restart p4d' '
+	kill_p4d &&
+	start_p4d &&
+	cd "$TRASH_DIRECTORY"
+'
+
+#
+# 1: //depot/branch1/file1
+#    //depot/branch1/file2
+# 2: integrate //depot/branch1/... -> //depot/branch2/...
+# 3: //depot/branch1/file3
+# 4: //depot/branch1/file2 (edit)
+# 5: integrate //depot/branch1/... -> //depot/branch3/...
+#
+## Create a simple branch structure in P4 depot.
+test_expect_success 'add simple p4 branches' '
+	cd "$cli" &&
+	mkdir branch1 &&
+	cd branch1 &&
+	echo file1 >file1 &&
+	echo file2 >file2 &&
+	p4 add file1 file2 &&
+	p4 submit -d "branch1" &&
+	p4 integrate //depot/branch1/... //depot/branch2/... &&
+	p4 submit -d "branch2" &&
+	echo file3 >file3 &&
+	p4 add file3 &&
+	p4 submit -d "add file3 in branch1" &&
+	p4 open file2 &&
+	echo update >>file2 &&
+	p4 submit -d "update file2 in branch1" &&
+	p4 integrate //depot/branch1/... //depot/branch3/... &&
+	p4 submit -d "branch3" &&
+	cd "$TRASH_DIRECTORY"
+'
+
+# Configure branches through git-config and clone them.
+# All files are tested to make sure branches were cloned correctly.
+# Finally, make an update to branch1 on P4 side to check if it is imported
+# correctly by git-p4.
+test_expect_success 'git-p4 clone simple branches' '
+	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 &&
+	"$GITP4" clone --dest=. --detect-branches //depot@all &&
+	git log --all --graph --decorate --stat &&
+	git reset --hard p4/depot/branch1 &&
+	test -f file1 &&
+	test -f file2 &&
+	test -f file3 &&
+	grep -q update file2 &&
+	git reset --hard p4/depot/branch2 &&
+	test -f file1 &&
+	test -f file2 &&
+	test ! -f file3 &&
+	test_must_fail grep -q update file2 &&
+	git reset --hard p4/depot/branch3 &&
+	test -f file1 &&
+	test -f file2 &&
+	test -f file3 &&
+	grep -q update file2 &&
+	cd "$cli" &&
+	cd branch1 &&
+	p4 edit file2 &&
+	echo file2_ >>file2 &&
+	p4 submit -d "update file2 in branch3" &&
+	cd "$git" &&
+	git reset --hard p4/depot/branch1 &&
+	"$GITP4" rebase &&
+	grep -q file2_ file2
+'
+
+# Create a complex branch structure in P4 depot to check if they are correctly
+# cloned. The branches are created from older changelists to check if git-p4 is
+# able to correctly detect them.
+#
+# The final expected structure is:
+# `branch1
+# | `- file1
+# | `- file2 (updated)
+# | `- file3
+# `branch2
+# | `- file1
+# | `- file2
+# `branch3
+# | `- file1
+# | `- file2 (updated)
+# | `- file3
+# `branch4
+# | `- file1
+# | `- file2
+# `branch5
+#   `- file1
+#   `- file2
+#   `- file3
+#
+# Changes 1..6 from previous tests, plus two new ones:
+# 1: //depot/branch1/file1
+#    //depot/branch1/file2
+# 2: integrate //depot/branch1/... -> //depot/branch2/...
+# 3: //depot/branch1/file3
+# 4: //depot/branch1/file2 (edit)
+# 5: integrate //depot/branch1/... -> //depot/branch3/...
+# 6: //depot/branch3/file2 (edit, from previous test)
+# 7: integrate //depot/branch1/...@1 -> //depot/branch4/...
+# 8: integrate //depot/branch1/...@3 -> //depot/branch5/...
+#
+test_expect_success 'add p4 complex branches' '
+	cd "$cli" &&
+	p4 integrate //depot/branch1/...@1 //depot/branch4/... &&
+	p4 submit -d "branch4 from branch1@1" &&
+	p4 integrate //depot/branch1/...@3 //depot/branch5/... &&
+	p4 submit -d "branch5 from branch1@3" &&
+	cd "$TRASH_DIRECTORY"
+'
+
+# Configure branches through git-config and clone them.  git-p4 will only be
+# able to clone the original structure if it is able to detect the origin
+# changelist of each branch.
+test_expect_success 'git-p4 clone complex branches' '
+	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 &&
+	"$GITP4" clone --dest=. --detect-branches //depot@all &&
+	git log --all --graph --decorate --stat &&
+	git reset --hard p4/depot/branch1 &&
+	test -f file1 &&
+	test -f file2 &&
+	test -f file3 &&
+	grep -q update file2 &&
+	git reset --hard p4/depot/branch2 &&
+	test -f file1 &&
+	test -f file2 &&
+	test ! -f file3 &&
+	test_must_fail grep -q update file2 &&
+	git reset --hard p4/depot/branch3 &&
+	test -f file1 &&
+	test -f file2 &&
+	test -f file3 &&
+	grep -q update file2 &&
+	git reset --hard p4/depot/branch4 &&
+	test -f file1 &&
+	test -f file2 &&
+	test ! -f file3 &&
+	test_must_fail grep -q update file2 &&
+	git reset --hard p4/depot/branch5 &&
+	test -f file1 &&
+	test -f file2 &&
+	test -f file3 &&
+	test_must_fail grep -q update file2
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done
-- 
1.7.5.4