[3/3] tests: optionally skip `git rebase -p` tests
diff mbox series

Message ID 39734e4805cbd695cd69e7c1a3016de629ac9b3c.1541016115.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • tests: allow to skip git rebase -p tests
Related show

Commit Message

sunlin via GitGitGadget Oct. 31, 2018, 8:02 p.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 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(-)

Comments

Junio C Hamano Nov. 1, 2018, 6:12 a.m. UTC | #1
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
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 Nov. 1, 2018, 5:18 p.m. UTC | #2
Am 01.11.18 um 07:12 schrieb Junio C Hamano:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> 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.

> 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 Nov. 1, 2018, 11:45 p.m. UTC | #3
Johannes Sixt <j6t@kdbg.org> writes:

> Am 01.11.18 um 07:12 schrieb Junio C Hamano:
>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
>> 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.

Thanks, that reduces my worries.

> 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.

Patch
diff mbox series

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 99d1fb79a8..68ca8dc9bb 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -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 &&
diff --git a/t/t3408-rebase-multi-line.sh b/t/t3408-rebase-multi-line.sh
index e7292f5b9b..d2bd7c17b0 100755
--- a/t/t3408-rebase-multi-line.sh
+++ b/t/t3408-rebase-multi-line.sh
@@ -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 &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 8c251c57a6..3b340f1ece 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -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
 
diff --git a/t/t3410-rebase-preserve-dropped-merges.sh b/t/t3410-rebase-preserve-dropped-merges.sh
index 6f73b95558..2e29866993 100755
--- a/t/t3410-rebase-preserve-dropped-merges.sh
+++ b/t/t3410-rebase-preserve-dropped-merges.sh
@@ -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
diff --git a/t/t3411-rebase-preserve-around-merges.sh b/t/t3411-rebase-preserve-around-merges.sh
index dc81bf27eb..fb45e7bf7b 100755
--- a/t/t3411-rebase-preserve-around-merges.sh
+++ b/t/t3411-rebase-preserve-around-merges.sh
@@ -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
diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
index 73a39f2923..21632a984e 100755
--- a/t/t3412-rebase-root.sh
+++ b/t/t3412-rebase-root.sh
@@ -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
diff --git a/t/t3414-rebase-preserve-onto.sh b/t/t3414-rebase-preserve-onto.sh
index ee0a6cccfd..72e04b5386 100755
--- a/t/t3414-rebase-preserve-onto.sh
+++ b/t/t3414-rebase-preserve-onto.sh
@@ -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:
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 031e3d8ddb..f0025c7810 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -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
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index 99b2aac921..23ad4cff35 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -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
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
index 846f85c27e..5f892e33d7 100755
--- a/t/t3425-rebase-topology-merges.sh
+++ b/t/t3425-rebase-topology-merges.sh
@@ -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 &&
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 5e501c8b08..cf4cc32fd0 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -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 &&
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
index 1f43b3cd4c..ebfcad9c4c 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -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" &&
diff --git a/t/t7517-per-repo-email.sh b/t/t7517-per-repo-email.sh
index 2a22fa7588..231b8cc19d 100755
--- a/t/t7517-per-repo-email.sh
+++ b/t/t7517-per-repo-email.sh
@@ -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
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 897e6fcc94..8b3d0ca7d1 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -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"
+'