diff mbox series

[v1,1/4] t: introduce WITH_BREAKING_CHANGES prerequisite

Message ID 20250310231652.3742490-2-gitster@pobox.com (mailing list archive)
State Superseded
Headers show
Series drop "name-rev --stdin" support | expand

Commit Message

Junio C Hamano March 10, 2025, 11:16 p.m. UTC
Earlier c5bc9a7f (Makefile: wire up build option for deprecated
features, 2025-01-22) made an unfortunate decision to introduce the
WITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure
the historical behaviour that may be different from what we will
have in the future.  It would inevitably invite doulbe negation when
we need to add tests to ensure the behaviour we want to have in the
future.

Introduce WITH_BREAKING_CHANGES prerequisite and replace the
existing uses of WITHOUT_BREAKING_CHANGES prerequisite.  Some
in-flight topics that add more uses of WITHOUT_BREAKING_CHANGES
would still need the old prerequisite, so let's keep its definition
for now while we'll eradicate its use.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5323-pack-redundant.sh    | 2 +-
 t/t5505-remote.sh            | 6 +++---
 t/t5515-fetch-merge-logic.sh | 2 +-
 t/t5516-fetch-push.sh        | 8 ++++----
 t/test-lib.sh                | 5 +++++
 5 files changed, 14 insertions(+), 9 deletions(-)

Comments

Eric Sunshine March 10, 2025, 11:53 p.m. UTC | #1
On Mon, Mar 10, 2025 at 7:17 PM Junio C Hamano <gitster@pobox.com> wrote:
> Earlier c5bc9a7f (Makefile: wire up build option for deprecated
> features, 2025-01-22) made an unfortunate decision to introduce the
> WITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure
> the historical behaviour that may be different from what we will
> have in the future.  It would inevitably invite doulbe negation when
> we need to add tests to ensure the behaviour we want to have in the
> future.

s/doulbe/double/

> Introduce WITH_BREAKING_CHANGES prerequisite and replace the
> existing uses of WITHOUT_BREAKING_CHANGES prerequisite.  Some
> in-flight topics that add more uses of WITHOUT_BREAKING_CHANGES
> would still need the old prerequisite, so let's keep its definition
> for now while we'll eradicate its use.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt March 11, 2025, 12:57 p.m. UTC | #2
On Mon, Mar 10, 2025 at 04:16:49PM -0700, Junio C Hamano wrote:
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 9001ed3a64..12fe82f660 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1862,6 +1862,11 @@ test_lazy_prereq CURL '
>  	curl --version
>  '
>  
> +test_lazy_prereq WITH_BREAKING_CHANGES '
> +	test -n "$WITH_BREAKING_CHANGES"
> +'
> +
> +# DEPRECATED; DO NOT USE THIS IN NEW TESTS
>  test_lazy_prereq WITHOUT_BREAKING_CHANGES '
>  	test -z "$WITH_BREAKING_CHANGES"
>  '

Do we maybe want to state that this can be removed once the next release
cycle is over? I find it to be a bit more actionable when stating hard
dates after which something can be dropped as it allows any drive-by
developers to act when they notice that Git v2.50 has been released
already.

Patrick
Junio C Hamano March 11, 2025, 5:07 p.m. UTC | #3
Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Mar 10, 2025 at 04:16:49PM -0700, Junio C Hamano wrote:
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 9001ed3a64..12fe82f660 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -1862,6 +1862,11 @@ test_lazy_prereq CURL '
>>  	curl --version
>>  '
>>  
>> +test_lazy_prereq WITH_BREAKING_CHANGES '
>> +	test -n "$WITH_BREAKING_CHANGES"
>> +'
>> +
>> +# DEPRECATED; DO NOT USE THIS IN NEW TESTS
>>  test_lazy_prereq WITHOUT_BREAKING_CHANGES '
>>  	test -z "$WITH_BREAKING_CHANGES"
>>  '
>
> Do we maybe want to state that this can be removed once the next release
> cycle is over?

Perhaps.  As 'seen' is pretty-much closed at this point and there is
nothing in flight that uses WITHOUT_ variant in there, v2 of this
series can just do without it, which may be simpler.

> I find it to be a bit more actionable when stating hard
> dates after which something can be dropped 

True, that is a good strategy for a transition that takes longer
time.
diff mbox series

Patch

diff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh
index 688cd9706c..bc30bc9652 100755
--- a/t/t5323-pack-redundant.sh
+++ b/t/t5323-pack-redundant.sh
@@ -36,7 +36,7 @@  relationship between packs and objects is as follows:
 
 . ./test-lib.sh
 
-if ! test_have_prereq WITHOUT_BREAKING_CHANGES
+if test_have_prereq WITH_BREAKING_CHANGES
 then
 	skip_all='skipping git-pack-redundant tests; built with breaking changes'
 	test_done
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index bb7e0c6879..82fccf8e36 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -1123,7 +1123,7 @@  Pull: refs/heads/main:refs/heads/origin
 Pull: refs/heads/next:refs/heads/origin2
 EOF
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '
+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '
 	git clone one five &&
 	origin_url=$(pwd)/one &&
 	(
@@ -1149,7 +1149,7 @@  test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i
 	)
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '
+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '
 	git clone --template= one six &&
 	origin_url=$(pwd)/one &&
 	(
@@ -1165,7 +1165,7 @@  test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i
 	)
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '
+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '
 	git clone --template= one seven &&
 	(
 		cd seven &&
diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
index 4e6026c611..8ac04d742c 100755
--- a/t/t5515-fetch-merge-logic.sh
+++ b/t/t5515-fetch-merge-logic.sh
@@ -104,7 +104,7 @@  test_expect_success setup '
 	git config remote.config-glob.fetch refs/heads/*:refs/remotes/rem/* &&
 	remotes="$remotes config-glob" &&
 
-	if test_have_prereq WITHOUT_BREAKING_CHANGES
+	if ! test_have_prereq WITH_BREAKING_CHANGES
 	then
 		mkdir -p .git/remotes &&
 		cat >.git/remotes/remote-explicit <<-\EOF &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 85ed049627..6e2b233157 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -975,7 +975,7 @@  test_expect_success 'allow push to HEAD of non-bare repository (config)' '
 	! grep "warning: updating the current branch" stderr
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '
+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches' '
 	mk_empty testrepo &&
 	git branch second $the_first_commit &&
 	git checkout second &&
@@ -991,7 +991,7 @@  test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '
 	git checkout main
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #' '
+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches containing #' '
 	mk_empty testrepo &&
 	mkdir testrepo/.git/branches &&
 	echo "..#second" > testrepo/.git/branches/branch2 &&
@@ -1005,7 +1005,7 @@  test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #'
 	git checkout main
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '
+test_expect_success !WITH_BREAKING_CHANGES 'push with branches' '
 	mk_empty testrepo &&
 	git checkout second &&
 
@@ -1022,7 +1022,7 @@  test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '
 	)
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches containing #' '
+test_expect_success !WITH_BREAKING_CHANGES 'push with branches containing #' '
 	mk_empty testrepo &&
 
 	test_when_finished "rm -rf .git/branches" &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9001ed3a64..12fe82f660 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1862,6 +1862,11 @@  test_lazy_prereq CURL '
 	curl --version
 '
 
+test_lazy_prereq WITH_BREAKING_CHANGES '
+	test -n "$WITH_BREAKING_CHANGES"
+'
+
+# DEPRECATED; DO NOT USE THIS IN NEW TESTS
 test_lazy_prereq WITHOUT_BREAKING_CHANGES '
 	test -z "$WITH_BREAKING_CHANGES"
 '