On Mon, Feb 24, 2020 at 6:09 AM Hariom verma <hariom18599@xxxxxxxxx> wrote: > On Mon, Feb 24, 2020 at 7:12 AM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > > if (!strbuf_strip_suffix(&worktree_path, "/.git/.") && /* in .git dir */ > > !strbuf_strip_suffix(&worktree_path, "/.git/")) /* in worktree */ > > strbuf_strip_suffix(&worktree_path, "/."); /* in bare repo */ > > > > Also, please add a test to ensure that this behavior doesn't regress > > in the future. You can probably test it via the "git worktree list" > > command, so perhaps add the test to t/t2402-worktree-list.sh. > > There already exists tests in "t/t2402-worktree-list.sh" which lists and > verifies all worktrees. Does this make sense to write a new test that > also does kinda the same thing? The change this patch is making is to correctly strip the suffix "/.git/." from the main worktree's path since that suffix was not getting stripped correctly by the existing code. We want a test that verifies that the "/.git/." suffix is indeed being stripped once this change is applied. If there is an existing test which already checks the output of "git worktree list" when invoked from within the .git directory, then that test should suffice, but then I would have expected that you would have had to tweak the existing test to make it succeed after this change. If there is no such test which verifies that "/.git/." is being stripped, then this patch should add one. "git worktree list" would be a possible way to implement such a test.
