| Message ID | pull.1008.v2.git.git.1619519903.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Prepare tests for reftable backend | expand |
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote: > Rewrites some tests to avoid direct filesystem access. > > Introduces the test prereq REFFILES to mark other tests that depend on > specifics of the files ref backend. > > changes in v2 (relative to v1 from Apr 19) > > * t9300: use ref-store test-helper to read toplevel tag. > * t1401: use $TAR for restoring. > * t1407: clarify what test is doing. > * t1417: "$TAR" rather than tar > * t1415: clarify test objective. > * t7003: formatting. > * README entry for REFFILES. > > cc: SZEDER Gábor szeder.dev@gmail.com cc: Ævar Arnfjörð Bjarmason > avarab@gmail.com cc: Han-Wen Nienhuys hanwen@google.com > > cc: Han-Wen Nienhuys hanwen@google.com > > Han-Wen Nienhuys (21): > t4202: split testcase for invalid HEAD symref and HEAD hash > t/helper/ref-store: initialize oid in resolve-ref > t9300: check ref existence using test-helper rather than a file system > check > t5601: read HEAD using rev-parse > t1401-symbolic-ref: avoid direct filesystem access > t1413: use tar to save and restore entire .git directory > t1301: fix typo in error message > t5000: reformat indentation to the latest fashion > t5000: inspect HEAD using git-rev-parse > t7003: use rev-parse rather than FS inspection > t5304: restyle: trim empty lines, drop ':' before > > t5304: use "reflog expire --all" to clear the reflog > test-lib: provide test prereq REFFILES > t1407: require REFFILES for for_each_reflog test > t1414: mark corruption test with REFFILES > t2017: mark --orphan/logAllRefUpdates=false test as REFFILES > t1404: mark tests that muck with .git directly as REFFILES. > t7900: mark pack-refs tests as REFFILES > t7003: check reflog existence only for REFFILES > t4202: mark bogus head hash test with REFFILES > t1415: set REFFILES for test specific to storage format > > t/README | 6 ++ > t/helper/test-ref-store.c | 2 +- > t/t1301-shared-repo.sh | 2 +- > t/t1401-symbolic-ref.sh | 34 +++++----- > t/t1404-update-ref-errors.sh | 30 ++++----- > t/t1407-worktree-ref-store.sh | 9 ++- > t/t1413-reflog-detach.sh | 5 +- > t/t1414-reflog-walk.sh | 2 +- > t/t1415-worktree-refs.sh | 5 +- > t/t2017-checkout-orphan.sh | 2 +- > t/t4202-log.sh | 10 ++- > t/t5000-tar-tree.sh | 113 +++++++++++++++++----------------- > t/t5304-prune.sh | 83 ++++++++----------------- > t/t5601-clone.sh | 3 +- > t/t7003-filter-branch.sh | 7 ++- > t/t7900-maintenance.sh | 2 +- > t/t9300-fast-import.sh | 2 +- > t/test-lib.sh | 2 + > 18 files changed, 156 insertions(+), 163 deletions(-) > > > base-commit: 311531c9de557d25ac087c1637818bd2aad6eb3a > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1008%2Fhanwen%2Freffiles-v2 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1008/hanwen/reffiles-v2 > Pull-Request: https://github.com/git/git/pull/1008 > > Range-diff vs v1: > > 1: 91ef012cbcc9 ! 1: 8ad4a35cda70 t4202: split testcase for invalid HEAD symref and HEAD hash > @@ t/t4202-log.sh: test_expect_success 'log --graph --no-walk is forbidden' ' > + test_i18ngrep broken stderr' > + > +test_expect_success 'log diagnoses bogus HEAD symref' ' > ++ rm -rf empty && > + git init empty && > + git --git-dir empty/.git symbolic-ref HEAD refs/heads/invalid.lock && > test_must_fail git -C empty log 2>stderr && > -: ------------ > 2: e6222944a3eb t/helper/ref-store: initialize oid in resolve-ref > 2: ccc26a8950be ! 3: c5855b0caa73 t9300: check ref existence using git-rev-parse rather than FS check > @@ Metadata > Author: Han-Wen Nienhuys <hanwen@google.com> > > ## Commit message ## > - t9300: check ref existence using git-rev-parse rather than FS check > + t9300: check ref existence using test-helper rather than a file system check > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > > @@ t/t9300-fast-import.sh: test_expect_success 'B: accept branch name "TEMP_TAG"' ' > git prune" && > git fast-import <input && > - test -f .git/TEMP_TAG && > -+ git rev-parse TEMP_TAG && > ++ test $(test-tool ref-store main resolve-ref TEMP_TAG 0 | cut -f1 -d " " ) != "$ZERO_OID" && > test $(git rev-parse main) = $(git rev-parse TEMP_TAG^) > ' > > 3: 47b5ec56a383 = 4: 369c968ab837 t5601: read HEAD using rev-parse > 4: 53cf1069552b ! 5: 248d9ffe7927 t1401-symbolic-ref: avoid direct filesystem access > @@ Commit message > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > > ## t/t1401-symbolic-ref.sh ## > -@@ t/t1401-symbolic-ref.sh: test_description='basic symbolic-ref tests' > - # the git repo, meaning that further tests will operate on > - # the surrounding git repo instead of the trash directory. > - reset_to_sane() { > -- echo ref: refs/heads/foo >.git/HEAD > -+ git --git-dir .git symbolic-ref HEAD refs/heads/foo > - } > +@@ > + test_description='basic symbolic-ref tests' > + . ./test-lib.sh > > +-# If the tests munging HEAD fail, they can break detection of > +-# the git repo, meaning that further tests will operate on > +-# the surrounding git repo instead of the trash directory. > +-reset_to_sane() { > +- echo ref: refs/heads/foo >.git/HEAD > +-} > +- > -test_expect_success 'symbolic-ref writes HEAD' ' > -- git symbolic-ref HEAD refs/heads/foo && > ++test_expect_success 'setup' ' > + git symbolic-ref HEAD refs/heads/foo && > - echo ref: refs/heads/foo >expect && > - test_cmp expect .git/HEAD > --' > -- > ++ test_commit file && > ++ "$TAR" cf .git.tar .git/ > + ' > + > -test_expect_success 'symbolic-ref reads HEAD' ' > - echo refs/heads/foo >expect && > -- git symbolic-ref HEAD >actual && > ++reset_to_sane() { > ++ rm -rf .git && > ++ "$TAR" xf .git.tar > ++} > ++ > +test_expect_success 'symbolic-ref read/write roundtrip' ' > + git symbolic-ref HEAD refs/heads/read-write-roundtrip && > -+ echo refs/heads/read-write-roundtrip > expect && > -+ git symbolic-ref HEAD > actual && > ++ echo refs/heads/read-write-roundtrip >expect && > + git symbolic-ref HEAD >actual && > test_cmp expect actual > ' > +@@ t/t1401-symbolic-ref.sh: test_expect_success 'symbolic-ref reads HEAD' ' > + test_expect_success 'symbolic-ref refuses non-ref for HEAD' ' > + test_must_fail git symbolic-ref HEAD foo > + ' > ++ > + reset_to_sane > + > + test_expect_success 'symbolic-ref refuses bare sha1' ' > +- echo content >file && git add file && git commit -m one && > + test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD) > + ' > ++ > + reset_to_sane > > + test_expect_success 'HEAD cannot be removed' ' > @@ t/t1401-symbolic-ref.sh: reset_to_sane > test_expect_success 'symbolic-ref can be deleted' ' > git symbolic-ref NOTHEAD refs/heads/foo && > @@ t/t1401-symbolic-ref.sh: reset_to_sane > - test_path_is_file .git/refs/heads/foo && > - test_path_is_missing .git/NOTHEAD > + git rev-parse refs/heads/foo && > -+ ! git symbolic-ref NOTHEAD > ++ test_must_fail git symbolic-ref NOTHEAD > ' > reset_to_sane > > @@ t/t1401-symbolic-ref.sh: reset_to_sane > git symbolic-ref -d NOTHEAD && > - test_path_is_missing .git/refs/heads/missing && > - test_path_is_missing .git/NOTHEAD > -+ ! git rev-parse refs/heads/missing && > -+ ! git symbolic-ref NOTHEAD > ++ test_must_fail git rev-parse refs/heads/missing && > ++ test_must_fail git symbolic-ref NOTHEAD > ' > reset_to_sane > > 5: 223583594c00 ! 6: e4e8fc1d4b4f t1413: use tar to save and restore entire .git directory > @@ t/t1413-reflog-detach.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > reset_state () { > - git checkout main && > - cp saved_reflog .git/logs/HEAD > -+ rm -rf .git && tar -xf .git-saved.tar > ++ rm -rf .git && "$TAR" xf .git-saved.tar > } > > test_expect_success setup ' > @@ t/t1413-reflog-detach.sh: test_expect_success setup ' > test_tick && > git commit --allow-empty -m second && > - cat .git/logs/HEAD >saved_reflog > -+ tar -cf .git-saved.tar .git > ++ "$TAR" cf .git-saved.tar .git > ' > > test_expect_success baseline ' > 6: 70da8f5631d0 = 7: 89cc215c6014 t1301: fix typo in error message > -: ------------ > 8: e67b90847c4e t5000: reformat indentation to the latest fashion > 7: 79843c0d5727 ! 9: d6072a70ae7d t5000: inspect HEAD using git-rev-parse > @@ Commit message > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > > ## t/t5000-tar-tree.sh ## > -@@ t/t5000-tar-tree.sh: test_expect_success \ > - test_cmp expected.mtime b.mtime' > +@@ t/t5000-tar-tree.sh: test_expect_success 'validate file modification time' ' > + test_cmp expected.mtime b.mtime > + ' > > - test_expect_success \ > +-test_expect_success \ > - 'git get-tar-commit-id' \ > - 'git get-tar-commit-id <b.tar >b.commitid && > - test_cmp .git/$(git symbolic-ref HEAD) b.commitid' > -+ 'git get-tar-commit-id' \ > -+ 'git get-tar-commit-id <b.tar >actual && > -+ git rev-parse HEAD > expect && > -+ test_cmp expect actual' > ++test_expect_success 'git get-tar-commit-id' ' > ++ git get-tar-commit-id <b.tar >actual && > ++ git rev-parse HEAD >expect && > ++ test_cmp expect actual > ++' > > test_expect_success 'git archive with --output, override inferred format' ' > git archive --format=tar --output=d4.zip HEAD && > 8: dbb81b5b89d8 = 10: 4bf1bf16bca3 t7003: use rev-parse rather than FS inspection > -: ------------ > 11: 6f15c15573af t5304: restyle: trim empty lines, drop ':' before > > 9: ba575839e422 ! 12: d8e80d83b6f8 t5304: use "reflog expire --all" to clear the reflog > @@ Commit message > > ## t/t5304-prune.sh ## > @@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' ' > - ' > > test_expect_success 'prune: prune unreachable heads' ' > -- > git config core.logAllRefUpdates false && > - mv .git/logs .git/logs.old && > - : > file2 && > +- : > file2 && > ++ >file2 && > git add file2 && > git commit -m temporary && > tmp_head=$(git rev-list -1 HEAD) && > @@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' ' > + git reflog expire --all && > git prune && > test_must_fail git reset $tmp_head -- > -- > ' > - > +@@ t/t5304-prune.sh: test_expect_success 'prune: prune unreachable heads' ' > test_expect_success 'prune: do not prune detached HEAD with no reflog' ' > - > git checkout --detach --quiet && > git commit --allow-empty -m "detached commit" && > - # verify that there is no reflogs > @@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' ' > + git reflog expire --all && > git prune -n >prune_actual && > test_must_be_empty prune_actual > - > -@@ t/t5304-prune.sh: test_expect_success 'prune: prune former HEAD after checking out branch' ' > - > + ' > +@@ t/t5304-prune.sh: test_expect_success 'prune: do not prune detached HEAD with no reflog' ' > + test_expect_success 'prune: prune former HEAD after checking out branch' ' > head_oid=$(git rev-parse HEAD) && > git checkout --quiet main && > + git reflog expire --all && > git prune -v >prune_actual && > grep "$head_oid" prune_actual > - > + ' > 10: 3d3b733c3127 ! 13: 180847f4db14 test-lib: provide test prereq REFFILES > @@ Commit message > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > > + ## t/README ## > +@@ t/README: use these, and "test_set_prereq" for how to define your own. > + > + Git wasn't compiled with NO_PTHREADS=YesPlease. > + > ++ - REFFILES > ++ > ++ Test is specific to packed/loose ref storage, and should be > ++ disabled for other ref storage backends > ++ > ++ > + Tips for Writing Tests > + ---------------------- > + > + > ## t/test-lib.sh ## > @@ t/test-lib.sh: parisc* | hppa*) > ;; > 11: dd1f6969c28d ! 14: f3307b62bfd7 t1407: require REFFILES for for_each_reflog test > @@ Metadata > ## Commit message ## > t1407: require REFFILES for for_each_reflog test > > - It tries to setup a reflog by catting to .git/logs/ > + Add extensive comment why this test needs a REFFILES annotation. > + > + I tried forcing universal reflog creation with core.logAllRefUpdates=true, but > + that apparently also doesn't cause reflogs to be created for pseudorefs > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > > @@ t/t1407-worktree-ref-store.sh: test_expect_success 'create_symref(FOO, refs/head > ' > > -test_expect_success 'for_each_reflog()' ' > ++# Some refs (refs/bisect/*, pseudorefs) are kept per worktree, so they should > ++# only appear in the for-each-reflog output if it is called from the correct > ++# worktree, which is exercised in this test. This test is poorly written (and > ++# therefore marked REFFILES) for mulitple reasons: 1) it creates invalidly > ++# formatted log entres. 2) it uses direct FS access for creating the reflogs. 3) > ++# PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is > ++# not testing a realistic scenario. > +test_expect_success REFFILES 'for_each_reflog()' ' > echo $ZERO_OID > .git/logs/PSEUDO-MAIN && > mkdir -p .git/logs/refs/bisect && > 12: 86951eb39cb6 = 15: 0d3b18cd3542 t1414: mark corruption test with REFFILES > 13: 1ce545043846 = 16: b64e3e7ade15 t2017: mark --orphan/logAllRefUpdates=false test as REFFILES > 14: a3abc4f70e7d ! 17: fcc2b714dd50 t1404: mark tests that muck with .git directly as REFFILES. > @@ Metadata > ## Commit message ## > t1404: mark tests that muck with .git directly as REFFILES. > > + The packed/loose ref storage is an overlay combination of packed-refs (refs and > + tags in a single file) and one-file-per-ref. This creates all kinds of edge > + cases related to directory/file conflicts, (non-)empty directories, and the > + locking scheme, none of which applies to reftable. > + > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > > ## t/t1404-update-ref-errors.sh ## > 15: 2b3021c4ba62 ! 18: ff3b67c84c41 t7900: mark pack-refs tests as REFFILES > @@ Metadata > ## Commit message ## > t7900: mark pack-refs tests as REFFILES > > + Reftable automatically compacts tables on writes. The functionality of > + incremental compaction is unittested in reftable/stack_test.c > + (test_reftable_stack_auto_compaction) > + > + In addition, pack-refs triggers a full compaction of the entire stack. This is > + exercised in t0031-reftable.sh. > + > + Given that git-maintenance simply calls out git-pack-refs, it seems superfluous > + to test this further for reftable. > + > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > > ## t/t7900-maintenance.sh ## > 16: a5b9439192db ! 19: 24dcf05d8fa6 t7003: check reflog existence only for REFFILES > @@ t/t7003-filter-branch.sh: test_expect_success '--prune-empty is able to prune en > git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire && > test_must_fail git rev-parse refs/heads/prune-entire && > - test_must_fail git reflog exists refs/heads/prune-entire > -+ if test_have_prereq REFFILES ; then > ++ if test_have_prereq REFFILES > ++ then > + test_must_fail git reflog exists refs/heads/prune-entire > + fi > ' > 17: a2cce772d44f = 20: a33cdfda74ff t4202: mark bogus head hash test with REFFILES > 18: 0665edb1308b ! 21: d7e5de8dba46 t1415: set REFFILES for test specific to storage format > @@ Metadata > ## Commit message ## > t1415: set REFFILES for test specific to storage format > > + Packing refs (and therefore checking that certain refs are not packed) is a > + property of the packed/loose ref storage. Add a comment to explain what the test > + checks. > + > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > > ## t/t1415-worktree-refs.sh ## > @@ t/t1415-worktree-refs.sh: test_expect_success 'setup' ' > ' > > -test_expect_success 'refs/worktree must not be packed' ' > ++# The 'packed-refs' files is stored directly in .git/. This means it is global > ++# to the repository, and can only contain refs that are shared across all > ++# worktrees. > +test_expect_success REFFILES 'refs/worktree must not be packed' ' > git pack-refs --all && > test_path_is_missing .git/refs/tags/wt1 && I looked this all over and left some nits, suspect/probably/definite bug comments on specific patches, the ones with no comments from me LGTM. I intentionally didn't review my own earlier feedback on v1 to look at this with fresh eyes, I'd forgetten what points I raised, aside from the general "let's not skip tests but test the new behavior" mantra I think I either mentioned there or in related discussions.
On Thu, May 20, 2021 at 6:35 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > I looked this all over and left some nits, suspect/probably/definite bug > comments on specific patches, the ones with no comments from me LGTM. Awesome. Thanks for reviewing. I'll try to go over your comments this week. > I intentionally didn't review my own earlier feedback on v1 to look at > this with fresh eyes, I'd forgetten what points I raised, aside from the > general "let's not skip tests but test the new behavior" mantra I think > I either mentioned there or in related discussions. "test new behavior" is troublesome, because it requires merging the reftable support first, which in itself is a tough job, and predicated on getting reviews for that code. This is why I split this series off, because it can be merged early without impacting coverage of the existing loose/packed backend. How about I document more clearly, for each test marked REFFILES, what is going on, and what should be done for coverage with reftable? We can go over it as part of the reftable series. "REFFILES" will be an easy term to grep for.
On Wed, May 26 2021, Han-Wen Nienhuys wrote: > On Thu, May 20, 2021 at 6:35 PM Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: > >> >> I looked this all over and left some nits, suspect/probably/definite bug >> comments on specific patches, the ones with no comments from me LGTM. > > Awesome. Thanks for reviewing. I'll try to go over your comments this week. > >> I intentionally didn't review my own earlier feedback on v1 to look at >> this with fresh eyes, I'd forgetten what points I raised, aside from the >> general "let's not skip tests but test the new behavior" mantra I think >> I either mentioned there or in related discussions. > > > "test new behavior" is troublesome, because it requires merging the > reftable support first, which in itself is a tough job, and predicated > on getting reviews for that code. This is why I split this series off, > because it can be merged early without impacting coverage of the > existing loose/packed backend. > > How about I document more clearly, for each test marked REFFILES, what > is going on, and what should be done for coverage with reftable? We > can go over it as part of the reftable series. "REFFILES" will be an > easy term to grep for. Indeed, as I pointed out in some (particularly later patches) v2 commentary there's that chicken & egg problem. Having re-skimmed my v2 reviews I think these would help: A. Just leaning on the side of splitting up tests, e.g in my comment on 20/21[1], there's a test with file backend specific logic there, but also a general assertion. Should just be two tests. B. My suggestion in 15/21[2] to split up maybe make these two prerequisites, i.e. REFFILES and REFTABLE. One one hand it's a bit silly to have two prerequisites required for the one reftable mode we don't have yet, and as long as we just have two backends it'll be an informed guess at best. But I'm leaning on the side of it being a good idea, to clearly document those cases that are really files-backend.c specific, such as low-level test to check that a .git/HEAD exists along with a "git rev-parse HEAD" for that backend. And then REFTABLE (and !REFTABLE) to test/skip things that are specific to the behavior of reftable, but not in a way that has to do with the "ref files" v.s. "a ref database" distinction. E.g. a maintenance command calling pack-refs is skipped under REFTABLE, but one could imagine us e.g. having a PostgreSQL backend where that would be mapped to "VACUUM refs" or whatever. I.e. it's testing/skipping based on reftable-specific features. Even if strictly speaking we'd make no use of the distinction now in that you could losslessly map REFFILES = !REFTABLE and REFTABLE = !REFFILES. Or maybe it's useless and we'd want to eventually do a second pass over both REFFILES and REFTABLE to see if we need to change them to positively assert the behavior of the "other" backend in both cases. Just a thought... 1. https://lore.kernel.org/git/87eee1pfzk.fsf@evledraar.gmail.com/ 2. http://lore.kernel.org/git/87pmxlpgb1.fsf@evledraar.gmail.com
On Wed, May 26, 2021 at 12:07 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > >> I looked this all over and left some nits, suspect/probably/definite bug > >> comments on specific patches, the ones with no comments from me LGTM. > > > > Awesome. Thanks for reviewing. I'll try to go over your comments this week. I'm sending out the series shortly, with a Reviewed-By footer added to the commits. Jun, is this OK to schedule for 'next' ? > >> I intentionally didn't review my own earlier feedback on v1 to look at > >> this with fresh eyes, I'd forgetten what points I raised, aside from the > >> general "let's not skip tests but test the new behavior" mantra I think > >> I either mentioned there or in related discussions. > > > > "test new behavior" is troublesome, because it requires merging the > > reftable support first, which in itself is a tough job, and predicated > > on getting reviews for that code. This is why I split this series off, > > because it can be merged early without impacting coverage of the > > existing loose/packed backend. > > > > How about I document more clearly, for each test marked REFFILES, what > > is going on, and what should be done for coverage with reftable? We > > can go over it as part of the reftable series. "REFFILES" will be an > > easy term to grep for. > > Indeed, as I pointed out in some (particularly later patches) v2 > commentary there's that chicken & egg problem. > > Having re-skimmed my v2 reviews I think these would help: > > A. Just leaning on the side of splitting up tests, e.g in my comment on > 20/21[1], there's a test with file backend specific logic there, but > also a general assertion. Should just be two tests. I split a few tests. > B. My suggestion in 15/21[2] to split up maybe make these two > prerequisites, i.e. REFFILES and REFTABLE. Until reftable is actually merged that will introduce a lot of dead code in the tests. Besides requiring time for me to write that code, it also introduces extra reviewer effort. I think that is best done as part of the reftable series, when it is close to being merged. > One one hand it's a bit silly to have two prerequisites required for > the one reftable mode we don't have yet, and as long as we just have > two backends it'll be an informed guess at best. > > But I'm leaning on the side of it being a good idea, to clearly > document those cases that are really files-backend.c specific, such > as low-level test to check that a .git/HEAD exists along with a "git > rev-parse HEAD" for that backend. > > And then REFTABLE (and !REFTABLE) to test/skip things that are > specific to the behavior of reftable, but not in a way that has to > do with the "ref files" v.s. "a ref database" distinction. E.g. a > maintenance command calling pack-refs is skipped under REFTABLE, but > one could imagine us e.g. having a PostgreSQL backend where that Let's first see the actual PostgreSQL backend materialize before making investments to support it.
Rewrites some tests to avoid direct filesystem access. Introduces the test prereq REFFILES to mark other tests that depend on specifics of the files ref backend. changes in v2 (relative to v1 from Apr 19) * t9300: use ref-store test-helper to read toplevel tag. * t1401: use $TAR for restoring. * t1407: clarify what test is doing. * t1417: "$TAR" rather than tar * t1415: clarify test objective. * t7003: formatting. * README entry for REFFILES. cc: SZEDER Gábor szeder.dev@gmail.com cc: Ævar Arnfjörð Bjarmason avarab@gmail.com cc: Han-Wen Nienhuys hanwen@google.com cc: Han-Wen Nienhuys hanwen@google.com Han-Wen Nienhuys (21): t4202: split testcase for invalid HEAD symref and HEAD hash t/helper/ref-store: initialize oid in resolve-ref t9300: check ref existence using test-helper rather than a file system check t5601: read HEAD using rev-parse t1401-symbolic-ref: avoid direct filesystem access t1413: use tar to save and restore entire .git directory t1301: fix typo in error message t5000: reformat indentation to the latest fashion t5000: inspect HEAD using git-rev-parse t7003: use rev-parse rather than FS inspection t5304: restyle: trim empty lines, drop ':' before > t5304: use "reflog expire --all" to clear the reflog test-lib: provide test prereq REFFILES t1407: require REFFILES for for_each_reflog test t1414: mark corruption test with REFFILES t2017: mark --orphan/logAllRefUpdates=false test as REFFILES t1404: mark tests that muck with .git directly as REFFILES. t7900: mark pack-refs tests as REFFILES t7003: check reflog existence only for REFFILES t4202: mark bogus head hash test with REFFILES t1415: set REFFILES for test specific to storage format t/README | 6 ++ t/helper/test-ref-store.c | 2 +- t/t1301-shared-repo.sh | 2 +- t/t1401-symbolic-ref.sh | 34 +++++----- t/t1404-update-ref-errors.sh | 30 ++++----- t/t1407-worktree-ref-store.sh | 9 ++- t/t1413-reflog-detach.sh | 5 +- t/t1414-reflog-walk.sh | 2 +- t/t1415-worktree-refs.sh | 5 +- t/t2017-checkout-orphan.sh | 2 +- t/t4202-log.sh | 10 ++- t/t5000-tar-tree.sh | 113 +++++++++++++++++----------------- t/t5304-prune.sh | 83 ++++++++----------------- t/t5601-clone.sh | 3 +- t/t7003-filter-branch.sh | 7 ++- t/t7900-maintenance.sh | 2 +- t/t9300-fast-import.sh | 2 +- t/test-lib.sh | 2 + 18 files changed, 156 insertions(+), 163 deletions(-) base-commit: 311531c9de557d25ac087c1637818bd2aad6eb3a Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1008%2Fhanwen%2Freffiles-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1008/hanwen/reffiles-v2 Pull-Request: https://github.com/git/git/pull/1008 Range-diff vs v1: 1: 91ef012cbcc9 ! 1: 8ad4a35cda70 t4202: split testcase for invalid HEAD symref and HEAD hash @@ t/t4202-log.sh: test_expect_success 'log --graph --no-walk is forbidden' ' + test_i18ngrep broken stderr' + +test_expect_success 'log diagnoses bogus HEAD symref' ' ++ rm -rf empty && + git init empty && + git --git-dir empty/.git symbolic-ref HEAD refs/heads/invalid.lock && test_must_fail git -C empty log 2>stderr && -: ------------ > 2: e6222944a3eb t/helper/ref-store: initialize oid in resolve-ref 2: ccc26a8950be ! 3: c5855b0caa73 t9300: check ref existence using git-rev-parse rather than FS check @@ Metadata Author: Han-Wen Nienhuys <hanwen@google.com> ## Commit message ## - t9300: check ref existence using git-rev-parse rather than FS check + t9300: check ref existence using test-helper rather than a file system check Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> @@ t/t9300-fast-import.sh: test_expect_success 'B: accept branch name "TEMP_TAG"' ' git prune" && git fast-import <input && - test -f .git/TEMP_TAG && -+ git rev-parse TEMP_TAG && ++ test $(test-tool ref-store main resolve-ref TEMP_TAG 0 | cut -f1 -d " " ) != "$ZERO_OID" && test $(git rev-parse main) = $(git rev-parse TEMP_TAG^) ' 3: 47b5ec56a383 = 4: 369c968ab837 t5601: read HEAD using rev-parse 4: 53cf1069552b ! 5: 248d9ffe7927 t1401-symbolic-ref: avoid direct filesystem access @@ Commit message Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> ## t/t1401-symbolic-ref.sh ## -@@ t/t1401-symbolic-ref.sh: test_description='basic symbolic-ref tests' - # the git repo, meaning that further tests will operate on - # the surrounding git repo instead of the trash directory. - reset_to_sane() { -- echo ref: refs/heads/foo >.git/HEAD -+ git --git-dir .git symbolic-ref HEAD refs/heads/foo - } +@@ + test_description='basic symbolic-ref tests' + . ./test-lib.sh +-# If the tests munging HEAD fail, they can break detection of +-# the git repo, meaning that further tests will operate on +-# the surrounding git repo instead of the trash directory. +-reset_to_sane() { +- echo ref: refs/heads/foo >.git/HEAD +-} +- -test_expect_success 'symbolic-ref writes HEAD' ' -- git symbolic-ref HEAD refs/heads/foo && ++test_expect_success 'setup' ' + git symbolic-ref HEAD refs/heads/foo && - echo ref: refs/heads/foo >expect && - test_cmp expect .git/HEAD --' -- ++ test_commit file && ++ "$TAR" cf .git.tar .git/ + ' + -test_expect_success 'symbolic-ref reads HEAD' ' - echo refs/heads/foo >expect && -- git symbolic-ref HEAD >actual && ++reset_to_sane() { ++ rm -rf .git && ++ "$TAR" xf .git.tar ++} ++ +test_expect_success 'symbolic-ref read/write roundtrip' ' + git symbolic-ref HEAD refs/heads/read-write-roundtrip && -+ echo refs/heads/read-write-roundtrip > expect && -+ git symbolic-ref HEAD > actual && ++ echo refs/heads/read-write-roundtrip >expect && + git symbolic-ref HEAD >actual && test_cmp expect actual ' +@@ t/t1401-symbolic-ref.sh: test_expect_success 'symbolic-ref reads HEAD' ' + test_expect_success 'symbolic-ref refuses non-ref for HEAD' ' + test_must_fail git symbolic-ref HEAD foo + ' ++ + reset_to_sane + + test_expect_success 'symbolic-ref refuses bare sha1' ' +- echo content >file && git add file && git commit -m one && + test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD) + ' ++ + reset_to_sane + test_expect_success 'HEAD cannot be removed' ' @@ t/t1401-symbolic-ref.sh: reset_to_sane test_expect_success 'symbolic-ref can be deleted' ' git symbolic-ref NOTHEAD refs/heads/foo && @@ t/t1401-symbolic-ref.sh: reset_to_sane - test_path_is_file .git/refs/heads/foo && - test_path_is_missing .git/NOTHEAD + git rev-parse refs/heads/foo && -+ ! git symbolic-ref NOTHEAD ++ test_must_fail git symbolic-ref NOTHEAD ' reset_to_sane @@ t/t1401-symbolic-ref.sh: reset_to_sane git symbolic-ref -d NOTHEAD && - test_path_is_missing .git/refs/heads/missing && - test_path_is_missing .git/NOTHEAD -+ ! git rev-parse refs/heads/missing && -+ ! git symbolic-ref NOTHEAD ++ test_must_fail git rev-parse refs/heads/missing && ++ test_must_fail git symbolic-ref NOTHEAD ' reset_to_sane 5: 223583594c00 ! 6: e4e8fc1d4b4f t1413: use tar to save and restore entire .git directory @@ t/t1413-reflog-detach.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME reset_state () { - git checkout main && - cp saved_reflog .git/logs/HEAD -+ rm -rf .git && tar -xf .git-saved.tar ++ rm -rf .git && "$TAR" xf .git-saved.tar } test_expect_success setup ' @@ t/t1413-reflog-detach.sh: test_expect_success setup ' test_tick && git commit --allow-empty -m second && - cat .git/logs/HEAD >saved_reflog -+ tar -cf .git-saved.tar .git ++ "$TAR" cf .git-saved.tar .git ' test_expect_success baseline ' 6: 70da8f5631d0 = 7: 89cc215c6014 t1301: fix typo in error message -: ------------ > 8: e67b90847c4e t5000: reformat indentation to the latest fashion 7: 79843c0d5727 ! 9: d6072a70ae7d t5000: inspect HEAD using git-rev-parse @@ Commit message Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> ## t/t5000-tar-tree.sh ## -@@ t/t5000-tar-tree.sh: test_expect_success \ - test_cmp expected.mtime b.mtime' +@@ t/t5000-tar-tree.sh: test_expect_success 'validate file modification time' ' + test_cmp expected.mtime b.mtime + ' - test_expect_success \ +-test_expect_success \ - 'git get-tar-commit-id' \ - 'git get-tar-commit-id <b.tar >b.commitid && - test_cmp .git/$(git symbolic-ref HEAD) b.commitid' -+ 'git get-tar-commit-id' \ -+ 'git get-tar-commit-id <b.tar >actual && -+ git rev-parse HEAD > expect && -+ test_cmp expect actual' ++test_expect_success 'git get-tar-commit-id' ' ++ git get-tar-commit-id <b.tar >actual && ++ git rev-parse HEAD >expect && ++ test_cmp expect actual ++' test_expect_success 'git archive with --output, override inferred format' ' git archive --format=tar --output=d4.zip HEAD && 8: dbb81b5b89d8 = 10: 4bf1bf16bca3 t7003: use rev-parse rather than FS inspection -: ------------ > 11: 6f15c15573af t5304: restyle: trim empty lines, drop ':' before > 9: ba575839e422 ! 12: d8e80d83b6f8 t5304: use "reflog expire --all" to clear the reflog @@ Commit message ## t/t5304-prune.sh ## @@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' ' - ' test_expect_success 'prune: prune unreachable heads' ' -- git config core.logAllRefUpdates false && - mv .git/logs .git/logs.old && - : > file2 && +- : > file2 && ++ >file2 && git add file2 && git commit -m temporary && tmp_head=$(git rev-list -1 HEAD) && @@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' ' + git reflog expire --all && git prune && test_must_fail git reset $tmp_head -- -- ' - +@@ t/t5304-prune.sh: test_expect_success 'prune: prune unreachable heads' ' test_expect_success 'prune: do not prune detached HEAD with no reflog' ' - git checkout --detach --quiet && git commit --allow-empty -m "detached commit" && - # verify that there is no reflogs @@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' ' + git reflog expire --all && git prune -n >prune_actual && test_must_be_empty prune_actual - -@@ t/t5304-prune.sh: test_expect_success 'prune: prune former HEAD after checking out branch' ' - + ' +@@ t/t5304-prune.sh: test_expect_success 'prune: do not prune detached HEAD with no reflog' ' + test_expect_success 'prune: prune former HEAD after checking out branch' ' head_oid=$(git rev-parse HEAD) && git checkout --quiet main && + git reflog expire --all && git prune -v >prune_actual && grep "$head_oid" prune_actual - + ' 10: 3d3b733c3127 ! 13: 180847f4db14 test-lib: provide test prereq REFFILES @@ Commit message Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> + ## t/README ## +@@ t/README: use these, and "test_set_prereq" for how to define your own. + + Git wasn't compiled with NO_PTHREADS=YesPlease. + ++ - REFFILES ++ ++ Test is specific to packed/loose ref storage, and should be ++ disabled for other ref storage backends ++ ++ + Tips for Writing Tests + ---------------------- + + ## t/test-lib.sh ## @@ t/test-lib.sh: parisc* | hppa*) ;; 11: dd1f6969c28d ! 14: f3307b62bfd7 t1407: require REFFILES for for_each_reflog test @@ Metadata ## Commit message ## t1407: require REFFILES for for_each_reflog test - It tries to setup a reflog by catting to .git/logs/ + Add extensive comment why this test needs a REFFILES annotation. + + I tried forcing universal reflog creation with core.logAllRefUpdates=true, but + that apparently also doesn't cause reflogs to be created for pseudorefs Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> @@ t/t1407-worktree-ref-store.sh: test_expect_success 'create_symref(FOO, refs/head ' -test_expect_success 'for_each_reflog()' ' ++# Some refs (refs/bisect/*, pseudorefs) are kept per worktree, so they should ++# only appear in the for-each-reflog output if it is called from the correct ++# worktree, which is exercised in this test. This test is poorly written (and ++# therefore marked REFFILES) for mulitple reasons: 1) it creates invalidly ++# formatted log entres. 2) it uses direct FS access for creating the reflogs. 3) ++# PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is ++# not testing a realistic scenario. +test_expect_success REFFILES 'for_each_reflog()' ' echo $ZERO_OID > .git/logs/PSEUDO-MAIN && mkdir -p .git/logs/refs/bisect && 12: 86951eb39cb6 = 15: 0d3b18cd3542 t1414: mark corruption test with REFFILES 13: 1ce545043846 = 16: b64e3e7ade15 t2017: mark --orphan/logAllRefUpdates=false test as REFFILES 14: a3abc4f70e7d ! 17: fcc2b714dd50 t1404: mark tests that muck with .git directly as REFFILES. @@ Metadata ## Commit message ## t1404: mark tests that muck with .git directly as REFFILES. + The packed/loose ref storage is an overlay combination of packed-refs (refs and + tags in a single file) and one-file-per-ref. This creates all kinds of edge + cases related to directory/file conflicts, (non-)empty directories, and the + locking scheme, none of which applies to reftable. + Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> ## t/t1404-update-ref-errors.sh ## 15: 2b3021c4ba62 ! 18: ff3b67c84c41 t7900: mark pack-refs tests as REFFILES @@ Metadata ## Commit message ## t7900: mark pack-refs tests as REFFILES + Reftable automatically compacts tables on writes. The functionality of + incremental compaction is unittested in reftable/stack_test.c + (test_reftable_stack_auto_compaction) + + In addition, pack-refs triggers a full compaction of the entire stack. This is + exercised in t0031-reftable.sh. + + Given that git-maintenance simply calls out git-pack-refs, it seems superfluous + to test this further for reftable. + Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> ## t/t7900-maintenance.sh ## 16: a5b9439192db ! 19: 24dcf05d8fa6 t7003: check reflog existence only for REFFILES @@ t/t7003-filter-branch.sh: test_expect_success '--prune-empty is able to prune en git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire && test_must_fail git rev-parse refs/heads/prune-entire && - test_must_fail git reflog exists refs/heads/prune-entire -+ if test_have_prereq REFFILES ; then ++ if test_have_prereq REFFILES ++ then + test_must_fail git reflog exists refs/heads/prune-entire + fi ' 17: a2cce772d44f = 20: a33cdfda74ff t4202: mark bogus head hash test with REFFILES 18: 0665edb1308b ! 21: d7e5de8dba46 t1415: set REFFILES for test specific to storage format @@ Metadata ## Commit message ## t1415: set REFFILES for test specific to storage format + Packing refs (and therefore checking that certain refs are not packed) is a + property of the packed/loose ref storage. Add a comment to explain what the test + checks. + Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> ## t/t1415-worktree-refs.sh ## @@ t/t1415-worktree-refs.sh: test_expect_success 'setup' ' ' -test_expect_success 'refs/worktree must not be packed' ' ++# The 'packed-refs' files is stored directly in .git/. This means it is global ++# to the repository, and can only contain refs that are shared across all ++# worktrees. +test_expect_success REFFILES 'refs/worktree must not be packed' ' git pack-refs --all && test_path_is_missing .git/refs/tags/wt1 &&