Re: [PATCH 6/7] worktree: add tests for `list` verbose and annotations
flat view
From: Eric Sunshine <hidden>
Date: 2021-01-06 09:39:56
On Mon, Jan 4, 2021 at 11:22 AM Rafael Silva [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Add tests for "git worktree list" verbose mode, prunable and locked annotations for both default and porcelain format and ensure the "prunable" annotation is consistent with what "git worktree prune" command will eventually remove. Additionally, add one test case to ensure any newline characters are escaped from locked reason for the porcelain format to prevent breaking the format. The 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 remove the worktree as the "git worktree prune" is not going to remove any locked worktrees. Let's fix that by unlocking the worktree before the "prune" command. Signed-off-by: Rafael Silva <redacted> ---diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh@@ -62,7 +62,9 @@ test_expect_success '"list" all worktrees --porcelain' ' test_expect_success '"list" all worktrees with locked annotation' ' - test_when_finished "rm -rf locked unlocked out && git worktree prune" && + test_when_finished "rm -rf locked unlocked out && + git worktree unlock locked && + git worktree prune" && git worktree add --detach locked master && git worktree add --detach unlocked master && git worktree lock locked &&
You might need to be a bit more careful here. If the test fails before
the worktree is locked, then the `git worktree unlock` in the cleanup
code will return an error, which will make the code executed by
test_when_finished() fail, as well, which makes it harder to debug
problems. Moving the `unlock` cleanup after you know the lock
succeeded will address this issue:
test_when_finished "rm -rf locked unlocked out && git worktree prune" &&
git worktree add --detach locked master &&
git worktree add --detach unlocked master &&
git worktree lock locked &&
test_when_finished "git worktree unlock locked" &&
...
Same comment applies to other new tests added by this patch which lock
worktrees.
quoted hunk ↗ jump to hunk
@@ -71,6 +73,96 @@ test_expect_success '"list" all worktrees with locked annotation' ' +test_expect_success '"list" all worktrees with prunable consistent with "prune"' ' + test_when_finished "rm -rf prunable out && git worktree prune" && + git worktree add --detach prunable && + rm -rf prunable && + git worktree list >out && + grep "/prunable *[0-9a-f].* prunable$" out && + git worktree prune --verbose >out && + test_i18ngrep "^Removing worktrees/prunable" out +'
To be trustworthy, doesn't this test also need to have an unprunable worktree, and test that `git worktree list` doesn't annotate it as "prunable" _and_ that `git worktree prune` didn't prune it? Otherwise, we really don't know that the two commands agree about what is and is not prunable -- we only know they agree that this particular worktree was prunable.
quoted hunk ↗ jump to hunk
+test_expect_success '"list" all worktrees --porcelain with newline escaped locked reason' ' + test_when_finished "rm -rf locked_lf locked_crlf reason_lf reason_crlf out actual expect reason && + git worktree unlock locked_lf && + git worktree unlock locked_crlf && + git worktree prune" &&
Nit: It's not a big deal, but we don't normally bother cleaning up every junk file in tests, such as `out`, `actual`, `expect` if those files aren't going to be a problem for subsequent tests. We are explicitly cleaning up the worktrees in these tests because this script is all about testing worktree behavior, and some random leftover worktree could be a problem for other tests. I don't care strongly one way or the other, but I worry a tiny bit that the list of files being cleaned up could become outdated as changes are made to the tests later...
quoted hunk ↗ jump to hunk
+test_expect_success '"list" all worktrees --porcelain with prunable' ' + test_when_finished "rm -rf prunable list out && git worktree prune" && + git worktree add --detach prunable && + rm -rf prunable && + git worktree list --porcelain >out && + test_i18ngrep "^prunable gitdir file points to non-existent location$" out +'
... for instance, the file `list` being cleaned up in this test is not even created by this test.
quoted hunk ↗ jump to hunk
+ +test_expect_success '"list" all worktrees --verbose and --porcelain mutually exclusive' ' + test_must_fail git worktree list --verbose --porcelain +'
Nit: This test title could probably be shortened to:
'"list' --verbose and --porcelain mutually exclusive' '
Finally, I haven't tried it myself, but I was wondering if it is
possible to make a test which shows both "locked" and "prunable"
annotations for the same worktree. Would that be possible by
specifying a particular value for `--expire`? If it's too much work,
don't worry about it.