Re: [PATCH v2 28/27] tests: run tests omitted by PREPARE_FOR_MAIN_BRANCH

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 18, 2020, 23:56 UTC
Message-ID: <nycvar.QRO.7.76.6.2011190044100.56@tvgsbejvaqbjf.bet>
In-Reply-To: <20201118114834.11137-1-avarab@gmail.com>

Hi Ævar,

On Wed, 18 Nov 2020, Ævar Arnfjörð Bjarmason wrote:

Show 37 quoted lines

> Reinstate the test coverage lost due to PREPARE_FOR_MAIN_BRANCH. The
> remaining impact of that prerequisite was mainly missing coverage from
> submodule fetches being lost[1], e.g. impacting my in-flight
> ab/retire-parse-remote. Now the prerequisite is effectively a
> noop. This goes on top of [2].
>
> I'm not removing the PREPARE_FOR_MAIN_BRANCH prerequisite to keep this
> change small, instead it's now effectively a noop. It can be removed
> in some later change.
>
> The only remaining occurrences in t5526-fetch-submodules.sh can be
> removed without breakage with:
>
>     perl -pi -e 's/PREPARE_FOR_MAIN_BRANCH //g' t/t5526-fetch-submodules.sh
>
> Which at this point leaves only the now-unused prerequisite
> declaration in test-lib.sh.
>
> The coverage in t9902-completion.sh was restored by partially
> reverting[3]. After that we were left with one test in a mixed
> state. It setup "master" but tested for "mai". Change it back to
> "mas", pending a more complete refactoring of that test.
>
> This change only conflicts between next..seen by clashing with Peter
> Kaestle's in-flights submodule fix[4]. Fixing the resulting logic
> error in t5526-fetch-submodules.sh is trivial, simply:
>
>     - compare_refs_in_dir A origin/master B origin/master
>     + compare_refs_in_dir A origin/main B origin/main
>
> 1. 66713e84e7 ("tests: prepare aligned mentions of the default branch name", 2020-10-23)
> 2. https://public-inbox.org/git/pull.762.v2.git.1605629547.gitgitgadget@gmail.com/
> 3. 8164360fc8 ("t9902: prepare a test for the upcoming default branch name", 2020-10-23)
> 4. https://public-inbox.org/git/1605196853-37359-1-git-send-email-peter.kaestle@nokia.com/
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---

Hmm. I have the feeling that doing this will just delay everything even further. I had really hoped that we could get this done without "three steps forward, one step back" dances.

Show 14 quoted lines

> On Tue, Nov 17 2020, Johannes Schindelin via GitGitGadget wrote:
> > To avoid even more conflicts with topics that did not even make it to seen
> > yet, this patch series specifically excludes t3404, t4013, t5310, t5526,
> > t6300, t7064, t7817, t9902: in those test scripts, we will still use master
> > for the time being. Once the topics in question have settled, I will send
> > the appropriate follow-up patches to adjust them to use main instead.
>
> This is not a replacement for that subsequent cleanup, but seems like
> a simple enough thing to have now to reinstate the missing test
> coverage.
>
> Perhaps there's some topics not in "seen" that you have in mind as
> conflicting, but as noted above the conflict produced here with
> in-flight in "seen" is trivial.

The conflicts might be trivial, but every conflict makes it harder to juggle branch thickets like Junio's `seen`. And those things add up. They're no fun, and they make life hard and tedious.

t5526 conflicts _semantically_ with `pk/subsub-fetch-fix` (which touches t5526 and adds a new test case referring to `master`).

t9902 conflicts with `fc/bash-completion-post-2.29`, and in contrast to the t5526 issue (which is trivial, even if it does require manual fixing), the t9902 is a bit more hairy to reason about.

So yes, I would love to have that test coverage back, but not by making the transition to `main` even harder by reverting parts of it.

That's why I promised publicly to take care of the loose ends as quickly as I can, after the conflicting issues graduate to `next` (or when they become stalled or even dropped from `seen`).

Ciao, Dscho

Show 88 quoted lines

>
> Seems worth having that sooner than later if Junio's willing juggle
> that.
>
>  t/t5526-fetch-submodules.sh | 6 +++---
>  t/t9902-completion.sh       | 6 +++---
>  t/test-lib.sh               | 9 +++------
>  3 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> index dd8e423d25..f45ba02b8a 100755
> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -481,7 +481,7 @@ test_expect_success PREPARE_FOR_MAIN_BRANCH "don't fetch submodule when newly re
>  	test_i18ncmp expect.err actual.err &&
>  	(
>  		cd submodule &&
> -		git checkout -q master
> +		git checkout -q main
>  	)
>  '
>
> @@ -663,9 +663,9 @@ test_expect_success 'fetch new submodule commits on-demand without .gitmodules e
>  	git config -f .gitmodules --remove-section submodule.sub1 &&
>  	git add .gitmodules &&
>  	git commit -m "delete gitmodules file" &&
> -	git checkout -B master &&
> +	git checkout -B main &&
>  	git -C downstream fetch &&
> -	git -C downstream checkout origin/master &&
> +	git -C downstream checkout origin/main &&
>
>  	C=$(git -C submodule commit-tree -m "yet another change outside refs/heads" HEAD^{tree}) &&
>  	git -C submodule update-ref refs/changes/7 $C &&
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 5c01c75d40..3696b85acb 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1055,13 +1055,13 @@ test_expect_success 'teardown after filtering matching refs' '
>  	git -C otherrepo branch -D matching/branch-in-other
>  '
>
> -test_expect_success PREPARE_FOR_MAIN_BRANCH '__git_refs - for-each-ref format specifiers in prefix' '
> +test_expect_success '__git_refs - for-each-ref format specifiers in prefix' '
>  	cat >expected <<-EOF &&
>  	evil-%%-%42-%(refname)..master
>  	EOF
>  	(
> -		cur="evil-%%-%42-%(refname)..mai" &&
> -		__git_refs "" "" "evil-%%-%42-%(refname).." mai >"$actual"
> +		cur="evil-%%-%42-%(refname)..mas" &&
> +		__git_refs "" "" "evil-%%-%42-%(refname).." mas >"$actual"
>  	) &&
>  	test_cmp expected "$actual"
>  '
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index d39bdf04ce..ed489b2213 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -257,7 +257,7 @@ case "$TRASH_DIRECTORY" in
>  esac
>
>  case "$TEST_NUMBER" in
> -3404|4013|5310|5526|6300|7064|7817|9902)
> +3404|4013|5310|6300|7064|7817|9902)
>  	# Avoid conflicts with patch series that are cooking at the same time
>  	# as the patch series changing the default of `init.defaultBranch`.
>  	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
> @@ -1725,12 +1725,9 @@ test_lazy_prereq REBASE_P '
>  	test -z "$GIT_TEST_SKIP_REBASE_P"
>  '
>
> -# Special-purpose prereq for transitioning to a new default branch name:
> -# Some tests need more than just a mindless (case-preserving) s/master/main/g
> -# replacement. The non-trivial adjustments are guarded behind this
> -# prerequisite, acting kind of as a feature flag
> +# Obsolete, do not use, removed soon!
>  test_lazy_prereq PREPARE_FOR_MAIN_BRANCH '
> -	test "$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME" = main
> +	test "$TEST_NAME" = "t5526-fetch-submodules"
>  '
>
>  # Ensure that no test accidentally triggers a Git command
> --
> 2.29.2.222.g5d2a92d10f8
>
>
>