From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sun, 21 Nov 2021 17:57:11 GMT
Subject: Re: [PATCH 1/8] t2501: add various tests for removing the current working directory
Message-ID: <211121.867dd11jbl.gmgdl@evledraar.gmail.com>
In-Reply-To: <0b71996a3b462d4147fb792b20057544b9ef1710.1637455620.git.gitgitgadget@gmail.com>


On Sun, Nov 21 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>

> +test_expect_failure 'checkout fails if cwd needs to be removed' '
> +	git checkout foo/bar/baz &&
> +	test_when_finished "git clean -fdx" &&
> +
> +	mkdir dirORfile &&
> +	(
> +		cd dirORfile &&
> +
> +		test_must_fail git checkout fd_conflict 2>../error &&
> +		grep "Refusing to remove the current working directory" ../error
> +	) &&
> +
> +	test_path_is_dir dirORfile


I'd find this & the rest of this series much easier to understand if we
started out by positively asserting the current behavior here, and
didn't test_cmp/grep for erro r messages that don't exist anymore.

It would also help to show how exactly operations that currently "work"
behave, e.g. if you git checkout a revision within "t/" which is a
subdir, and it turns into "t" the file etc.

I'm also generally knee-jerk reactionary to test_expect_failure for its
fragility[1], but in this case more because it makes explaining this
step-by-step harder & not as obvious.

1. https://lore.kernel.org/git/87tuhmk19c.fsf@evledraar.gmail.com/