| Message ID | dd1f6969c28d95027d1ac9f7b6f6a43640babada.1618829583.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Prepare tests for reftable backend | expand |
On Mon, Apr 19 2021, Han-Wen Nienhuys via GitGitGadget wrote: > From: Han-Wen Nienhuys <hanwen@google.com> > > It tries to setup a reflog by catting to .git/logs/ > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > t/t1407-worktree-ref-store.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh > index d3fe77751122..27b57f248a94 100755 > --- a/t/t1407-worktree-ref-store.sh > +++ b/t/t1407-worktree-ref-store.sh > @@ -52,7 +52,7 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' ' > test_cmp expected actual > ' > > -test_expect_success 'for_each_reflog()' ' > +test_expect_success REFFILES 'for_each_reflog()' ' > echo $ZERO_OID > .git/logs/PSEUDO-MAIN && > mkdir -p .git/logs/refs/bisect && > echo $ZERO_OID > .git/logs/refs/bisect/random && Hrm, so already the first use of REFFILES has me questioning the need for it. I mean obviously this depends on ref-files in the strict sense. But it seems to me that there's two classes of those issues: A. Test where we inherently depend 100% on "reffiles", e.g. the later tests of "empty directories" in .git/refs/, presumably there's no equivalent of those in reftable. B. Tests that are really wanting to test some specific type of ref corruption, that could (or not!) happen under reftable, but just uses ref files for the setup now. I think (but am not sure) that this is the latter case. I.e. the distiniction I noted in [1]. Just skimming the context I wonder which of these can/can't happen under reftable: * We have a PSEUDO-MAIN ref * It's set to $ZERO_OID * We have a $ZERO_OID in a refs/bisect/random Etc., the test is just a setup for a call to refs_for_each_reflog(). If I'm wrong and it really is a case of "A" then presumably that has implications down to refs/iterator.c and beyond. I don't think we need to exhaustively dig into every one of these for some initial pass at getting the test suite running, but again re [1] I worry that if we over-skip we'll end up not marking the distinction between "A" and "B" above, and thus will have an end-state of losing test coverage. 1. https://lore.kernel.org/git/87wnt2th1v.fsf@evledraar.gmail.com/
On Wed, Apr 21, 2021 at 8:23 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > -test_expect_success 'for_each_reflog()' ' > > +test_expect_success REFFILES 'for_each_reflog()' ' > > echo $ZERO_OID > .git/logs/PSEUDO-MAIN && > > mkdir -p .git/logs/refs/bisect && > > echo $ZERO_OID > .git/logs/refs/bisect/random && > > Hrm, so already the first use of REFFILES has me questioning the need > for it. > > I mean obviously this depends on ref-files in the strict sense. > > .. > > * We have a PSEUDO-MAIN ref > * It's set to $ZERO_OID > * We have a $ZERO_OID in a refs/bisect/random I've added some comments about what is happening here. The $ZERO_OID is irrelevant here. The test tries to verify that a per-worktree ref only appears in output of an invocation from that worktree. It's a useful test, but this needs to be tested in an entirely different way. (looks like setting logAllRefUpdates also doesn't trigger creating reflogs for pseudorefs.)
diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh index d3fe77751122..27b57f248a94 100755 --- a/t/t1407-worktree-ref-store.sh +++ b/t/t1407-worktree-ref-store.sh @@ -52,7 +52,7 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' ' test_cmp expected actual ' -test_expect_success 'for_each_reflog()' ' +test_expect_success REFFILES 'for_each_reflog()' ' echo $ZERO_OID > .git/logs/PSEUDO-MAIN && mkdir -p .git/logs/refs/bisect && echo $ZERO_OID > .git/logs/refs/bisect/random &&