[PATCH] tests: Remove some direct access to .git/logs

2015-07-27 Thread David Turner
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

2015-07-27 Thread Junio C Hamano
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

2015-07-27 Thread David Turner
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