tests: allow to skip git rebase -p tests

PATCH In review Johannes Schindelin via GitGitGadget 3 participants 7 messages Oct 31, 2018 – Nov 1, 2018

Share & tools

Johannes Schindelin via GitGitGadget

author

2018-10-31 20:02 UTC #0 lore raw

cover tests: allow to skip git rebase -p tests

The --preserve-merges mode of the git rebase command is on its way out, being superseded by the --rebase-merges mode. My plan is to contribute patches to deprecate the former in favor of the latter before long.

In the meantime, it seems a bit pointless to keep running the git rebase -p tests, in particular in the Windows phase of the automated testing. In preparation for skipping those tests, this patch series starts out by decoupling test cases so that no non-rebase -p ones depend on side effects of rebase -p ones, and it concludes by a patch that allows skipping the rebase -p ones by setting the environment variable GIT_TEST_SKIP_REBASE_P.

In a quick 'n dirty test, skipping the rebase -p tests seems to shave off about 8 minutes from the 1h20 running time of the test suite on Windows (without git svn tests, we skip them for a long time already, as they are really, really slow on Windows).

Johannes Schindelin (3):
t3404: decouple some test cases from outcomes of previous test cases
t3418: decouple test cases from a previous rebase -p test case
tests: optionally skip git rebase -p tests

 t/t3404-rebase-interactive.sh             | 23 ++++++++-------
 t/t3408-rebase-multi-line.sh              |  2 +-
 t/t3409-rebase-preserve-merges.sh         |  5 ++++
 t/t3410-rebase-preserve-dropped-merges.sh |  5 ++++
 t/t3411-rebase-preserve-around-merges.sh  |  5 ++++
 t/t3412-rebase-root.sh                    | 12 ++++----
 t/t3414-rebase-preserve-onto.sh           |  5 ++++
 t/t3418-rebase-continue.sh                |  5 ++--
 t/t3421-rebase-topology-linear.sh         | 36 +++++++++++------------
 t/t3425-rebase-topology-merges.sh         |  5 ++++
 t/t5520-pull.sh                           |  6 ++--
 t/t7505-prepare-commit-msg-hook.sh        |  2 +-
 t/t7517-per-repo-email.sh                 |  6 ++--
 t/test-lib.sh                             |  4 +++
 14 files changed, 78 insertions(+), 43 deletions(-)

base-commit: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-63%2Fdscho%2Fsplit-out-rebase-p-tests-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-63/dscho/split-out-rebase-p-tests-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/63

Signature · 1 line

gitgitgadget

Johannes Schindelin via GitGitGadget

author

2018-10-31 20:02 UTC #1 lore raw

PATCH 1/3 t3404: decouple some test cases from outcomes of previous test cases

From: Johannes Schindelin <[email protected]>

Originally, the --preserve-merges option of the git rebase command piggy-backed on top of the --interactive feature. For that reason, the early test cases were added to the very same test script that contains the git rebase -i tests: t3404-rebase-interactive.sh.

However, since c42abfe7857 (rebase: introduce a dedicated backend for --preserve-merges, 2018-05-28), the --preserve-merges feature got its own backend, in preparation for converting the rest of the --interactive code to built-in code, written in C rather than shell.

The reason why the --preserve-merges feature was not converted at the same time is that we have something much better now: --rebase-merges. That option intends to supersede --preserve-merges, and we will probably deprecate the latter soon.

Once --preserve-merges has been deprecated for a good amount of time, it will be time to remove it, and along with it, its tests.

In preparation for that, let's make the rest of the test cases in t3404-rebase-interactive.sh independent of the test cases dedicated to --preserve-merges.

Signed-off-by Johannes Schindelin <[email protected]>

---
 t/t3404-rebase-interactive.sh | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

t/t3404-rebase-interactive.sh +8 -7

index ff89b6341a..99d1fb79a8 100755
@@ -387,6 +387,7 @@ test_expect_success 'edit ancestor with -p' '
 '
 
 test_expect_success '--continue tries to commit' '
+	git reset --hard D &&
 	test_tick &&
 	set_fake_editor &&
 	test_must_fail git rebase -i --onto new-branch1 HEAD^ &&

@@ -426,7 +427,7 @@ test_expect_success C_LOCALE_OUTPUT 'multi-fixup does not fire up editor' '
 		git rebase -i $base &&
 	test $base = $(git rev-parse HEAD^) &&
 	test 0 = $(git show | grep NEVER | wc -l) &&
-	git checkout to-be-rebased &&
+	git checkout @{-1} &&
 	git branch -D multi-fixup
 '

@@ -441,7 +442,7 @@ test_expect_success 'commit message used after conflict' '
 		git rebase --continue &&
 	test $base = $(git rev-parse HEAD^) &&
 	test 1 = $(git show | grep ONCE | wc -l) &&
-	git checkout to-be-rebased &&
+	git checkout @{-1} &&
 	git branch -D conflict-fixup
 '

@@ -456,7 +457,7 @@ test_expect_success 'commit message retained after conflict' '
 		git rebase --continue &&
 	test $base = $(git rev-parse HEAD^) &&
 	test 2 = $(git show | grep TWICE | wc -l) &&
-	git checkout to-be-rebased &&
+	git checkout @{-1} &&
 	git branch -D conflict-squash
 '

@@ -481,7 +482,7 @@ test_expect_success C_LOCALE_OUTPUT 'squash and fixup generate correct log messa
 		grep "^# This is a combination of 3 commits\."  &&
 	git cat-file commit HEAD@{3} |
 		grep "^# This is a combination of 2 commits\."  &&
-	git checkout to-be-rebased &&
+	git checkout @{-1} &&
 	git branch -D squash-fixup
 '

@@ -494,7 +495,7 @@ test_expect_success C_LOCALE_OUTPUT 'squash ignores comments' '
 		git rebase -i $base &&
 	test $base = $(git rev-parse HEAD^) &&
 	test 1 = $(git show | grep ONCE | wc -l) &&
-	git checkout to-be-rebased &&
+	git checkout @{-1} &&
 	git branch -D skip-comments
 '

@@ -507,7 +508,7 @@ test_expect_success C_LOCALE_OUTPUT 'squash ignores blank lines' '
 		git rebase -i $base &&
 	test $base = $(git rev-parse HEAD^) &&
 	test 1 = $(git show | grep ONCE | wc -l) &&
-	git checkout to-be-rebased &&
+	git checkout @{-1} &&
 	git branch -D skip-blank-lines
 '

@@ -648,7 +649,7 @@ test_expect_success 'rebase with a file named HEAD in worktree' '
 	) &&
 
 	set_fake_editor &&
-	FAKE_LINES="1 squash 2" git rebase -i to-be-rebased &&
+	FAKE_LINES="1 squash 2" git rebase -i @{-1} &&
 	test "$(git show -s --pretty=format:%an)" = "Squashed Away"
 
 '

Signature · 1 line

gitgitgadget

Johannes Schindelin via GitGitGadget

author

2018-10-31 20:02 UTC #2 lore raw

PATCH 2/3 t3418: decouple test cases from a previous `rebase -p` test case

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

It is in general a good idea for regression test cases to be as independent of each other as possible (with the one exception of an initial setup test case, which is only a test case in Git's test suite because it does not have a notion of a fixture or setup).

This patch addresses one particular instance of this principle being
violated: a few test cases in t3418-rebase-continue.sh depend on a side effect of a test case that verifies a specific rebase -p behavior. The later test cases should, however, still succeed even if the rebase -p test case is skipped.

Signed-off-by Johannes Schindelin <[email protected]>

---
 t/t3418-rebase-continue.sh | 1 +
 1 file changed, 1 insertion(+)

t/t3418-rebase-continue.sh +1 -0

index 25099d715c..031e3d8ddb 100755
@@ -177,6 +177,7 @@ test_expect_success 'setup rerere database' '
 	git checkout master &&
 	test_commit "commit-new-file-F3" F3 3 &&
 	test_config rerere.enabled true &&
+	git update-ref refs/heads/topic commit-new-file-F3-on-topic-branch &&
 	test_must_fail git rebase -m master topic &&
 	echo "Resolved" >F2 &&
 	cp F2 expected-F2 &&

Signature · 1 line

gitgitgadget

Johannes Schindelin via GitGitGadget

author

2018-10-31 20:02 UTC #3 lore raw

PATCH 3/3 tests: optionally skip `git rebase -p` tests

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

The --preserve-merges mode of the rebase command is slated to be deprecated soon, as the more powerful --rebase-merges mode is available now, and the latter was designed with the express intent to address the shortcomings of --preserve-merges' design (e.g. the inability to reorder commits in an interactive rebase).

As such, we will eventually even remove the --preserve-merges support, and along with it, its tests.

In preparation for this, and also to allow the Windows phase of our automated tests to save some well-needed time when running the test suite, this commit introduces a new prerequisite REBASE_P, which can be forced to being unmet by setting the environment variable GIT_TEST_SKIP_REBASE_P to any non-empty string.

Signed-off-by Johannes Schindelin <[email protected]>

---
 t/t3404-rebase-interactive.sh             |  8 ++---
 t/t3408-rebase-multi-line.sh              |  2 +-
 t/t3409-rebase-preserve-merges.sh         |  5 ++++
 t/t3410-rebase-preserve-dropped-merges.sh |  5 ++++
 t/t3411-rebase-preserve-around-merges.sh  |  5 ++++
 t/t3412-rebase-root.sh                    | 12 ++++----
 t/t3414-rebase-preserve-onto.sh           |  5 ++++
 t/t3418-rebase-continue.sh                |  4 +--
 t/t3421-rebase-topology-linear.sh         | 36 +++++++++++------------
 t/t3425-rebase-topology-merges.sh         |  5 ++++
 t/t5520-pull.sh                           |  6 ++--
 t/t7505-prepare-commit-msg-hook.sh        |  2 +-
 t/t7517-per-repo-email.sh                 |  6 ++--
 t/test-lib.sh                             |  4 +++
 14 files changed, 69 insertions(+), 36 deletions(-)

t/t3404-rebase-interactive.sh +4 -4

index 99d1fb79a8..68ca8dc9bb 100755
@@ -312,7 +312,7 @@ test_expect_success 'retain authorship when squashing' '
 	git show HEAD | grep "^Author: Twerp Snog"
 '
 
-test_expect_success '-p handles "no changes" gracefully' '
+test_expect_success REBASE_P '-p handles "no changes" gracefully' '
 	HEAD=$(git rev-parse HEAD) &&
 	set_fake_editor &&
 	git rebase -i -p HEAD^ &&

@@ -322,7 +322,7 @@ test_expect_success '-p handles "no changes" gracefully' '
 	test $HEAD = $(git rev-parse HEAD)
 '
 
-test_expect_failure 'exchange two commits with -p' '
+test_expect_failure REBASE_P 'exchange two commits with -p' '
 	git checkout H &&
 	set_fake_editor &&
 	FAKE_LINES="2 1" git rebase -i -p HEAD~2 &&

@@ -330,7 +330,7 @@ test_expect_failure 'exchange two commits with -p' '
 	test G = $(git cat-file commit HEAD | sed -ne \$p)
 '
 
-test_expect_success 'preserve merges with -p' '
+test_expect_success REBASE_P 'preserve merges with -p' '
 	git checkout -b to-be-preserved master^ &&
 	: > unrelated-file &&
 	git add unrelated-file &&

@@ -373,7 +373,7 @@ test_expect_success 'preserve merges with -p' '
 	test $(git show HEAD:unrelated-file) = 1
 '
 
-test_expect_success 'edit ancestor with -p' '
+test_expect_success REBASE_P 'edit ancestor with -p' '
 	set_fake_editor &&
 	FAKE_LINES="1 2 edit 3 4" git rebase -i -p HEAD~3 &&
 	echo 2 > unrelated-file &&

t/t3408-rebase-multi-line.sh +1 -1

index e7292f5b9b..d2bd7c17b0 100755
@@ -52,7 +52,7 @@ test_expect_success rebase '
 	test_cmp expect actual
 
 '
-test_expect_success rebasep '
+test_expect_success REBASE_P rebasep '
 
 	git checkout side-merge &&
 	git rebase -p side &&

t/t3409-rebase-preserve-merges.sh +5 -0

index 8c251c57a6..3b340f1ece 100755
@@ -8,6 +8,11 @@ Run "git rebase -p" and check that merges are properly carried along
 '
 . ./test-lib.sh
 
+if ! test_have_prereq REBASE_P; then
+	skip_all='skipping git rebase -p tests, as asked for'
+	test_done
+fi
+
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL

t/t3410-rebase-preserve-dropped-merges.sh +5 -0

index 6f73b95558..2e29866993 100755
@@ -11,6 +11,11 @@ rewritten.
 '
 . ./test-lib.sh
 
+if ! test_have_prereq REBASE_P; then
+	skip_all='skipping git rebase -p tests, as asked for'
+	test_done
+fi
+
 # set up two branches like this:
 #
 # A - B - C - D - E

t/t3411-rebase-preserve-around-merges.sh +5 -0

index dc81bf27eb..fb45e7bf7b 100755
@@ -10,6 +10,11 @@ a merge to before the merge.
 '
 . ./test-lib.sh
 
+if ! test_have_prereq REBASE_P; then
+	skip_all='skipping git rebase -p tests, as asked for'
+	test_done
+fi
+
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
 set_fake_editor

t/t3412-rebase-root.sh +6 -6

index 73a39f2923..21632a984e 100755
@@ -86,14 +86,14 @@ test_expect_success 'pre-rebase got correct input (4)' '
 	test "z$(cat .git/PRE-REBASE-INPUT)" = z--root,work4
 '
 
-test_expect_success 'rebase -i -p with linear history' '
+test_expect_success REBASE_P 'rebase -i -p with linear history' '
 	git checkout -b work5 other &&
 	git rebase -i -p --root --onto master &&
 	git log --pretty=tformat:"%s" > rebased5 &&
 	test_cmp expect rebased5
 '
 
-test_expect_success 'pre-rebase got correct input (5)' '
+test_expect_success REBASE_P 'pre-rebase got correct input (5)' '
 	test "z$(cat .git/PRE-REBASE-INPUT)" = z--root,
 '

@@ -120,7 +120,7 @@ commit work6~4
 1
 EOF
 
-test_expect_success 'rebase -i -p with merge' '
+test_expect_success REBASE_P 'rebase -i -p with merge' '
 	git checkout -b work6 other &&
 	git rebase -i -p --root --onto master &&
 	log_with_names work6 > rebased6 &&

@@ -155,7 +155,7 @@ commit work7~5
 1
 EOF
 
-test_expect_success 'rebase -i -p with two roots' '
+test_expect_success REBASE_P 'rebase -i -p with two roots' '
 	git checkout -b work7 other &&
 	git rebase -i -p --root --onto master &&
 	log_with_names work7 > rebased7 &&

@@ -261,7 +261,7 @@ commit conflict3~6
 1
 EOF
 
-test_expect_success 'rebase -i -p --root with conflict (first part)' '
+test_expect_success REBASE_P 'rebase -i -p --root with conflict (first part)' '
 	git checkout -b conflict3 other &&
 	test_must_fail git rebase -i -p --root --onto master &&
 	git ls-files -u | grep "B$"

@@ -272,7 +272,7 @@ test_expect_success 'fix the conflict' '
 	git add B
 '
 
-test_expect_success 'rebase -i -p --root with conflict (second part)' '
+test_expect_success REBASE_P 'rebase -i -p --root with conflict (second part)' '
 	git rebase --continue &&
 	log_with_names conflict3 >out &&
 	test_cmp expect-conflict-p out

t/t3414-rebase-preserve-onto.sh +5 -0

index ee0a6cccfd..72e04b5386 100755
@@ -10,6 +10,11 @@ aren'"'"'t on top of $ONTO, even if they are on top of $UPSTREAM.
 '
 . ./test-lib.sh
 
+if ! test_have_prereq REBASE_P; then
+	skip_all='skipping git rebase -p tests, as asked for'
+	test_done
+fi
+
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
 # Set up branches like this:

t/t3418-rebase-continue.sh +2 -2

index 031e3d8ddb..f0025c7810 100755
@@ -106,7 +106,7 @@ test_expect_success 'rebase -i --continue handles merge strategy and options' '
 	test -f funny.was.run
 '
 
-test_expect_success 'rebase passes merge strategy options correctly' '
+test_expect_success REBASE_P 'rebase passes merge strategy options correctly' '
 	rm -fr .git/rebase-* &&
 	git reset --hard commit-new-file-F3-on-topic-branch &&
 	test_commit theirs-to-merge &&

@@ -241,6 +241,6 @@ test_rerere_autoupdate
 test_rerere_autoupdate -m
 GIT_SEQUENCE_EDITOR=: && export GIT_SEQUENCE_EDITOR
 test_rerere_autoupdate -i
-test_rerere_autoupdate --preserve-merges
+test_have_prereq !REBASE_P || test_rerere_autoupdate --preserve-merges
 
 test_done

t/t3421-rebase-topology-linear.sh +18 ~~-18~~

index 99b2aac921..23ad4cff35 100755
@@ -29,7 +29,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 test_run_rebase () {
 	result=$1

@@ -43,7 +43,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 test_run_rebase () {
 	result=$1

@@ -59,7 +59,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase failure -p
+test_have_prereq !REBASE_P || test_run_rebase failure -p
 
 test_run_rebase () {
 	result=$1

@@ -73,7 +73,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 #       f
 #      /

@@ -113,7 +113,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase failure -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 test_run_rebase () {
 	result=$1

@@ -128,7 +128,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase failure -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 test_run_rebase () {
 	result=$1

@@ -143,7 +143,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase failure -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 test_run_rebase () {
 	result=$1

@@ -158,7 +158,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 # a---b---c---j!
 #      \

@@ -186,7 +186,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 test_run_rebase () {
 	result=$1

@@ -201,7 +201,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase failure -p
+test_have_prereq !REBASE_P || test_run_rebase failure -p
 
 test_run_rebase () {
 	result=$1

@@ -216,7 +216,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase failure -p
+test_have_prereq !REBASE_P || test_run_rebase failure -p
 test_run_rebase success --rebase-merges
 
 #       m

@@ -256,7 +256,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 test_run_rebase () {
 	result=$1

@@ -271,7 +271,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase failure -p
+test_have_prereq !REBASE_P || test_run_rebase failure -p
 
 test_run_rebase () {
 	result=$1

@@ -286,7 +286,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase failure -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 test_run_rebase () {
 	result=$1

@@ -302,7 +302,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase failure -p
+test_have_prereq !REBASE_P || test_run_rebase failure -p
 
 test_run_rebase () {
 	result=$1

@@ -317,7 +317,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase failure -m
 test_run_rebase success -i
-test_run_rebase failure -p
+test_have_prereq !REBASE_P || test_run_rebase failure -p
 
 test_run_rebase () {
 	result=$1

@@ -331,7 +331,7 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase failure -p
+test_have_prereq !REBASE_P || test_run_rebase failure -p
 
 test_run_rebase () {
 	result=$1

@@ -346,6 +346,6 @@ test_run_rebase () {
 test_run_rebase success ''
 test_run_rebase success -m
 test_run_rebase success -i
-test_run_rebase success -p
+test_have_prereq !REBASE_P || test_run_rebase success -p
 
 test_done

t/t3425-rebase-topology-merges.sh +5 -0

index 846f85c27e..5f892e33d7 100755
@@ -109,6 +109,11 @@ test_run_rebase success 'd e n o' ''
 test_run_rebase success 'd e n o' -m
 test_run_rebase success 'd n o e' -i
 
+if ! test_have_prereq REBASE_P; then
+	skip_all='skipping git rebase -p tests, as asked for'
+	test_done
+fi
+
 test_expect_success "rebase -p is no-op in non-linear history" "
 	reset_rebase &&
 	git rebase -p d w &&

t/t5520-pull.sh +4 -2

index 5e501c8b08..cf4cc32fd0 100755
@@ -461,7 +461,8 @@ test_expect_success 'pull.rebase=1 is treated as true and flattens keep-merge' '
 	test file3 = "$(git show HEAD:file3.t)"
 '
 
-test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '
+test_expect_success REBASE_P \
+	'pull.rebase=preserve rebases and merges keep-merge' '
 	git reset --hard before-preserve-rebase &&
 	test_config pull.rebase preserve &&
 	git pull . copy &&

@@ -514,7 +515,8 @@ test_expect_success '--rebase=true rebases and flattens keep-merge' '
 	test file3 = "$(git show HEAD:file3.t)"
 '
 
-test_expect_success '--rebase=preserve rebases and merges keep-merge' '
+test_expect_success REBASE_P \
+	'--rebase=preserve rebases and merges keep-merge' '
 	git reset --hard before-preserve-rebase &&
 	test_config pull.rebase true &&
 	git pull --rebase=preserve . copy &&

t/t7505-prepare-commit-msg-hook.sh +1 -1

index 1f43b3cd4c..ebfcad9c4c 100755
@@ -253,7 +253,7 @@ test_rebase () {
 }
 
 test_rebase success -i
-test_rebase success -p
+test_have_prereq !REBASE_P || test_rebase success -p
 
 test_expect_success 'with hook (cherry-pick)' '
 	test_when_finished "git checkout -f master" &&

t/t7517-per-repo-email.sh +4 -2

index 2a22fa7588..231b8cc19d 100755
@@ -72,12 +72,14 @@ test_expect_success 'noop interactive rebase does not care about ident' '
 	git rebase -i HEAD^
 '
 
-test_expect_success 'fast-forward rebase does not care about ident (preserve)' '
+test_expect_success REBASE_P \
+	'fast-forward rebase does not care about ident (preserve)' '
 	git checkout -B tmp side-without-commit &&
 	git rebase -p master
 '
 
-test_expect_success 'non-fast-forward rebase refuses to write commits (preserve)' '
+test_expect_success REBASE_P \
+	'non-fast-forward rebase refuses to write commits (preserve)' '
 	test_when_finished "git rebase --abort || true" &&
 	git checkout -B tmp side-with-commit &&
 	test_must_fail git rebase -p master

t/test-lib.sh +4 -0

index 897e6fcc94..8b3d0ca7d1 100644
@@ -1268,3 +1268,7 @@ test_lazy_prereq CURL '
 test_lazy_prereq SHA1 '
 	test $(git hash-object /dev/null) = e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
 '
+
+test_lazy_prereq REBASE_P '
+	test -z "$GIT_TEST_SKIP_REBASE_P"
+'

Signature · 1 line

gitgitgadget

Junio C Hamano

2018-11-01 06:12 UTC #4 lore raw

Re: [PATCH 3/3] tests: optionally skip `git rebase -p` tests

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 2 lines · writes:

The --preserve-merges mode of the rebase command is slated to be deprecated soon, ...

Is everybody on board on this statement? I vaguely recall that some people wanted to have something different from what rebase-merges does (e.g. wrt first-parent history), and extending perserve-merges might be one way to do so.

I do not mind at all if the way forward were to extend rebase-merges for any future features. To me, it is preferrable having to deal with just one codepath than two written in different languages.

I just want to make sure we know everybody is on board the plan that we will eventually remove preserve-merges, tell those who want to use it to switch to rebase-merges, and we will extend rebase-merges when they raise issues with it saying that they cannot do something preserve-merges would have served them well with rebase-merges.

Johannes Sixt

2018-11-01 17:18 UTC #5 lore raw

Re: [PATCH 3/3] tests: optionally skip `git rebase -p` tests

Am 01.11.18 um 07:12 schrieb Junio C Hamano:

Quoted 10 lines

"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 2 lines · writes:
The --preserve-merges mode of the rebase command is slated to be deprecated soon, ...
Is everybody on board on this statement? I vaguely recall that some people wanted to have something different from what rebase-merges does (e.g. wrt first-parent history), and extending perserve-merges might be one way to do so.

Maybe you are referring to my proposals from a long time ago. My first-parent hack did not work very well, and I have changed my workflow. --preserve-merges is certainly not a feature that *I* would like to keep.

The important question is whether there are too many users of preserve-merges who would be hurt when it is removed. It is irrelevant whether preserve-merges is a good base for extensions because it can easily be resurrected if the need arises.

Quoted 9 lines

I do not mind at all if the way forward were to extend rebase-merges for any future features. To me, it is preferrable having to deal with just one codepath than two written in different languages.
I just want to make sure we know everybody is on board the plan that we will eventually remove preserve-merges, tell those who want to use it to switch to rebase-merges, and we will extend rebase-merges when they raise issues with it saying that they cannot do something preserve-merges would have served them well with rebase-merges.

-- Hannes

Junio C Hamano

2018-11-01 23:46 UTC #6 lore raw

Re: [PATCH 3/3] tests: optionally skip `git rebase -p` tests

Quoted 12 lines · Johannes Sixt

Am 01.11.18 um 07:12 schrieb Junio C Hamano:
Quoted 10 lines
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 2 lines · writes:
The --preserve-merges mode of the rebase command is slated to be deprecated soon, ...
Is everybody on board on this statement? I vaguely recall that some people wanted to have something different from what rebase-merges does (e.g. wrt first-parent history), and extending perserve-merges might be one way to do so.

Quoted 4 lines

Maybe you are referring to my proposals from a long time ago. My first-parent hack did not work very well, and I have changed my workflow. --preserve-merges is certainly not a feature that *I* would like to keep.

Thanks, that reduces my worries.

Quoted 2 lines

The important question is whether there are too many users of preserve-merges who would be hurt when it is removed.

Yes, and the claim this series makes is that there is none and all existing users should be able to happily use the rebase-merges, which also means that we need to commit to improve rebase-merges to support them, if there were some corner cases, which we failed to consider so far, that are not yet served well.

As I said, as long as everybody agrees with the plan (e.g. we'll know when we hear no objections to the planned deprecation in a few weeks), I am perfectly OK with it.

Thanks.