diff mbox series

[v3,4/7] t2402: ensure locked worktree is properly cleaned up

Message ID 20210119212739.77882-5-rafaeloliveira.cs@gmail.com (mailing list archive)
State New
Headers show
Series teach `worktree list` verbose mode and prunable annotations | expand

Commit Message

Rafael Silva Jan. 19, 2021, 9:27 p.m. UTC
In c57b3367be (worktree: teach `list` to annotate locked worktree,
2020-10-11) introduced a new test to ensure locked worktrees are listed
with "locked" annotation. However, the test does not clean up after
itself as "git worktree prune" is not going to remove the locked worktree
in the first place. This not only leaves the test in an unclean state it
also potentially breaks following tests that relies on the
"git worktree list" output.

Let's fix that by unlocking the worktree before the "prune" command.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com>
---
 t/t2402-worktree-list.sh | 1 +
 1 file changed, 1 insertion(+)

Comments

Eric Sunshine Jan. 24, 2021, 7:50 a.m. UTC | #1
On Tue, Jan 19, 2021 at 4:28 PM Rafael Silva
<rafaeloliveira.cs@gmail.com> wrote:
> In c57b3367be (worktree: teach `list` to annotate locked worktree,
> 2020-10-11) introduced a new test to ensure locked worktrees are listed
> with "locked" annotation. However, the test does not clean up after
> itself as "git worktree prune" is not going to remove the locked worktree
> in the first place. This not only leaves the test in an unclean state it
> also potentially breaks following tests that relies on the
> "git worktree list" output.

A couple grammos:

1) Drop "In" from the start of the first sentence.
2) s/relies/rely/

But please do not re-roll just for this. It's good enough as-is.

The patch itself is fine.

> Let's fix that by unlocking the worktree before the "prune" command.
>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com>
Rafael Silva Jan. 24, 2021, 10:19 a.m. UTC | #2
Eric Sunshine writes:

> On Tue, Jan 19, 2021 at 4:28 PM Rafael Silva
> <rafaeloliveira.cs@gmail.com> wrote:
>> In c57b3367be (worktree: teach `list` to annotate locked worktree,
>> 2020-10-11) introduced a new test to ensure locked worktrees are listed
>> with "locked" annotation. However, the test does not clean up after
>> itself as "git worktree prune" is not going to remove the locked worktree
>> in the first place. This not only leaves the test in an unclean state it
>> also potentially breaks following tests that relies on the
>> "git worktree list" output.
>
> A couple grammos:
>
> 1) Drop "In" from the start of the first sentence.
> 2) s/relies/rely/
>
> But please do not re-roll just for this. It's good enough as-is.
>
> The patch itself is fine.
>

Nice catch. I'll be re-rolling the series with the documentation addition
suggested by Phillip and the test fixes suggested by you. So, I'll
include these changes as well.
diff mbox series

Patch

diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
index 795ddca2e4..1866ea09f6 100755
--- a/t/t2402-worktree-list.sh
+++ b/t/t2402-worktree-list.sh
@@ -66,6 +66,7 @@  test_expect_success '"list" all worktrees with locked annotation' '
 	git worktree add --detach locked master &&
 	git worktree add --detach unlocked master &&
 	git worktree lock locked &&
+	test_when_finished "git worktree unlock locked" &&
 	git worktree list >out &&
 	grep "/locked  *[0-9a-f].* locked$" out &&
 	! grep "/unlocked  *[0-9a-f].* locked$" out