t0050: use explicit branch name and verify checkout result#2054
Open
mdferdousalam wants to merge 1 commit intogitgitgadget:masterfrom
Open
t0050: use explicit branch name and verify checkout result#2054mdferdousalam wants to merge 1 commit intogitgitgadget:masterfrom
mdferdousalam wants to merge 1 commit intogitgitgadget:masterfrom
Conversation
Author
|
/submit |
|
Submitted as pull.2054.git.1771837399472.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
Author
|
/submit |
|
Error: 06df4ca was already submitted |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Md Ferdous Alam via GitGitGadget" <gitgitgadget@gmail.com> writes:
Adding CC the authors of 69f272b9 (dir: special case check for the
possibility that pathspec is NULL, 2019-10-01) and 06d53148 (t[01]*:
adjust the references to the default branch name "main", 2020-11-18)
the block of lines removed by this patch are blamed for.
One thing that I noticed that needs checking but I didn't do so
myself is that the original is prepared not to break after Git 3.0
by using 'main' in t0050 (which forces the initial branch name to be
'main'). Whereever the corresponding new code goes, there needs a
similar provision to prevent the test from getting broken with the
default change.
I think specifying the initial branch name explicitly when the test
creates "repo-case" test repository and use that to go back to that
branch would be better for the purpose of this single test, rather
than using the blanket "GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME" for
the entire script.
> From: mdferdousalam <mdferdousalam1989@yahoo.com>
>
> The test 'checkout with no pathspec and a case insensitive fs' in
> t0050 does not really belong there as it tests branch checkout
> behavior, not filesystem properties. It also had an unnecessary
> CASE_INSENSITIVE_FS prereq since the sequence of commands should
> succeed on any filesystem, and it did not verify the resulting
> worktree contents.
>
> Move it to t2018-checkout-branch.sh where it belongs, drop the
> prereq, and add a check that the expected file is present after
> the checkout.
>
> Signed-off-by: mdferdousalam <mdferdousalam1989@yahoo.com>
> ---
> t2018: move checkout case-insensitive test from t0050
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2054%2Fmdferdousalam%2Fmove-checkout-test-from-t0050-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2054/mdferdousalam/move-checkout-test-from-t0050-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2054
>
> t/t0050-filesystem.sh | 20 --------------------
> t/t2018-checkout-branch.sh | 21 +++++++++++++++++++++
> 2 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
> index ca8568067d..003329c082 100755
> --- a/t/t0050-filesystem.sh
> +++ b/t/t0050-filesystem.sh
> @@ -117,24 +117,4 @@ $test_unicode 'merge (silent unicode normalization)' '
> git merge topic
> '
>
> -test_expect_success CASE_INSENSITIVE_FS 'checkout with no pathspec and a case insensitive fs' '
> - git init repo &&
> - (
> - cd repo &&
> -
> - >Gitweb &&
> - git add Gitweb &&
> - git commit -m "add Gitweb" &&
> -
> - git checkout --orphan todo &&
> - git reset --hard &&
> - mkdir -p gitweb/subdir &&
> - >gitweb/subdir/file &&
> - git add gitweb &&
> - git commit -m "add gitweb/subdir/file" &&
> -
> - git checkout main
> - )
> -'
> -
> test_done
> diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> index a48ebdbf4d..5f37e40591 100755
> --- a/t/t2018-checkout-branch.sh
> +++ b/t/t2018-checkout-branch.sh
> @@ -285,4 +285,25 @@ test_expect_success 'checkout -b rejects an extra path argument' '
> test_grep "Cannot update paths and switch to branch" err
> '
>
> +test_expect_success 'checkout a branch when file and directory share case-insensitive name' '
> + git init repo-case &&
> + (
> + cd repo-case &&
> +
> + >Gitweb &&
> + git add Gitweb &&
> + git commit -m "add Gitweb" &&
> +
> + git checkout --orphan other &&
> + git reset --hard &&
> + mkdir -p gitweb/subdir &&
> + >gitweb/subdir/file &&
> + git add gitweb &&
> + git commit -m "add gitweb/subdir/file" &&
> +
> + git checkout master &&
> + test_path_is_file Gitweb
> + )
> +'
> +
> test_done
>
> base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4 |
|
Elijah Newren wrote on the Git mailing list (how to reply to this email): On Mon, Feb 23, 2026 at 8:12 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Md Ferdous Alam via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> Adding CC the authors of 69f272b9 (dir: special case check for the
> possibility that pathspec is NULL, 2019-10-01) and 06d53148 (t[01]*:
> adjust the references to the default branch name "main", 2020-11-18)
> the block of lines removed by this patch are blamed for.
>
> One thing that I noticed that needs checking but I didn't do so
> myself is that the original is prepared not to break after Git 3.0
> by using 'main' in t0050 (which forces the initial branch name to be
> 'main'). Whereever the corresponding new code goes, there needs a
> similar provision to prevent the test from getting broken with the
> default change.
>
> I think specifying the initial branch name explicitly when the test
> creates "repo-case" test repository and use that to go back to that
> branch would be better for the purpose of this single test, rather
> than using the blanket "GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME" for
> the entire script.
>
>
> > From: mdferdousalam <mdferdousalam1989@yahoo.com>
> >
> > The test 'checkout with no pathspec and a case insensitive fs' in
> > t0050 does not really belong there as it tests branch checkout
> > behavior, not filesystem properties. It also had an unnecessary
> > CASE_INSENSITIVE_FS prereq since the sequence of commands should
> > succeed on any filesystem, and it did not verify the resulting
> > worktree contents.
Actually, the CASE_INSENSTIVE_FS was very much intentional and
critical to triggering the original bug before it was fixed, and in
fact was overlooked by both Denton (the original reporter) and I,
which made us unable to figure out how to reproduce the segfault he
had triggered. (The testcase does use both "Gitweb" and "gitweb" in
it as paths). Luckily, Gábor came along and noticed the case
differences and created a testcase for us, and then I dug further for
other surrounding issues with that initial guide. See the threads
around https://lore.kernel.org/git/20190925215530.GA9013@generichostname/
and https://lore.kernel.org/git/20190927021746.GL2637@szeder.dev/
> > Move it to t2018-checkout-branch.sh where it belongs, drop the
> > prereq,
I think it belongs where it already is, and the prereq should be kept.
> > and add a check that the expected file is present after
> > the checkout.
Not sure if that's worth changing, but if others feel strongly then it
doesn't hurt anything.
> >
> > Signed-off-by: mdferdousalam <mdferdousalam1989@yahoo.com>
From Documentation/SubmittingPatches:
"""
Please use a known identity in the `Signed-off-by` trailer, since we cannot
accept anonymous contributions. It is common, but not required, to use some form
of your real name. We realize that some contributors are not comfortable doing
so or prefer to contribute under a pseudonym or preferred name and we can accept
your patch either way, as long as the name and email you use are distinctive,
identifying, and not misleading.
The goal of this policy is to allow us to have sufficient information to contact
you if questions arise about your contribution.
"""
Since your patch was sent by "Md Ferdous Alam via GitGitGadget" I
suspect that your Signoff should have been "Signed-off-by: Md Ferdous
Alam <mdferdousalam1989@yahoo.com>" (and the From line updated to
match). If I'm wrong about that, it might be helpful for you to
include an explanation of the name differences with or before your
next patch submission. |
|
User |
06df4ca to
8a1cb73
Compare
The test 'checkout with no pathspec and a case insensitive fs' relies on the script-wide GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME to set the initial branch to 'main'. Use 'git init -b main' instead, so that the test is self-contained and does not depend on the blanket setting for the entire script. While at it, add a test_path_is_file check after the checkout to verify that the expected file is present in the worktree. Signed-off-by: Md Ferdous Alam <mdferdousalam1989@yahoo.com>
8a1cb73 to
ac7c0a3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The test 'checkout with no pathspec and a case insensitive fs' in
t0050 relies on the script-wide GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
to set the initial branch to 'main'. Use 'git init -b main' instead,
so that the test is self-contained. Also add a test_path_is_file
check to verify the checkout result.
Changes since v1:
original bug (Elijah Newren)
relying on GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME (Junio C Hamano)
cc: Elijah Newren newren@gmail.com