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@xxxxxxxxx cc: Ævar Arnfjörð Bjarmason > avarab@xxxxxxxxx cc: Han-Wen Nienhuys hanwen@xxxxxxxxxx > > cc: Han-Wen Nienhuys hanwen@xxxxxxxxxx > > 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@xxxxxxxxxx> > > ## 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@xxxxxxxxxx> > > @@ 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@xxxxxxxxxx> > > ## 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@xxxxxxxxxx> > > ## 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@xxxxxxxxxx> > > + ## 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@xxxxxxxxxx> > > @@ 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@xxxxxxxxxx> > > ## 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@xxxxxxxxxx> > > ## 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@xxxxxxxxxx> > > ## 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.
