Re: [PATCH v3 0/4] breaking changes: switch default initial branch name to "main"
From: Junio C Hamano <hidden>
Date: 2025-09-17 15:21:33
Phillip Wood [off-list ref] writes:
quoted hunk
The test changes up to this point strike me as somewhat unfortunate as these tests are perfectly content with the existing advice. I assume this stems from us not setting GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME anymore when WITH_BREAKING_CHANGES is enabled. I think we should do something likediff --git b/t/test-lib.sh b/t/test-lib.sh --- a/t/test-lib.sh +++ b/t/test-lib.sh@@ -127,13 +127,15 @@ then export GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS fi +# Explicitly set the default branch name for testing, to avoid the +# transitory "git init" warning under --verbose. if test -z "$WITH_BREAKING_CHANGES" then - # Explicitly set the default branch name for testing, to avoid the - # transitory "git init" warning under --verbose. : ${GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME:=master} - export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +else + : ${GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME:=main} fi +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
That does make it more in line in spirit with what your "breaking-changes: switch default branch to main" does here. Instead of assuming that we won't have "no default set" hint after 3.0, we do assume the "no default set" hint will be there and squelch for a bit longer. OK.
To address that which will also stop the advice appearing when the tests are run with --verbose as well.quoted
git -C repo rev-parse --show-ref-format >actual && echo $GIT_DEFAULT_REF_FORMAT >expected &&@@ -868,18 +868,18 @@ test_expect_success 'overridden default initial branch name (config)' ' grep nmb actual ' -test_expect_success !WITH_BREAKING_CHANGES 'advice onunconfigured init.defaultBranch' ' +test_expect_success 'advice on unconfigured init.defaultBranch' ' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= git -c color.advice=always \ init unconfigured-default-branch-name 2>err && test_decode_color <err >decoded && test_grep "<YELLOW>hint: " decoded ' -test_expect_success 'advice on unconfigured init.defaultBranch disabled' ' +test_expect_success 'advice on unconfigured init.defaultBranch can be disabled' ' test_when_finished "rm -rf no-advice" && GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \ - git -c advice.defaultBranchName=false init no-advice 2>err && + git -c advice.defaultBranchName=false init no-advice 2>err &&The indentation is changed here but not above. I'd be happy to leave this alone, but if we're going to remove the indentation here shouldn't we do the same above?
QUite honestly, I didn't even notice that the line that begins with "init unconfigured..." was a continuation line. We definitely should fix it to match.
quoted
test_grep ! "hint: " errDo we want to check the advice that is printed now that we have two different messages?
I am not sure; isn't this checking that there is no advice, not two advice messages? Ahh, you mean two new tests, one with !WITH_BREAKING_CHANGES and one with WITH_BREAKING_CHANGES, to see default_branch_name_advice[] appears in the advice output? I am not sure if that is needed here. Without [PATCH 5/4] there is no test that checks the exact wording of "... will change to main in Git 3.0" message, either, and I think that is a sensible thing *not* to have a test that insists on the exact phrasing for such things.