Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:32

Martin von Zweigbergk [off-list ref] writes:

quoted hunk

Helped-by: Johannes Sixt [off-list ref]
---
 t/lib-rebase.sh                   | 15 ++++++++
 t/t3421-rebase-topology-linear.sh | 78 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100755 t/t3421-rebase-topology-linear.sh

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6ccf797..62b3887 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh

@@ -65,3 +65,18 @@ EOF
 	test_set_editor "$(pwd)/fake-editor.sh"
 	chmod a+x fake-editor.sh
 }
+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+	! { git log --format=%p "$2" | sane_grep " " ;} &&

An interesting way to spell:

    test $(git rev-list --merges "$2" | wc -l) = 0

I think I am fine with either, though.

+	expected=$1
+	set -- $(git log --reverse --format=%s "$2")
+	test "$expected" = "$*"

OK.

+}
+
+reset_rebase () {
+	git rebase --abort # may fail; ignore exit code

test_might_fail to catch unusual exit codes?

quoted hunk

+	git reset --hard &&
+	git clean -f
+}

diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
new file mode 100755
index 0000000..c4b32db
--- /dev/null
+++ b/t/t3421-rebase-topology-linear.sh

@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='basic rebase topology tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+# a---b---c
+#      \
+#       d---e
+test_expect_success 'setup' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git checkout b &&
+	test_commit d &&
+	test_commit e
+'
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "simple rebase $*" "
+		reset_rebase &&
+		git rebase $* c e &&
+		test_cmp_rev c HEAD~2 &&
+		test_linear_range 'd e' c..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* is no-op if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* b e &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" "
+		reset_rebase &&
+		git rebase $* -f b e &&

Asking to rebase the history leading to e from b on top of the merge
base (which happens to be b) may be no-op or force-create a new
history that is parallel.  OK.

+		! test_cmp_rev e HEAD &&
+		test_cmp_rev b HEAD~2 &&
+		test_linear_range 'd e' b..
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase failure -p
+
+test_run_rebase () {
+	result=$1
+	shift
+	test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "

The description is a non-sentence, and while I can tell what it
wants to say, I do not have a good suggestion for rephrasing this.

This is asking to rebase the history leading to b on top of e, but e
already includes everything in b, so it just turns into a no-op of
not moving from e.  So it is not even a fast-forward.

+		reset_rebase &&
+		git rebase $* e b &&
+		test_cmp_rev e HEAD
+	"
+}
+test_run_rebase success ''
+test_run_rebase success -m
+test_run_rebase success -i
+test_run_rebase success -p
+
+test_done

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

From: Martin von Zweigbergk <hidden>
Date: 2016-06-15 22:57:33

On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano [off-list ref] wrote:

quoted

+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+     ! { git log --format=%p "$2" | sane_grep " " ;} &&

An interesting way to spell:

    test $(git rev-list --merges "$2" | wc -l) = 0

Heh, true. I'll change that. ("My" version was based on the one in
git-rebase.sh, around line 495.)

quoted

+reset_rebase () {
+     git rebase --abort # may fail; ignore exit code

test_might_fail to catch unusual exit codes?

Will change.

quoted

+# a---b---c
+#      \
+#       d---e

quoted

+test_run_rebase () {
+     result=$1
+     shift
+     test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "

The description is a non-sentence, and while I can tell what it
wants to say, I do not have a good suggestion for rephrasing this.

Changing description to "... fast-forwards from an ancestor of upstream".

This is asking to rebase the history leading to b on top of e, but e
already includes everything in b, so it just turns into a no-op of
not moving from e.  So it is not even a fast-forward.

quoted

+             reset_rebase &&
+             git rebase $* e b &&
+             test_cmp_rev e HEAD

Well, "git rebase e b" is of course a kind of short form of "git
checkout b && git rebase e". While it's true that the implementation
doesn't bother checking out b first, that's just an optimization, but
let me know if you meant something else.

Thanks. Will wait another day or two for further comments before I
send another version.

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:57:33

Am 6/4/2013 7:14, schrieb Martin von Zweigbergk:

On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano [off-list ref] wrote:

quoted

+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+     ! { git log --format=%p "$2" | sane_grep " " ;} &&

An interesting way to spell:

    test $(git rev-list --merges "$2" | wc -l) = 0

Heh, true. I'll change that.

Then I think it would be even better written as

	revlist_merges=$(git rev-list --merges "$2") &&
	test -z "$revlist_merges"

so as not to ignore errors in the git invocation (and at least one less
fork()).

-- Hannes

Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

From: Martin von Zweigbergk <hidden>
Date: 2016-06-15 22:57:33

On Mon, Jun 3, 2013 at 11:15 PM, Johannes Sixt [off-list ref] wrote:

Am 6/4/2013 7:14, schrieb Martin von Zweigbergk:

quoted

On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano [off-list ref] wrote:

quoted

+
+# checks that the revisions in "$2" represent a linear range with the
+# subjects in "$1"
+test_linear_range () {
+     ! { git log --format=%p "$2" | sane_grep " " ;} &&

An interesting way to spell:

    test $(git rev-list --merges "$2" | wc -l) = 0

Heh, true. I'll change that.

Then I think it would be even better written as

        revlist_merges=$(git rev-list --merges "$2") &&
        test -z "$revlist_merges"

so as not to ignore errors in the git invocation (and at least one less
fork()).

Done. I'll send it out in a day or two.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help