{"thread":{"id":"48998","subject":"[PATCH] t1404: increase core.packedRefsTimeout to avoid occasional test failure","startedAt":"2018-07-31T23:33:00Z","lastAt":"2018-08-01T01:37:42Z","messageCount":3,"participants":["SZEDER Gábor","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"354135","messageId":"20180731233248.25482-1-szeder.dev@gmail.com","threadId":"48998","inReplyTo":null,"subject":"[PATCH] t1404: increase core.packedRefsTimeout to avoid occasional test failure","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-07-31T23:32:48Z","receivedAt":"2018-07-31T23:33:00Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The test 'no bogus intermediate values during delete' in\n't1404-update-ref-errors.sh', added in 6a2a7736d8 (t1404: demonstrate\ntwo problems with reference transactions, 2017-09-08), tries to catch\nundesirable side effects of deleting a ref, both loose and packed, in\na transaction.  To do so it is holding the packed refs file locked\nwhen it starts 'git update-ref -d' in the background with a 3secs\n'core.packedRefsTimeout' value.  After performing a few checks it is\nthen supposed to unlock the packed refs file before the background\n'git update-ref's attempt to acquire the lock times out.\n\nWhile 3secs timeout seems plenty, and indeed is sufficient in most\ncases, on rare occasions it's just not quite enough: I saw this test\nfail in Travis CI build jobs two, maybe three times because 'git\nupdate-ref' timed out.\n\nIncrease that timeout by an order of magnitude to 30s to make such an\noccasional failure even more improbable.  This won't make the test run\nany longer under normal circumstances, because 'git update-ref' will\nacquire the lock and resume execution as soon as it can.  And if it\nturns out that even this increased timeout is still not enough, then\nthere are most likely bigger problems, e.g. the Travis CI build job\nwill exceed its time limit anyway, or the lockfile module is broken.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t1404-update-ref-errors.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh\nindex 3a887b5113..372f0b1fbb 100755\n--- a/t/t1404-update-ref-errors.sh\n+++ b/t/t1404-update-ref-errors.sh\n@@ -559,9 +559,9 @@ test_expect_success 'no bogus intermediate values during delete' '\n \t{\n \t\t# Note: the following command is intentionally run in the\n \t\t# background. We increase the timeout so that `update-ref`\n-\t\t# attempts to acquire the `packed-refs` lock for longer than\n-\t\t# it takes for us to do the check then delete it:\n-\t\tgit -c core.packedrefstimeout=3000 update-ref -d $prefix/foo &\n+\t\t# attempts to acquire the `packed-refs` lock for much longer\n+\t\t# than it takes for us to do the check then delete it:\n+\t\tgit -c core.packedrefstimeout=30000 update-ref -d $prefix/foo &\n \t} &&\n \tpid2=$! &&\n \t# Give update-ref plenty of time to get to the point where it tries\n-- \n2.18.0.408.g42635c01bc\n\n"},{"id":"354137","messageId":"20180731233906.GB113554@aiede.svl.corp.google.com","threadId":"48998","inReplyTo":"20180731233248.25482-1-szeder.dev@gmail.com","subject":"Re: [PATCH] t1404: increase core.packedRefsTimeout to avoid occasional test failure","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-07-31T23:39:06Z","receivedAt":"2018-07-31T23:39:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nSZEDER Gábor wrote:\n\n> While 3secs timeout seems plenty, and indeed is sufficient in most\n> cases, on rare occasions it's just not quite enough: I saw this test\n> fail in Travis CI build jobs two, maybe three times because 'git\n> update-ref' timed out.\n\nI suspect these tests will fail with valgrind (just because valgrind\nmakes things super slow).  Would it be safe to use timeout=-1?\n\nThanks,\nJonathan\n"},{"id":"354145","messageId":"CAM0VKj=aQ1kO_i2OkkcMRJZ0a8PHVQH-oKQ1_qZ5UpxEPEK5kQ@mail.gmail.com","threadId":"48998","inReplyTo":"20180731233906.GB113554@aiede.svl.corp.google.com","subject":"Re: [PATCH] t1404: increase core.packedRefsTimeout to avoid occasional test failure","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-08-01T01:37:27Z","receivedAt":"2018-08-01T01:37:42Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Aug 1, 2018 at 1:39 AM Jonathan Nieder <jrnieder@gmail.com> wrote:\n> SZEDER Gábor wrote:\n>\n> > While 3secs timeout seems plenty, and indeed is sufficient in most\n> > cases, on rare occasions it's just not quite enough: I saw this test\n> > fail in Travis CI build jobs two, maybe three times because 'git\n> > update-ref' timed out.\n>\n> I suspect these tests will fail with valgrind (just because valgrind\n> makes things super slow).  Would it be safe to use timeout=-1?\n\nI don't know.\nTravis CI has a time limit of about 45mins for the whole build job\n(including installing dependencies and compilation), and any sensible\nautomated build system must have a similar time limit, so it would be\nfine to wait indefinitely in such an environment, though in case of a\ntimeout we'd lose failure reports of failed tests, if there are any.\n\nOTOH, my (and I guess most devs') test runs don't have such a time\nlimit, so I'm reluctant to change it to wait indefinitely.  But then\nagain, waiting indefinitely for a lock file is not all that different\nfrom messing up something and creating an endless loop or a\ndeadlock...\n"}]}