* [PATCH 0/4] Reftable test fixes 20211221
@ 2021-12-22 10:59 Han-Wen Nienhuys via GitGitGadget
2021-12-22 10:59 ` [PATCH 1/4] t5540: require REFFILES Han-Wen Nienhuys via GitGitGadget
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 10:59 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys
this is an assorted set of fixes to make some tests pass with reftable.
Han-Wen Nienhuys (4):
t5540: require REFFILES
t5550: require REFFILES
t7004: create separate tags for different tests
t7004: use "test-tool ref-store" for reflog inspection
t/t5540-http-push-webdav.sh | 6 ++++++
t/t5550-http-fetch-dumb.sh | 7 +++++++
t/t7004-tag.sh | 22 +++++++++++-----------
3 files changed, 24 insertions(+), 11 deletions(-)
base-commit: 69a9c10c95e28df457e33b3c7400b16caf2e2962
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1168%2Fhanwen%2Freftable-test-fixes-20211221-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1168/hanwen/reftable-test-fixes-20211221-v1
Pull-Request: https://github.com/git/git/pull/1168
--
gitgitgadget
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/4] t5540: require REFFILES 2021-12-22 10:59 [PATCH 0/4] Reftable test fixes 20211221 Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 10:59 ` Han-Wen Nienhuys via GitGitGadget 2021-12-24 5:14 ` Bagas Sanjaya 2021-12-22 10:59 ` [PATCH 2/4] t5550: " Han-Wen Nienhuys via GitGitGadget ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 10:59 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> The dumb HTTP protocol exposes ref storage details as part of the protocol, so it only works with the FILES refstorage backend Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t5540-http-push-webdav.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/t5540-http-push-webdav.sh b/t/t5540-http-push-webdav.sh index 8b68bb38a44..b0dbacf0b9b 100755 --- a/t/t5540-http-push-webdav.sh +++ b/t/t5540-http-push-webdav.sh @@ -18,6 +18,12 @@ then test_done fi +if test_have_prereq !REFFILES +then + skip_all='skipping test; dumb HTTP protocol not supported with reftable.' + test_done +fi + LIB_HTTPD_DAV=t . "$TEST_DIRECTORY"/lib-httpd.sh ROOT_PATH="$PWD" -- gitgitgadget ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] t5540: require REFFILES 2021-12-22 10:59 ` [PATCH 1/4] t5540: require REFFILES Han-Wen Nienhuys via GitGitGadget @ 2021-12-24 5:14 ` Bagas Sanjaya 2022-01-10 18:29 ` Han-Wen Nienhuys 0 siblings, 1 reply; 9+ messages in thread From: Bagas Sanjaya @ 2021-12-24 5:14 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget, git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys On 22/12/21 17.59, Han-Wen Nienhuys via GitGitGadget wrote: > +if test_have_prereq !REFFILES > +then > + skip_all='skipping test; dumb HTTP protocol not supported with reftable.' > + test_done > +fi > + Did you mean reftable doesn't support dumb HTTP protocol or vice versa? -- An old man doll... just what I always wanted! - Clara ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] t5540: require REFFILES 2021-12-24 5:14 ` Bagas Sanjaya @ 2022-01-10 18:29 ` Han-Wen Nienhuys 0 siblings, 0 replies; 9+ messages in thread From: Han-Wen Nienhuys @ 2022-01-10 18:29 UTC (permalink / raw) To: Bagas Sanjaya; +Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys On Fri, Dec 24, 2021 at 6:14 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote: > > On 22/12/21 17.59, Han-Wen Nienhuys via GitGitGadget wrote: > > +if test_have_prereq !REFFILES > > +then > > + skip_all='skipping test; dumb HTTP protocol not supported with reftable.' > > + test_done > > +fi > > + > > Did you mean reftable doesn't support dumb HTTP protocol or vice versa? AFAIK, the 'dumb protocol' isn't really a protocol, but is simply exposing the file system over HTTP. I suppose we could expose the reftable files over HTTP, but that means we have to rewrite the client to understand reftable files if they are served. However, I get the impression that nobody really uses the dumb protocol anymore, so is it worth trying to make this work for reftable? -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] t5550: require REFFILES 2021-12-22 10:59 [PATCH 0/4] Reftable test fixes 20211221 Han-Wen Nienhuys via GitGitGadget 2021-12-22 10:59 ` [PATCH 1/4] t5540: require REFFILES Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 10:59 ` Han-Wen Nienhuys via GitGitGadget 2021-12-22 10:59 ` [PATCH 3/4] t7004: create separate tags for different tests Han-Wen Nienhuys via GitGitGadget 2021-12-22 10:59 ` [PATCH 4/4] t7004: use "test-tool ref-store" for reflog inspection Han-Wen Nienhuys via GitGitGadget 3 siblings, 0 replies; 9+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 10:59 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> The dumb HTTP protocol exposes ref storage details as part of the protocol, so it only works with the FILES refstorage backend Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t5550-http-fetch-dumb.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 6d9142afc3b..259203926a9 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -5,6 +5,13 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh + +if test_have_prereq !REFFILES +then + skip_all='skipping test; dumb HTTP protocol not supported with reftable.' + test_done +fi + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd -- gitgitgadget ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] t7004: create separate tags for different tests 2021-12-22 10:59 [PATCH 0/4] Reftable test fixes 20211221 Han-Wen Nienhuys via GitGitGadget 2021-12-22 10:59 ` [PATCH 1/4] t5540: require REFFILES Han-Wen Nienhuys via GitGitGadget 2021-12-22 10:59 ` [PATCH 2/4] t5550: " Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 10:59 ` Han-Wen Nienhuys via GitGitGadget 2021-12-22 20:11 ` Junio C Hamano 2021-12-22 10:59 ` [PATCH 4/4] t7004: use "test-tool ref-store" for reflog inspection Han-Wen Nienhuys via GitGitGadget 3 siblings, 1 reply; 9+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 10:59 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> Reftable intentionally keeps reflog data for deleted refs. This breaks tests that delete and recreate "refs/tags/tag_with_reflog" as traces of the deletion are left in reflog. To resolve this, use a differently named ref for each test case. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t7004-tag.sh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 082be85dffc..301d1190482 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -94,10 +94,10 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' ' git log -1 \ --format="format:tag: tagging %h (%s, %cd)%n" \ --date=format:%Y-%m-%d >expected && - test_when_finished "git tag -d tag_with_reflog" && - git tag --create-reflog tag_with_reflog && - git reflog exists refs/tags/tag_with_reflog && - sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog >actual && + test_when_finished "git tag -d tag_with_reflog1" && + git tag --create-reflog tag_with_reflog1 && + git reflog exists refs/tags/tag_with_reflog1 && + sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog1 >actual && test_cmp expected actual ' @@ -105,10 +105,10 @@ test_expect_success 'annotated tag with --create-reflog has correct message' ' git log -1 \ --format="format:tag: tagging %h (%s, %cd)%n" \ --date=format:%Y-%m-%d >expected && - test_when_finished "git tag -d tag_with_reflog" && - git tag -m "annotated tag" --create-reflog tag_with_reflog && - git reflog exists refs/tags/tag_with_reflog && - sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog >actual && + test_when_finished "git tag -d tag_with_reflog2" && + git tag -m "annotated tag" --create-reflog tag_with_reflog2 && + git reflog exists refs/tags/tag_with_reflog2 && + sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog2 >actual && test_cmp expected actual ' @@ -118,10 +118,10 @@ test_expect_success '--create-reflog does not create reflog on failure' ' ' test_expect_success 'option core.logAllRefUpdates=always creates reflog' ' - test_when_finished "git tag -d tag_with_reflog" && + test_when_finished "git tag -d tag_with_reflog3" && test_config core.logAllRefUpdates always && - git tag tag_with_reflog && - git reflog exists refs/tags/tag_with_reflog + git tag tag_with_reflog3 && + git reflog exists refs/tags/tag_with_reflog3 ' test_expect_success 'listing all tags if one exists should succeed' ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] t7004: create separate tags for different tests 2021-12-22 10:59 ` [PATCH 3/4] t7004: create separate tags for different tests Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 20:11 ` Junio C Hamano 0 siblings, 0 replies; 9+ messages in thread From: Junio C Hamano @ 2021-12-22 20:11 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Han-Wen Nienhuys <hanwen@google.com> > > Reftable intentionally keeps reflog data for deleted refs. > > This breaks tests that delete and recreate "refs/tags/tag_with_reflog" as traces > of the deletion are left in reflog. To resolve this, use a differently named ref > for each test case. > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- Makes sense. These tests are *not* trying to see if a ref is recreated, its logs will reflect only the events after the last such recreation event. The only thing they care about is that the logs record the events after the ref was created the last time. Hmmm, is it? After spelling it out like the above to make sure I understand the argument, I am not so sure. If these tests are surprised to see reflog entries from the previous life, it will certainly surprise users and their tools if we suddenly start showing them without being told. Their expectation is that a deletion and recreation is a life resetting event for a ref. It feels as if we would need a new option in reflog traversal to take advantage of the new capability reftable offers (i.e. "git reflog --show-previous-life"), which, use of it is an error if the backend does not support it. An alternative is to document it, when reftable integration happens, as a limitation of files-backend that it cannot store or show the logs of the ref from its previous life. I do not care too deeply about it either way right now, because this patch allows us sidestep the design issue altogether in this test script. But we need to think about it when integrating the reftable backend into refs API. Thanks. > t/t7004-tag.sh | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh > index 082be85dffc..301d1190482 100755 > --- a/t/t7004-tag.sh > +++ b/t/t7004-tag.sh > @@ -94,10 +94,10 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' ' > git log -1 \ > --format="format:tag: tagging %h (%s, %cd)%n" \ > --date=format:%Y-%m-%d >expected && > - test_when_finished "git tag -d tag_with_reflog" && > - git tag --create-reflog tag_with_reflog && > - git reflog exists refs/tags/tag_with_reflog && > - sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog >actual && > + test_when_finished "git tag -d tag_with_reflog1" && > + git tag --create-reflog tag_with_reflog1 && > + git reflog exists refs/tags/tag_with_reflog1 && > + sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog1 >actual && > test_cmp expected actual > ' > > @@ -105,10 +105,10 @@ test_expect_success 'annotated tag with --create-reflog has correct message' ' > git log -1 \ > --format="format:tag: tagging %h (%s, %cd)%n" \ > --date=format:%Y-%m-%d >expected && > - test_when_finished "git tag -d tag_with_reflog" && > - git tag -m "annotated tag" --create-reflog tag_with_reflog && > - git reflog exists refs/tags/tag_with_reflog && > - sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog >actual && > + test_when_finished "git tag -d tag_with_reflog2" && > + git tag -m "annotated tag" --create-reflog tag_with_reflog2 && > + git reflog exists refs/tags/tag_with_reflog2 && > + sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog2 >actual && > test_cmp expected actual > ' > > @@ -118,10 +118,10 @@ test_expect_success '--create-reflog does not create reflog on failure' ' > ' > > test_expect_success 'option core.logAllRefUpdates=always creates reflog' ' > - test_when_finished "git tag -d tag_with_reflog" && > + test_when_finished "git tag -d tag_with_reflog3" && > test_config core.logAllRefUpdates always && > - git tag tag_with_reflog && > - git reflog exists refs/tags/tag_with_reflog > + git tag tag_with_reflog3 && > + git reflog exists refs/tags/tag_with_reflog3 > ' > > test_expect_success 'listing all tags if one exists should succeed' ' ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] t7004: use "test-tool ref-store" for reflog inspection 2021-12-22 10:59 [PATCH 0/4] Reftable test fixes 20211221 Han-Wen Nienhuys via GitGitGadget ` (2 preceding siblings ...) 2021-12-22 10:59 ` [PATCH 3/4] t7004: create separate tags for different tests Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 10:59 ` Han-Wen Nienhuys via GitGitGadget 2021-12-22 20:17 ` Junio C Hamano 3 siblings, 1 reply; 9+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 10:59 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> This makes the test work with reftable. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- t/t7004-tag.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 301d1190482..bc8a221b2c7 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -97,7 +97,7 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' ' test_when_finished "git tag -d tag_with_reflog1" && git tag --create-reflog tag_with_reflog1 && git reflog exists refs/tags/tag_with_reflog1 && - sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog1 >actual && + test-tool ref-store main for-each-reflog-ent refs/tags/tag_with_reflog1 | sed -e "s/^.* //" >actual && test_cmp expected actual ' @@ -108,7 +108,7 @@ test_expect_success 'annotated tag with --create-reflog has correct message' ' test_when_finished "git tag -d tag_with_reflog2" && git tag -m "annotated tag" --create-reflog tag_with_reflog2 && git reflog exists refs/tags/tag_with_reflog2 && - sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog2 >actual && + test-tool ref-store main for-each-reflog-ent refs/tags/tag_with_reflog2 | sed -e "s/^.* //" >actual && test_cmp expected actual ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] t7004: use "test-tool ref-store" for reflog inspection 2021-12-22 10:59 ` [PATCH 4/4] t7004: use "test-tool ref-store" for reflog inspection Han-Wen Nienhuys via GitGitGadget @ 2021-12-22 20:17 ` Junio C Hamano 0 siblings, 0 replies; 9+ messages in thread From: Junio C Hamano @ 2021-12-22 20:17 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Han-Wen Nienhuys <hanwen@google.com> > > This makes the test work with reftable. > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > ... > - sed -e "s/^.* //" .git/logs/refs/tags/tag_with_reflog2 >actual && > + test-tool ref-store main for-each-reflog-ent refs/tags/tag_with_reflog2 | sed -e "s/^.* //" >actual && > test_cmp expected actual > ' Yup, this makes perfect sense. Purists may be bothered by having anything _we_ write, like "test-tool" on the upstream of a pipe to hide its exit code, though. After applying these four patches on top of 'master', I still see some mention of logs/refs in the t/ directory. Some of them may be a low-hanging fruit other people can take a look at and convert to the test-helper calls to help us move the reftable topic forward without stepping on your toes, hopefully? Thanks. t/t0060-path-utils.sh:test_git_path GIT_COMMON_DIR=bar logs/refs/bisect/foo .git/logs/refs/bisect/foo t/t0060-path-utils.sh:test_git_path GIT_COMMON_DIR=bar logs/refs bar/logs/refs t/t0060-path-utils.sh:test_git_path GIT_COMMON_DIR=bar logs/refs/ bar/logs/refs/ t/t0060-path-utils.sh:test_git_path GIT_COMMON_DIR=bar logs/refs/bisec/foo bar/logs/refs/bisec/foo t/t0060-path-utils.sh:test_git_path GIT_COMMON_DIR=bar logs/refs/bisec bar/logs/refs/bisec t/t0060-path-utils.sh:test_git_path GIT_COMMON_DIR=bar logs/refs/bisectfoo bar/logs/refs/bisectfoo t/t0060-path-utils.sh:test_git_path GIT_COMMON_DIR=bar logs/refs/heads/main bar/logs/refs/heads/main t/t1301-shared-repo.sh: actual="$(ls -l .git/logs/refs/heads/main)" && t/t1301-shared-repo.sh: echo Ooops, .git/logs/refs/heads/main is not 066x [$actual] t/t1400-update-ref.sh:rm -f .git/logs/refs/heads/main t/t1400-update-ref.sh: test_path_is_file .git/logs/refs/heads/d1/d2/r1 && t/t1400-update-ref.sh: test_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 && t/t1400-update-ref.sh: test_path_is_file .git/logs/refs/heads/d1/r2 t/t1400-update-ref.sh: test_path_is_file .git/logs/refs/heads/e1/e2/r1 && t/t1400-update-ref.sh: test_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 && t/t1400-update-ref.sh: test_path_is_file .git/logs/refs/heads/e1/r2 && t/t1407-worktree-ref-store.sh: mkdir -p .git/logs/refs/bisect && t/t1407-worktree-ref-store.sh: echo $ZERO_OID > .git/logs/refs/bisect/random && t/t1407-worktree-ref-store.sh: mkdir -p .git/worktrees/wt/logs/refs/bisect && t/t1407-worktree-ref-store.sh: echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random && t/t1410-reflog.sh: # now logs/refs/heads/one is a stale directory, but t/t1410-reflog.sh: done >.git/logs/refs/heads/reflogskip && t/t3200-branch.sh: test_path_is_file .git/logs/refs/heads/d/e/f && t/t3200-branch.sh: test_cmp expect .git/logs/refs/heads/d/e/f t/t3200-branch.sh: mv .git/logs/refs/heads/u real-u && t/t3200-branch.sh: ln -s real-u .git/logs/refs/heads/u && t/t3200-branch.sh: test_path_is_file .git/logs/refs/heads/g/h/i && t/t3200-branch.sh: test_cmp expect .git/logs/refs/heads/g/h/i ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-01-10 18:29 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-22 10:59 [PATCH 0/4] Reftable test fixes 20211221 Han-Wen Nienhuys via GitGitGadget 2021-12-22 10:59 ` [PATCH 1/4] t5540: require REFFILES Han-Wen Nienhuys via GitGitGadget 2021-12-24 5:14 ` Bagas Sanjaya 2022-01-10 18:29 ` Han-Wen Nienhuys 2021-12-22 10:59 ` [PATCH 2/4] t5550: " Han-Wen Nienhuys via GitGitGadget 2021-12-22 10:59 ` [PATCH 3/4] t7004: create separate tags for different tests Han-Wen Nienhuys via GitGitGadget 2021-12-22 20:11 ` Junio C Hamano 2021-12-22 10:59 ` [PATCH 4/4] t7004: use "test-tool ref-store" for reflog inspection Han-Wen Nienhuys via GitGitGadget 2021-12-22 20:17 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).