[PATCH] tests: Remove some direct access to .git/logs
Alternate refs backends might store reflogs somewhere other than .git/logs. Change most test code that directly accesses .git/logs to instead use git reflog commands. There are still a few tests which need direct access to reflogs: to check reflog permissions, to manually create reflogs from scratch, to save/restore reflogs, to check the format of raw reflog data, and to remove not just reflog contents, but the reflogs themselves. All cases which don't need direct access have been modified. Signed-off-by: David Turner dtur...@twopensource.com --- This is a follow-up to the patch series that introduced --create-reflog Many tests need to be changed in order to pass under alternate refs backends. This particular set of issues happens to be easy to clean up, so I'm sending this patch now. --- t/t1400-update-ref.sh | 5 ++--- t/t1410-reflog.sh | 24 t/t1411-reflog-show.sh| 2 +- t/t1503-rev-parse-verify.sh | 9 +++-- t/t3200-branch.sh | 12 ++-- t/t3210-pack-refs.sh | 2 +- t/t3404-rebase-interactive.sh | 10 +- t/t3903-stash.sh | 2 +- t/t5312-prune-corruption.sh | 2 +- t/t6501-freshen-objects.sh| 2 +- t/t7509-commit.sh | 13 - 11 files changed, 37 insertions(+), 46 deletions(-) diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index d787bf5..8bf1559 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -155,12 +155,11 @@ test_expect_success (not) changed .git/$m ' rm -f .git/$m -: a repository with working tree always has reflog these days... -: .git/logs/refs/heads/master +rm -f .git/logs/refs/heads/master test_expect_success \ create $m (logged by touch) \ 'GIT_COMMITTER_DATE=2005-05-26 23:30 \ -git update-ref HEAD '$A' -m Initial Creation +git update-ref --create-reflog HEAD '$A' -m Initial Creation test '$A' = $(cat .git/'$m')' test_expect_success \ update $m (logged by touch) \ diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh index 779d4e3..b79049f 100755 --- a/t/t1410-reflog.sh +++ b/t/t1410-reflog.sh @@ -100,7 +100,8 @@ test_expect_success setup ' check_fsck - test_line_count = 4 .git/logs/refs/heads/master + git reflog refs/heads/master output + test_line_count = 4 output ' test_expect_success rewind ' @@ -116,7 +117,8 @@ test_expect_success rewind ' check_have A B C D E F G H I J K L - test_line_count = 5 .git/logs/refs/heads/master + git reflog refs/heads/master output + test_line_count = 5 output ' test_expect_success 'corrupt and check' ' @@ -134,7 +136,8 @@ test_expect_success 'reflog expire --dry-run should not touch reflog' ' --stale-fix \ --all - test_line_count = 5 .git/logs/refs/heads/master + git reflog refs/heads/master output + test_line_count = 5 output check_fsck missing blob $F ' @@ -147,7 +150,8 @@ test_expect_success 'reflog expire' ' --stale-fix \ --all - test_line_count = 2 .git/logs/refs/heads/master + git reflog refs/heads/master output + test_line_count = 2 output check_fsck dangling commit $K ' @@ -213,7 +217,8 @@ test_expect_success 'delete' ' test_expect_success 'rewind2' ' test_tick git reset --hard HEAD~2 - test_line_count = 4 .git/logs/refs/heads/master + git reflog refs/heads/master output + test_line_count = 4 output ' test_expect_success '--expire=never' ' @@ -222,7 +227,8 @@ test_expect_success '--expire=never' ' --expire=never \ --expire-unreachable=never \ --all - test_line_count = 4 .git/logs/refs/heads/master + git reflog refs/heads/master output + test_line_count = 4 output ' test_expect_success 'gc.reflogexpire=never' ' @@ -230,7 +236,8 @@ test_expect_success 'gc.reflogexpire=never' ' git config gc.reflogexpire never git config gc.reflogexpireunreachable never git reflog expire --verbose --all - test_line_count = 4 .git/logs/refs/heads/master + git reflog refs/heads/master output + test_line_count = 4 output ' test_expect_success 'gc.reflogexpire=false' ' @@ -238,7 +245,8 @@ test_expect_success 'gc.reflogexpire=false' ' git config gc.reflogexpire false git config gc.reflogexpireunreachable false git reflog expire --verbose --all - test_line_count = 4 .git/logs/refs/heads/master + git reflog refs/heads/master output + test_line_count = 4 output git config --unset gc.reflogexpire git config --unset gc.reflogexpireunreachable diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh index 6f47c0d..d568b35 100755 --- a/t/t1411-reflog-show.sh +++ b/t/t1411-reflog-show.sh @@ -138,7 +138,7 @@
Re: [PATCH] tests: Remove some direct access to .git/logs
David Turner dtur...@twopensource.com writes: -: a repository with working tree always has reflog these days... -: .git/logs/refs/heads/master +rm -f .git/logs/refs/heads/master This looks quite different from how other tests are updated (which looked a lot more sensible). The original has the effect of (1) ensuring that the log exists/is enabled for 'master' branch and (2) that log is emptied. The updated has a quite different effect, but only when you are using filesystem based backend. test_expect_success \ create $m (logged by touch) \ 'GIT_COMMITTER_DATE=2005-05-26 23:30 \ - git update-ref HEAD '$A' -m Initial Creation + git update-ref --create-reflog HEAD '$A' -m Initial Creation test '$A' = $(cat .git/'$m')' And this update compensates for the earlier remove 'master' reflog (where it should have been ensure creation and empty it) by creating, but it is unclear what would happen when we start using different ref backends. Earlier we did not remove reflog from the backend, and we say create here---it would hopefully not error out, but it does not quite feel right. Perhaps create and immediately prune all instead? That feels like more in line with the way the other change in this patch do things. - test_line_count = 4 .git/logs/refs/heads/master + git reflog refs/heads/master output + test_line_count = 4 output ' These all look sensible. diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh index 6f47c0d..d568b35 100755 --- a/t/t1411-reflog-show.sh +++ b/t/t1411-reflog-show.sh @@ -138,7 +138,7 @@ test_expect_success '--date magic does not override explicit @{0} syntax' ' : expect test_expect_success 'empty reflog file' ' git branch empty - : .git/logs/refs/heads/empty + git reflog expire --expire=all refs/heads/empty This one is what I was talking about in the earlier part of this message. This looks very sensible. test_expect_success 'fails silently when using -q with deleted reflogs' ' ref=$(git rev-parse HEAD) - : .git/logs/refs/test - git update-ref -m message for refs/test refs/test $ref + git update-ref --create-reflog -m message for refs/test refs/test $ref I cannot tell without enough pre-context, but I assume that we know there is no reflog for refs/test when this test piece starts---in which case this change looks sensible. diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 467e6c1..dc76754 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -961,13 +961,13 @@ test_expect_success 'rebase -i produces readable reflog' ' set_fake_editor git rebase -i --onto I F branch-reflog-test cat expect -\EOF - rebase -i (start): checkout I - rebase -i (pick): G - rebase -i (pick): H rebase -i (finish): returning to refs/heads/branch-reflog-test + rebase -i (pick): H + rebase -i (pick): G + rebase -i (start): checkout I EOF ;-) - tail -n 4 .git/logs/HEAD | - sed -e s/.*// actual + git reflog HEAD -n4 | This may happen to work but teaches a bad habit to readers. Always order your command line by starting with dashed-options, then refs and then pathspecs. diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh index 9ac7940..db9774e 100755 --- a/t/t7509-commit.sh +++ b/t/t7509-commit.sh @@ -90,22 +90,10 @@ sha1_file() { remove_object() { rm -f $(sha1_file $*) } -no_reflog() { - cp .git/config .git/config.saved - echo [core] logallrefupdates = false .git/config - test_when_finished mv -f .git/config.saved .git/config - - if test -e .git/logs - then - mv .git/logs . - test_when_finished mv logs .git/ - fi -} test_expect_success '--amend option with empty author' ' git cat-file commit Initial tmp sed s/author [^]* /author / tmp empty-author - no_reflog sha=$(git hash-object -t commit -w empty-author) test_when_finished remove_object $sha git checkout $sha @@ -119,7 +107,6 @@ test_expect_success '--amend option with empty author' ' test_expect_success '--amend option with missing author' ' git cat-file commit Initial tmp sed s/author [^]* /author / tmp malformed - no_reflog sha=$(git hash-object -t commit -w malformed) test_when_finished remove_object $sha git checkout $sha I can understand that .git/logs/ cannot be relied upon when you move your ref backend out of filesystem, but that does not quite explain why this change makes sense. Puzzled... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tests: Remove some direct access to .git/logs
On Mon, 2015-07-27 at 14:47 -0700, Junio C Hamano wrote: David Turner dtur...@twopensource.com writes: -: a repository with working tree always has reflog these days... -: .git/logs/refs/heads/master +rm -f .git/logs/refs/heads/master This looks quite different from how other tests are updated (which looked a lot more sensible). The original has the effect of (1) ensuring that the log exists/is enabled for 'master' branch and (2) that log is emptied. The updated has a quite different effect, but only when you are using filesystem based backend. Yes, this is one of the areas where we need to do more work for alternate backends -- for instance, we could provide a git reflog remove command to replace that rm. As the commit message notes, this patch is a subset of what's necessary for multiple backends. There are a number of other tests that e.g. rm -r .git/logs, which we would also need to change for alternate backends. test_expect_success \ create $m (logged by touch) \ 'GIT_COMMITTER_DATE=2005-05-26 23:30 \ -git update-ref HEAD '$A' -m Initial Creation +git update-ref --create-reflog HEAD '$A' -m Initial Creation test '$A' = $(cat .git/'$m')' And this update compensates for the earlier remove 'master' reflog (where it should have been ensure creation and empty it) by creating, but it is unclear what would happen when we start using different ref backends. Earlier we did not remove reflog from the backend, and we say create here---it would hopefully not error out, but it does not quite feel right. Perhaps create and immediately prune all instead? That feels like more in line with the way the other change in this patch do things. If we create and immediately prune all, we'll lose the initial creation entry, which we presumably want to test. test_expect_success 'fails silently when using -q with deleted reflogs' ' ref=$(git rev-parse HEAD) - : .git/logs/refs/test - git update-ref -m message for refs/test refs/test $ref + git update-ref --create-reflog -m message for refs/test refs/test $ref I cannot tell without enough pre-context, but I assume that we know there is no reflog for refs/test when this test piece starts---in which case this change looks sensible. Yes, that is correct. - tail -n 4 .git/logs/HEAD | - sed -e s/.*// actual + git reflog HEAD -n4 | This may happen to work but teaches a bad habit to readers. Always order your command line by starting with dashed-options, then refs and then pathspecs. Will fix on reroll. diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh index 9ac7940..db9774e 100755 --- a/t/t7509-commit.sh +++ b/t/t7509-commit.sh @@ -90,22 +90,10 @@ sha1_file() { remove_object() { rm -f $(sha1_file $*) } -no_reflog() { - cp .git/config .git/config.saved - echo [core] logallrefupdates = false .git/config - test_when_finished mv -f .git/config.saved .git/config - - if test -e .git/logs - then - mv .git/logs . - test_when_finished mv logs .git/ - fi -} test_expect_success '--amend option with empty author' ' git cat-file commit Initial tmp sed s/author [^]* /author / tmp empty-author - no_reflog sha=$(git hash-object -t commit -w empty-author) test_when_finished remove_object $sha git checkout $sha @@ -119,7 +107,6 @@ test_expect_success '--amend option with empty author' ' test_expect_success '--amend option with missing author' ' git cat-file commit Initial tmp sed s/author [^]* /author / tmp malformed - no_reflog sha=$(git hash-object -t commit -w malformed) test_when_finished remove_object $sha git checkout $sha I can understand that .git/logs/ cannot be relied upon when you move your ref backend out of filesystem, but that does not quite explain why this change makes sense. Puzzled... It turns out that the removed portion of the test is totally unnecessary; the tests are completely unrelated to reflogs. On the reroll, I'll split this into a separate patch with its own explanation. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html