From: Eric Sunshine Date: Mon, 24 Feb 2020 17:00:43 GMT Subject: Re: [PATCH v3 1/3] get_main_worktree(): allow it to be called in the Git directory Message-ID: In-Reply-To: On Mon, Feb 24, 2020 at 6:09 AM Hariom verma wrote: > On Mon, Feb 24, 2020 at 7:12 AM Eric Sunshine 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.