{"thread":{"id":"45449","subject":"[PATCH 0/5] t1400: modernize style","startedAt":"2017-03-21T00:56:27Z","lastAt":"2017-03-22T01:59:40Z","messageCount":16,"participants":["Kyle Meyer","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"314752","messageId":"20170321005616.31324-1-kyle@kyleam.com","threadId":"45449","inReplyTo":null,"subject":"[PATCH 0/5] t1400: modernize style","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-03-21T00:56:11Z","receivedAt":"2017-03-21T00:56:27Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"These patches follow up on Peff's suggestion to modernize the style in\nt1400-update-ref.sh.\n\n 20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net\n\nThe first two commits aren't concerned with \"modernizing\" the tests,\nbut instead address issues that I noticed while looking more closely\nat t1400.\n\nI also considered\n\n * making the quoting/spacing/breaks around the test descriptions and\n bodies more consistent, but I think this leads to too much code\n churn.\n\n * moving the here-documents for log creation into the following\n tests, but I don't think it's worth it because it makes already\n long lines even longer.\n\n t1400: rename test descriptions to be unique\n t1400: set core.logAllRefUpdates in \"logged by touch\" tests\n t1400: use test_path_is_* helpers\n t1400: remove a set of unused output files\n t1400: use test_when_finished for cleanup\n\n t/t1400-update-ref.sh | 154 +++++++++++++++++++++++++-------------------------\n 1 file changed, 78 insertions(+), 76 deletions(-)\n\n-- \n2.12.0\n\n"},{"id":"314753","messageId":"20170321005616.31324-2-kyle@kyleam.com","threadId":"45449","inReplyTo":"20170321005616.31324-1-kyle@kyleam.com","subject":"[PATCH 1/5] t1400: rename test descriptions to be unique","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-03-21T00:56:12Z","receivedAt":"2017-03-21T00:56:30Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"A few tests share their description with another test. Extend the\ndescriptions to indicate how the tests differ.\n\nSigned-off-by: Kyle Meyer \n---\n t/t1400-update-ref.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 825422341..fde5b98af 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -40,7 +40,7 @@ test_expect_success \\\n \t\"git update-ref $m $A &&\n \t test $A\"' = $(cat .git/'\"$m\"')'\n test_expect_success \\\n-\t\"create $m\" \\\n+\t\"create $m with oldvalue verification\" \\\n \t\"git update-ref $m $B $A &&\n \t test $B\"' = $(cat .git/'\"$m\"')'\n test_expect_success \"fail to delete $m with stale ref\" '\n@@ -72,7 +72,7 @@ test_expect_success \\\n \t\"git update-ref HEAD $A &&\n \t test $A\"' = $(cat .git/'\"$m\"')'\n test_expect_success \\\n-\t\"create $m (by HEAD)\" \\\n+\t\"create $m (by HEAD) with oldvalue verification\" \\\n \t\"git update-ref HEAD $B $A &&\n \t test $B\"' = $(cat .git/'\"$m\"')'\n test_expect_success \"fail to delete $m (by HEAD) with stale ref\" '\n@@ -307,7 +307,7 @@ $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000\tSwitch\n $B $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150860 +0000\n EOF\n test_expect_success \\\n-\t\"verifying $m's log\" \\\n+\t\"verifying $m's log (logged by touch)\" \\\n \t\"test_cmp expect .git/logs/$m\"\n rm -rf .git/$m .git/logs expect\n \n@@ -338,7 +338,7 @@ $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 +0000\tSwitch\n $B $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 +0000\n EOF\n test_expect_success \\\n-\t\"verifying $m's log\" \\\n+\t\"verifying $m's log (logged by config)\" \\\n \t'test_cmp expect .git/logs/$m'\n rm -f .git/$m .git/logs/$m expect\n \n-- \n2.12.0\n\n"},{"id":"314754","messageId":"20170321005616.31324-3-kyle@kyleam.com","threadId":"45449","inReplyTo":"20170321005616.31324-1-kyle@kyleam.com","subject":"[PATCH 2/5] t1400: set core.logAllRefUpdates in \"logged by touch\" tests","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-03-21T00:56:13Z","receivedAt":"2017-03-21T00:56:32Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"A group of update-ref tests verifies that logs are created when either\nthe log file for the ref already exists or core.logAllRefUpdates is\n\"true\". However, when the default for core.logAllRefUpdates was\nchanged in 0bee59186 (Enable reflogs by default in any repository with\na working directory., 2006-12-14), the setup for the tests was not\nupdated. As a result, the \"logged by touch\" tests would pass even if\nthe log file did not exist (i.e., if \"--create-reflog\" was removed\nfrom the first \"git update-ref\" call).\n\nUpdate the \"logged by touch\" tests to disable core.logAllRefUpdates\nexplicitly so that the behavior does not depend on the default value.\nWhile we're here, update the \"logged by config\" tests to use\ntest_config() rather than setting core.logAllRefUpdates to \"true\"\noutside of the tests.\n\nSigned-off-by: Kyle Meyer \n---\n\nI'm confused about the setup for the \"logged by touch\" tests.\nd0ab05849 (tests: remove some direct access to .git/logs, 2015-07-27)\nchanged the setup to delete the log file itself rather than its\ncontents. The reflog was then recreated by using \"--create-reflog\" in\nthe \"create $m (logged by touch)\" test. What I don't understand is\nhow this change fits with d0ab05849, which seems to be concerned with\nloosening the assumption that the logs are stored in .git/logs. For\nthese particular tests, we are still removing\n.git/logs/refs/heads/master explictly, so why not leave the empty file\naround so that the \"create $m (logged by touch)\" actually tests that\nupdate-ref call is logged by the existence of the log rather than\nusing \"--create-reflog\", which overrides this?\n\n t/t1400-update-ref.sh | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex fde5b98af..be8b113b1 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -260,17 +260,20 @@ rm -f .git/$m\n rm -f .git/logs/refs/heads/master\n test_expect_success \\\n \t\"create $m (logged by touch)\" \\\n-\t'GIT_COMMITTER_DATE=\"2005-05-26 23:30\" \\\n+\t'test_config core.logAllRefUpdates false &&\n+\t GIT_COMMITTER_DATE=\"2005-05-26 23:30\" \\\n \t git update-ref --create-reflog HEAD '\"$A\"' -m \"Initial Creation\" &&\n \t test '\"$A\"' = $(cat .git/'\"$m\"')'\n test_expect_success \\\n \t\"update $m (logged by touch)\" \\\n-\t'GIT_COMMITTER_DATE=\"2005-05-26 23:31\" \\\n+\t'test_config core.logAllRefUpdates false &&\n+\t GIT_COMMITTER_DATE=\"2005-05-26 23:31\" \\\n \t git update-ref HEAD'\" $B $A \"'-m \"Switch\" &&\n \t test '\"$B\"' = $(cat .git/'\"$m\"')'\n test_expect_success \\\n \t\"set $m (logged by touch)\" \\\n-\t'GIT_COMMITTER_DATE=\"2005-05-26 23:41\" \\\n+\t'test_config core.logAllRefUpdates false &&\n+\t GIT_COMMITTER_DATE=\"2005-05-26 23:41\" \\\n \t git update-ref HEAD'\" $A &&\n \t test $A\"' = $(cat .git/'\"$m\"')'\n \n@@ -312,23 +315,21 @@ test_expect_success \\\n rm -rf .git/$m .git/logs expect\n \n test_expect_success \\\n-\t'enable core.logAllRefUpdates' \\\n-\t'git config core.logAllRefUpdates true &&\n-\t test true = $(git config --bool --get core.logAllRefUpdates)'\n-\n-test_expect_success \\\n \t\"create $m (logged by config)\" \\\n-\t'GIT_COMMITTER_DATE=\"2005-05-26 23:32\" \\\n+\t'test_config core.logAllRefUpdates true &&\n+\t GIT_COMMITTER_DATE=\"2005-05-26 23:32\" \\\n \t git update-ref HEAD'\" $A \"'-m \"Initial Creation\" &&\n \t test '\"$A\"' = $(cat .git/'\"$m\"')'\n test_expect_success \\\n \t\"update $m (logged by config)\" \\\n-\t'GIT_COMMITTER_DATE=\"2005-05-26 23:33\" \\\n+\t'test_config core.logAllRefUpdates true &&\n+\t GIT_COMMITTER_DATE=\"2005-05-26 23:33\" \\\n \t git update-ref HEAD'\" $B $A \"'-m \"Switch\" &&\n \t test '\"$B\"' = $(cat .git/'\"$m\"')'\n test_expect_success \\\n \t\"set $m (logged by config)\" \\\n-\t'GIT_COMMITTER_DATE=\"2005-05-26 23:43\" \\\n+\t'test_config core.logAllRefUpdates true &&\n+\t GIT_COMMITTER_DATE=\"2005-05-26 23:43\" \\\n \t git update-ref HEAD '\"$A &&\n \t test $A\"' = $(cat .git/'\"$m\"')'\n \n-- \n2.12.0\n\n"},{"id":"314755","messageId":"20170321005616.31324-4-kyle@kyleam.com","threadId":"45449","inReplyTo":"20170321005616.31324-1-kyle@kyleam.com","subject":"[PATCH 3/5] t1400: use test_path_is_* helpers","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-03-21T00:56:14Z","receivedAt":"2017-03-21T00:56:34Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Signed-off-by: Kyle Meyer \n---\n t/t1400-update-ref.sh | 44 ++++++++++++++++++++++----------------------\n 1 file changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex be8b113b1..c5c8e95fc 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -49,7 +49,7 @@ test_expect_success \"fail to delete $m with stale ref\" '\n '\n test_expect_success \"delete $m\" '\n \tgit update-ref -d $m $B &&\n-\t! test -f .git/$m\n+\ttest_path_is_missing .git/$m\n '\n rm -f .git/$m\n \n@@ -57,7 +57,7 @@ test_expect_success \"delete $m without oldvalue verification\" \"\n \tgit update-ref $m $A &&\n \ttest $A = \\$(cat .git/$m) &&\n \tgit update-ref -d $m &&\n-\t! test -f .git/$m\n+\ttest_path_is_missing .git/$m\n \"\n rm -f .git/$m\n \n@@ -81,7 +81,7 @@ test_expect_success \"fail to delete $m (by HEAD) with stale ref\" '\n '\n test_expect_success \"delete $m (by HEAD)\" '\n \tgit update-ref -d HEAD $B &&\n-\t! test -f .git/$m\n+\ttest_path_is_missing .git/$m\n '\n rm -f .git/$m\n \n@@ -89,7 +89,7 @@ test_expect_success \"deleting current branch adds message to HEAD's log\" '\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-$m -d $m &&\n-\t! test -f .git/$m &&\n+\ttest_path_is_missing .git/$m &&\n \tgrep \"delete-$m$\" .git/logs/HEAD\n '\n rm -f .git/$m\n@@ -98,7 +98,7 @@ test_expect_success \"deleting by HEAD adds message to HEAD's log\" '\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-by-head -d HEAD &&\n-\t! test -f .git/$m &&\n+\ttest_path_is_missing .git/$m &&\n \tgrep \"delete-by-head$\" .git/logs/HEAD\n '\n rm -f .git/$m\n@@ -190,14 +190,14 @@ test_expect_success \\\n test_expect_success \"delete $m (by HEAD) should remove both packed and loose $m\" '\n \tgit update-ref -d HEAD $B &&\n \t! grep \"$m\" .git/packed-refs &&\n-\t! test -f .git/$m\n+\ttest_path_is_missing .git/$m\n '\n rm -f .git/$m\n \n cp -f .git/HEAD .git/HEAD.orig\n test_expect_success \"delete symref without dereference\" '\n \tgit update-ref --no-deref -d HEAD &&\n-\t! test -f .git/HEAD\n+\ttest_path_is_missing .git/HEAD\n '\n cp -f .git/HEAD.orig .git/HEAD\n \n@@ -207,7 +207,7 @@ test_expect_success \"delete symref without dereference when the referred ref is\n \tgit commit -m foo &&\n \tgit pack-refs --all &&\n \tgit update-ref --no-deref -d HEAD &&\n-\t! test -f .git/HEAD\n+\ttest_path_is_missing .git/HEAD\n '\n cp -f .git/HEAD.orig .git/HEAD\n git update-ref -d $m\n@@ -242,7 +242,7 @@ test_expect_success '(not) create HEAD with old sha1' \"\n \ttest_must_fail git update-ref HEAD $A $B\n \"\n test_expect_success \"(not) prior created .git/$m\" \"\n-\t! test -f .git/$m\n+\ttest_path_is_missing .git/$m\n \"\n rm -f .git/$m\n \n@@ -280,13 +280,13 @@ test_expect_success \\\n test_expect_success \"empty directory removal\" '\n \tgit branch d1/d2/r1 HEAD &&\n \tgit branch d1/r2 HEAD &&\n-\ttest -f .git/refs/heads/d1/d2/r1 &&\n-\ttest -f .git/logs/refs/heads/d1/d2/r1 &&\n+\ttest_path_is_file .git/refs/heads/d1/d2/r1 &&\n+\ttest_path_is_file .git/logs/refs/heads/d1/d2/r1 &&\n \tgit branch -d d1/d2/r1 &&\n-\t! test -e .git/refs/heads/d1/d2 &&\n-\t! test -e .git/logs/refs/heads/d1/d2 &&\n-\ttest -f .git/refs/heads/d1/r2 &&\n-\ttest -f .git/logs/refs/heads/d1/r2\n+\ttest_path_is_missing .git/refs/heads/d1/d2 &&\n+\ttest_path_is_missing .git/logs/refs/heads/d1/d2 &&\n+\ttest_path_is_file .git/refs/heads/d1/r2 &&\n+\ttest_path_is_file .git/logs/refs/heads/d1/r2\n '\n \n test_expect_success \"symref empty directory removal\" '\n@@ -294,14 +294,14 @@ test_expect_success \"symref empty directory removal\" '\n \tgit branch e1/r2 HEAD &&\n \tgit checkout e1/e2/r1 &&\n \ttest_when_finished \"git checkout master\" &&\n-\ttest -f .git/refs/heads/e1/e2/r1 &&\n-\ttest -f .git/logs/refs/heads/e1/e2/r1 &&\n+\ttest_path_is_file .git/refs/heads/e1/e2/r1 &&\n+\ttest_path_is_file .git/logs/refs/heads/e1/e2/r1 &&\n \tgit update-ref -d HEAD &&\n-\t! test -e .git/refs/heads/e1/e2 &&\n-\t! test -e .git/logs/refs/heads/e1/e2 &&\n-\ttest -f .git/refs/heads/e1/r2 &&\n-\ttest -f .git/logs/refs/heads/e1/r2 &&\n-\ttest -f .git/logs/HEAD\n+\ttest_path_is_missing .git/refs/heads/e1/e2 &&\n+\ttest_path_is_missing .git/logs/refs/heads/e1/e2 &&\n+\ttest_path_is_file .git/refs/heads/e1/r2 &&\n+\ttest_path_is_file .git/logs/refs/heads/e1/r2 &&\n+\ttest_path_is_file .git/logs/HEAD\n '\n \n cat >expect <\n---\n t/t1400-update-ref.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex c5c8e95fc..9e7e0227e 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -64,8 +64,8 @@ rm -f .git/$m\n test_expect_success \\\n \t\"fail to create $n\" \\\n \t\"touch .git/$n_dir &&\n-\t test_must_fail git update-ref $n $A >out 2>err\"\n-rm -f .git/$n_dir out err\n+\t test_must_fail git update-ref $n $A\"\n+rm -f .git/$n_dir\n \n test_expect_success \\\n \t\"create $m (by HEAD)\" \\\n-- \n2.12.0\n\n"},{"id":"314757","messageId":"20170321005616.31324-6-kyle@kyleam.com","threadId":"45449","inReplyTo":"20170321005616.31324-1-kyle@kyleam.com","subject":"[PATCH 5/5] t1400: use test_when_finished for cleanup","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-03-21T00:56:16Z","receivedAt":"2017-03-21T00:56:47Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Move cleanup lines that occur after test blocks into\ntest_when_finished calls within the test bodies. Don't move cleanup\nlines that seem to be related to mutiple tests rather than a single\ntest.\n\nSigned-off-by: Kyle Meyer \n---\n t/t1400-update-ref.sh | 81 ++++++++++++++++++++++++++-------------------------\n 1 file changed, 41 insertions(+), 40 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 9e7e0227e..a23cd7b57 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -48,24 +48,24 @@ test_expect_success \"fail to delete $m with stale ref\" '\n \ttest $B = \"$(cat .git/$m)\"\n '\n test_expect_success \"delete $m\" '\n+\ttest_when_finished \"rm -f .git/$m\" &&\n \tgit update-ref -d $m $B &&\n \ttest_path_is_missing .git/$m\n '\n-rm -f .git/$m\n \n-test_expect_success \"delete $m without oldvalue verification\" \"\n+test_expect_success \"delete $m without oldvalue verification\" '\n+\ttest_when_finished \"rm -f .git/$m\" &&\n \tgit update-ref $m $A &&\n-\ttest $A = \\$(cat .git/$m) &&\n+\ttest $A = $(cat .git/$m) &&\n \tgit update-ref -d $m &&\n \ttest_path_is_missing .git/$m\n-\"\n-rm -f .git/$m\n+'\n \n-test_expect_success \\\n-\t\"fail to create $n\" \\\n-\t\"touch .git/$n_dir &&\n-\t test_must_fail git update-ref $n $A\"\n-rm -f .git/$n_dir\n+test_expect_success \"fail to create $n\" '\n+\ttest_when_finished \"rm -f .git/$n_dir\" &&\n+\ttouch .git/$n_dir &&\n+\ttest_must_fail git update-ref $n $A\n+'\n \n test_expect_success \\\n \t\"create $m (by HEAD)\" \\\n@@ -80,28 +80,28 @@ test_expect_success \"fail to delete $m (by HEAD) with stale ref\" '\n \ttest $B = $(cat .git/$m)\n '\n test_expect_success \"delete $m (by HEAD)\" '\n+\ttest_when_finished \"rm -f .git/$m\" &&\n \tgit update-ref -d HEAD $B &&\n \ttest_path_is_missing .git/$m\n '\n-rm -f .git/$m\n \n test_expect_success \"deleting current branch adds message to HEAD's log\" '\n+\ttest_when_finished \"rm -f .git/$m\" &&\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-$m -d $m &&\n \ttest_path_is_missing .git/$m &&\n \tgrep \"delete-$m$\" .git/logs/HEAD\n '\n-rm -f .git/$m\n \n test_expect_success \"deleting by HEAD adds message to HEAD's log\" '\n+\ttest_when_finished \"rm -f .git/$m\" &&\n \tgit update-ref $m $A &&\n \tgit symbolic-ref HEAD $m &&\n \tgit update-ref -m delete-by-head -d HEAD &&\n \ttest_path_is_missing .git/$m &&\n \tgrep \"delete-by-head$\" .git/logs/HEAD\n '\n-rm -f .git/$m\n \n test_expect_success 'update-ref does not create reflogs by default' '\n \ttest_when_finished \"git update-ref -d $outside\" &&\n@@ -188,20 +188,21 @@ test_expect_success \\\n \t\"git update-ref HEAD $B $A &&\n \t test $B\"' = $(cat .git/'\"$m\"')'\n test_expect_success \"delete $m (by HEAD) should remove both packed and loose $m\" '\n+\ttest_when_finished \"rm -f .git/$m\" &&\n \tgit update-ref -d HEAD $B &&\n \t! grep \"$m\" .git/packed-refs &&\n \ttest_path_is_missing .git/$m\n '\n-rm -f .git/$m\n \n cp -f .git/HEAD .git/HEAD.orig\n test_expect_success \"delete symref without dereference\" '\n+\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n \tgit update-ref --no-deref -d HEAD &&\n \ttest_path_is_missing .git/HEAD\n '\n-cp -f .git/HEAD.orig .git/HEAD\n \n test_expect_success \"delete symref without dereference when the referred ref is packed\" '\n+\ttest_when_finished \"cp -f .git/HEAD.orig .git/HEAD\" &&\n \techo foo >foo.c &&\n \tgit add foo.c &&\n \tgit commit -m foo &&\n@@ -209,7 +210,7 @@ test_expect_success \"delete symref without dereference when the referred ref is\n \tgit update-ref --no-deref -d HEAD &&\n \ttest_path_is_missing .git/HEAD\n '\n-cp -f .git/HEAD.orig .git/HEAD\n+\n git update-ref -d $m\n \n test_expect_success 'update-ref -d is not confused by self-reference' '\n@@ -241,10 +242,10 @@ test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '\n test_expect_success '(not) create HEAD with old sha1' \"\n \ttest_must_fail git update-ref HEAD $A $B\n \"\n-test_expect_success \"(not) prior created .git/$m\" \"\n+test_expect_success \"(not) prior created .git/$m\" '\n+\ttest_when_finished \"rm -f .git/$m\" &&\n \ttest_path_is_missing .git/$m\n-\"\n-rm -f .git/$m\n+'\n \n test_expect_success \\\n \t\"create HEAD\" \\\n@@ -252,10 +253,10 @@ test_expect_success \\\n test_expect_success '(not) change HEAD with wrong SHA1' \"\n \ttest_must_fail git update-ref HEAD $B $Z\n \"\n-test_expect_success \"(not) changed .git/$m\" \"\n-\t! test $B\"' = $(cat .git/'\"$m\"')\n+test_expect_success \"(not) changed .git/$m\" '\n+\ttest_when_finished \"rm -f .git/$m\" &&\n+\t! test $B = $(cat .git/$m)\n '\n-rm -f .git/$m\n \n rm -f .git/logs/refs/heads/master\n test_expect_success \\\n@@ -309,10 +310,10 @@ $Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000\tInitial Creati\n $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000\tSwitch\n $B $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150860 +0000\n EOF\n-test_expect_success \\\n-\t\"verifying $m's log (logged by touch)\" \\\n-\t\"test_cmp expect .git/logs/$m\"\n-rm -rf .git/$m .git/logs expect\n+test_expect_success \"verifying $m's log (logged by touch)\" '\n+\ttest_when_finished \"rm -rf .git/$m .git/logs expect\" &&\n+\ttest_cmp expect .git/logs/$m\n+'\n \n test_expect_success \\\n \t\"create $m (logged by config)\" \\\n@@ -340,8 +341,8 @@ $B $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 +0000\n EOF\n test_expect_success \\\n \t\"verifying $m's log (logged by config)\" \\\n-\t'test_cmp expect .git/logs/$m'\n-rm -f .git/$m .git/logs/$m expect\n+\t'test_when_finished \"rm -f .git/$m .git/logs/$m expect\" &&\n+\t test_cmp expect .git/logs/$m'\n \n git update-ref $m $D\n cat >.git/logs/$m <o 2>e &&\n \t test '\"$C\"' = $(cat o) &&\n \t test \"warning: Log for '\\'master\\'' only goes back to $ed.\" = \"$(cat e)\"'\n test_expect_success \\\n \t\"Query master@{2005-05-25} (before history)\" \\\n-\t'rm -f o e &&\n+\t'test_when_finished \"rm -f o e\" &&\n \t git rev-parse --verify master@{2005-05-25} >o 2>e &&\n \t test '\"$C\"' = $(cat o) &&\n \t echo test \"warning: Log for '\\'master\\'' only goes back to $ed.\" = \"$(cat e)\"'\n test_expect_success \\\n \t'Query \"master@{May 26 2005 23:31:59}\" (1 second before history)' \\\n-\t'rm -f o e &&\n+\t'test_when_finished \"rm -f o e\" &&\n \t git rev-parse --verify \"master@{May 26 2005 23:31:59}\" >o 2>e &&\n \t test '\"$C\"' = $(cat o) &&\n \t test \"warning: Log for '\\''master'\\'' only goes back to $ed.\" = \"$(cat e)\"'\n test_expect_success \\\n \t'Query \"master@{May 26 2005 23:32:00}\" (exactly history start)' \\\n-\t'rm -f o e &&\n+\t'test_when_finished \"rm -f o e\" &&\n \t git rev-parse --verify \"master@{May 26 2005 23:32:00}\" >o 2>e &&\n \t test '\"$C\"' = $(cat o) &&\n \t test \"\" = \"$(cat e)\"'\n test_expect_success \\\n \t'Query \"master@{May 26 2005 23:32:30}\" (first non-creation change)' \\\n-\t'rm -f o e &&\n+\t'test_when_finished \"rm -f o e\" &&\n \t git rev-parse --verify \"master@{May 26 2005 23:32:30}\" >o 2>e &&\n \t test '\"$A\"' = $(cat o) &&\n \t test \"\" = \"$(cat e)\"'\n test_expect_success \\\n \t'Query \"master@{2005-05-26 23:33:01}\" (middle of history with gap)' \\\n-\t'rm -f o e &&\n+\t'test_when_finished \"rm -f o e\" &&\n \t git rev-parse --verify \"master@{2005-05-26 23:33:01}\" >o 2>e &&\n \t test '\"$B\"' = $(cat o) &&\n \t test \"warning: Log for ref '\"$m has gap after $gd\"'.\" = \"$(cat e)\"'\n test_expect_success \\\n \t'Query \"master@{2005-05-26 23:38:00}\" (middle of history)' \\\n-\t'rm -f o e &&\n+\t'test_when_finished \"rm -f o e\" &&\n \t git rev-parse --verify \"master@{2005-05-26 23:38:00}\" >o 2>e &&\n \t test '\"$Z\"' = $(cat o) &&\n \t test \"\" = \"$(cat e)\"'\n test_expect_success \\\n \t'Query \"master@{2005-05-26 23:43:00}\" (exact end of history)' \\\n-\t'rm -f o e &&\n+\t'test_when_finished \"rm -f o e\" &&\n \t git rev-parse --verify \"master@{2005-05-26 23:43:00}\" >o 2>e &&\n \t test '\"$E\"' = $(cat o) &&\n \t test \"\" = \"$(cat e)\"'\n test_expect_success \\\n \t'Query \"master@{2005-05-28}\" (past end of history)' \\\n-\t'rm -f o e &&\n+\t'test_when_finished \"rm -f o e\" &&\n \t git rev-parse --verify \"master@{2005-05-28}\" >o 2>e &&\n \t test '\"$D\"' = $(cat o) &&\n \t test \"warning: Log for ref '\"$m unexpectedly ended on $ld\"'.\" = \"$(cat e)\"'\n@@ -415,7 +416,8 @@ rm -f .git/$m .git/logs/$m expect\n \n test_expect_success \\\n 'creating initial files' \\\n- 'echo TEST >F &&\n+ 'test_when_finished rm -f M &&\n+ echo TEST >F &&\n git add F &&\n \t GIT_AUTHOR_DATE=\"2005-05-26 23:30\" \\\n \t GIT_COMMITTER_DATE=\"2005-05-26 23:30\" git commit -m add -a &&\n@@ -433,8 +435,7 @@ test_expect_success \\\n \t echo $h_TEST >.git/MERGE_HEAD &&\n \t GIT_AUTHOR_DATE=\"2005-05-26 23:45\" \\\n \t GIT_COMMITTER_DATE=\"2005-05-26 23:45\" git commit -F M &&\n-\t h_MERGED=$(git rev-parse --verify HEAD) &&\n-\t rm -f M'\n+\t h_MERGED=$(git rev-parse --verify HEAD)'\n \n cat >expect < 1117150200 +0000\tcommit (initial): add\n-- \n2.12.0\n\n"},{"id":"314766","messageId":"20170321013823.icefpibh2cjcnmsi@sigill.intra.peff.net","threadId":"45449","inReplyTo":"20170321005616.31324-2-kyle@kyleam.com","subject":"Re: [PATCH 1/5] t1400: rename test descriptions to be unique","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-21T01:38:23Z","receivedAt":"2017-03-21T01:38:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 20, 2017 at 08:56:12PM -0400, Kyle Meyer wrote:\n\n> A few tests share their description with another test. Extend the\n> descriptions to indicate how the tests differ.\n> \n> Signed-off-by: Kyle Meyer \n\nMakes sense, and your descriptions look good.\n\n-Peff\n"},{"id":"314767","messageId":"20170321015108.h62cba36qrxtxpj7@sigill.intra.peff.net","threadId":"45449","inReplyTo":"20170321005616.31324-4-kyle@kyleam.com","subject":"Re: [PATCH 3/5] t1400: use test_path_is_* helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-21T01:51:08Z","receivedAt":"2017-03-21T01:51:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 20, 2017 at 08:56:14PM -0400, Kyle Meyer wrote:\n\n> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> index be8b113b1..c5c8e95fc 100755\n> --- a/t/t1400-update-ref.sh\n> +++ b/t/t1400-update-ref.sh\n> @@ -49,7 +49,7 @@ test_expect_success \"fail to delete $m with stale ref\" '\n> '\n> test_expect_success \"delete $m\" '\n> \tgit update-ref -d $m $B &&\n> -\t! test -f .git/$m\n> +\ttest_path_is_missing .git/$m\n> '\n\nI think this is an improvement for now. I suspect that all of these will\nhave to become \"test_must_fail git rev-parse --verify $m\" in the long\nrun, when we want to run the test suite against an alternate ref\nbackend. That can wait, though.\n\n-Peff\n"},{"id":"314768","messageId":"20170321015153.iezolz5gcbaveyto@sigill.intra.peff.net","threadId":"45449","inReplyTo":"20170321005616.31324-5-kyle@kyleam.com","subject":"Re: [PATCH 4/5] t1400: remove a set of unused output files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-21T01:51:53Z","receivedAt":"2017-03-21T01:52:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 20, 2017 at 08:56:15PM -0400, Kyle Meyer wrote:\n\n> This test case redirects stdout and stderr to output files, but,\n> unlike the other cases of redirection in the t1400 tests, these files\n> are not examined downstream. Remove the redirection so that the\n> output is visible when running the tests verbosely.\n\nMakes sense.\n\n-Peff\n"},{"id":"314769","messageId":"20170321014920.qas2xbbx66zqo5lp@sigill.intra.peff.net","threadId":"45449","inReplyTo":"20170321005616.31324-3-kyle@kyleam.com","subject":"Re: [PATCH 2/5] t1400: set core.logAllRefUpdates in \"logged by touch\" tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-21T01:49:21Z","receivedAt":"2017-03-21T01:56:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 20, 2017 at 08:56:13PM -0400, Kyle Meyer wrote:\n\n> A group of update-ref tests verifies that logs are created when either\n> the log file for the ref already exists or core.logAllRefUpdates is\n> \"true\". However, when the default for core.logAllRefUpdates was\n> changed in 0bee59186 (Enable reflogs by default in any repository with\n> a working directory., 2006-12-14), the setup for the tests was not\n> updated. As a result, the \"logged by touch\" tests would pass even if\n> the log file did not exist (i.e., if \"--create-reflog\" was removed\n> from the first \"git update-ref\" call).\n> \n> Update the \"logged by touch\" tests to disable core.logAllRefUpdates\n> explicitly so that the behavior does not depend on the default value.\n\nGood catch.\n\n> While we're here, update the \"logged by config\" tests to use\n> test_config() rather than setting core.logAllRefUpdates to \"true\"\n> outside of the tests.\n\nYeah, I agree that makes the result more obvious to read.\n\n> I'm confused about the setup for the \"logged by touch\" tests.\n> d0ab05849 (tests: remove some direct access to .git/logs, 2015-07-27)\n> changed the setup to delete the log file itself rather than its\n> contents. The reflog was then recreated by using \"--create-reflog\" in\n> the \"create $m (logged by touch)\" test. What I don't understand is\n> how this change fits with d0ab05849, which seems to be concerned with\n> loosening the assumption that the logs are stored in .git/logs.\n\nI suspect the answer is that the conversion was incomplete. That commit\nwas done for alternate ref backends, which is an ongoing saga.\n\nI think it's OK to leave it as-is for now. It's not clear what \"logged\nby touch\" will look like for backends that don't use the filesystem.\nProbably it will need to call \"update-ref --create-reflog\" to kickstart\nit, and then further updates will automatically write to it.\n\nAt that point the \"rm -f\" would need to become \"tell the backend to\ndelete this reflog\". There's no command for that now, but we can add one\nlater. Until then, I suspect the \"rm -f\" would be a noop. That means\nthat the first --create-reflog test is failing to test what it claims,\nbut the result passes anyway.\n\nAnd that probably answers the question about why the conversion is\nhalf-done. It was enough to get the tests to stop complaining when built\nwith an alternate ref backend. :)\n\n> For these particular tests, we are still removing\n> .git/logs/refs/heads/master explictly, so why not leave the empty file\n> around so that the \"create $m (logged by touch)\" actually tests that\n> update-ref call is logged by the existence of the log rather than\n> using \"--create-reflog\", which overrides this?\n\nI think the tests just wanted to start at a known state. I agree that\nbe correct (and would even work with an alternate backend), but it is a\nbit subtle that we are relying on the leftover state from the previous\ntests.\n\n> t/t1400-update-ref.sh | 23 ++++++++++++-----------\n> 1 file changed, 12 insertions(+), 11 deletions(-)\n\nPatch itself looks good.\n\n-Peff\n"},{"id":"314772","messageId":"20170321015356.3unr3ptwaaerfu2y@sigill.intra.peff.net","threadId":"45449","inReplyTo":"20170321005616.31324-6-kyle@kyleam.com","subject":"Re: [PATCH 5/5] t1400: use test_when_finished for cleanup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-21T01:53:56Z","receivedAt":"2017-03-21T02:00:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 20, 2017 at 08:56:16PM -0400, Kyle Meyer wrote:\n\n> Move cleanup lines that occur after test blocks into\n> test_when_finished calls within the test bodies. Don't move cleanup\n> lines that seem to be related to mutiple tests rather than a single\n> test.\n\nSounds logical. The patch looks good to me (though I'll admit my eyes\nmay have glazed a little in the middle).\n\n-Peff\n"},{"id":"314773","messageId":"20170321020155.dx5juulntg5k6fpr@sigill.intra.peff.net","threadId":"45449","inReplyTo":"20170321005616.31324-1-kyle@kyleam.com","subject":"Re: [PATCH 0/5] t1400: modernize style","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-21T02:01:55Z","receivedAt":"2017-03-21T02:02:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 20, 2017 at 08:56:11PM -0400, Kyle Meyer wrote:\n\n> These patches follow up on Peff's suggestion to modernize the style in\n> t1400-update-ref.sh.\n> \n> 20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net\n> \n> The first two commits aren't concerned with \"modernizing\" the tests,\n> but instead address issues that I noticed while looking more closely\n> at t1400.\n\nLooks good overall to me. Thanks for following up.\n\n> I also considered\n> \n> * making the quoting/spacing/breaks around the test descriptions and\n> bodies more consistent, but I think this leads to too much code\n> churn.\n\nI wouldn't mind the churn if you wanted to do it on top, but it's\ndefinitely not necessary. There's nothing in 'pu' right now that touches\nthe file.\n\n> * moving the here-documents for log creation into the following\n> tests, but I don't think it's worth it because it makes already\n> long lines even longer.\n\nYeah, they're quite long. Probably something like:\n\n # arguments: