{"thread":{"id":"30397","subject":"[PATCH] t/Makefile: retain cache t/.prove across prove runs","startedAt":"2012-05-02T15:31:52Z","lastAt":"2012-05-20T21:18:17Z","messageCount":5,"participants":["mhagger@alum.mit.edu","Jeff King","Michael Haggerty","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"190570","messageId":"1335972712-20621-1-git-send-email-mhagger@alum.mit.edu","threadId":"30397","inReplyTo":null,"subject":"[PATCH] t/Makefile: retain cache t/.prove across prove runs","fromName":"","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-05-02T15:31:52Z","receivedAt":"2012-05-02T15:31:52Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"From: Michael Haggerty <mhagger@alum.mit.edu>\n\nprove(1) can write a summary of its test results and timings into a\ncache file, t/.prove, then use this information during later runs for\nvarious purposes.  But deleting t/.prove after every test run defeats\nthis purpose.  So do not delete t/.prove as part of \"make\nDEFAILT_TEST_TARGET=prove test\".  (Continue to delete the file on\n\"make clean\".)\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\nprove(1) can be told to retain information about test runs, and to use\nit for interesting purposes during future invocations.  For example,\nit can be told to run tests that failed during the last run:\n\n    GIT_PROVE_OPTS = --state=failed,save\n\nor that have failed recently:\n\n    GIT_PROVE_OPTS = --state=hot,save\n\nor that have been edited since the last run of prove:\n\n    GIT_PROVE_OPTS = --state=fresh,save\n\nIt can also run tests in parallel, and start them in order from\nslowest to fastest, which, combined with running tests in parallel,\ncan improve CPU utilization:\n\n    GIT_PROVE_OPTS = --timer --jobs 5 --state=slow,save\n\n(On my 4-core notebook, the latter speeds up the tests by about 10%\ncompared to running them in parallel in numerical order.)\n\n t/Makefile |    6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 6091211..88e289f 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -28,7 +28,7 @@ test: pre-clean $(TEST_LINT)\n \n prove: pre-clean $(TEST_LINT)\n \t@echo \"*** prove ***\"; GIT_CONFIG=.git/config $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n-\t$(MAKE) clean\n+\t$(MAKE) clean-except-prove-cache\n \n $(T):\n \t@echo \"*** $@ ***\"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n@@ -36,9 +36,11 @@ $(T):\n pre-clean:\n \t$(RM) -r test-results\n \n-clean:\n+clean-except-prove-cache:\n \t$(RM) -r 'trash directory'.* test-results\n \t$(RM) -r valgrind/bin\n+\n+clean: clean-except-prove-cache\n \t$(RM) .prove\n \n test-lint: test-lint-duplicates test-lint-executable\n-- \n1.7.10\n"},{"id":"190572","messageId":"20120502160753.GA7193@sigill.intra.peff.net","threadId":"30397","inReplyTo":"1335972712-20621-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH] t/Makefile: retain cache t/.prove across prove runs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-02T16:07:53Z","receivedAt":"2012-05-02T16:07:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 02, 2012 at 05:31:52PM +0200, mhagger@alum.mit.edu wrote:\n\n> prove(1) can write a summary of its test results and timings into a\n> cache file, t/.prove, then use this information during later runs for\n> various purposes.  But deleting t/.prove after every test run defeats\n> this purpose.  So do not delete t/.prove as part of \"make\n> DEFAILT_TEST_TARGET=prove test\".  (Continue to delete the file on\n> \"make clean\".)\n\nThanks. I had found some odd behaviors with putting \"--state\" into\nGIT_PROVE_OPTS when I first switched my setup to use prove, but I didn't\ninvestigate. I think this issue explained some of what I saw.\n\nDo note, though, that we only clean \".prove\" on a successful run. So\nsomething like \"--state=hot,save\" will still see the cache from the last\nrun if there were failures. However, the prove cache is meant to be kept\nacross many runs, even successful ones, so I think your patch makes\nsense.\n\n> ---\n> prove(1) can be told to retain information about test runs, and to use\n> it for interesting purposes during future invocations.  For example,\n> it can be told to run tests that failed during the last run:\n> \n>     GIT_PROVE_OPTS = --state=failed,save\n\nI don't think this actually works, because we also feed all of the test\nscripts to prove on the command line. So it will run them all, both\nfailed and successful.\n\nI'm not sure if it is worth solving for \"make test\". I suspect having\nthat not test everything would be error prone (i.e., it's too easy to\naccidentally not run the full suite when you meant to, and unexpected\nregressions in other tests will go unnoticed). You can always run\n\"prove\" yourself from the command line, or we can add a new target to do\na partial test (with other options set up properly).\n\n-Peff\n"},{"id":"191738","messageId":"4FB7B429.80805@alum.mit.edu","threadId":"30397","inReplyTo":"20120502160753.GA7193@sigill.intra.peff.net","subject":"Re: [PATCH] t/Makefile: retain cache t/.prove across prove runs","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-05-19T14:54:33Z","receivedAt":"2012-05-19T14:54:33Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/02/2012 06:07 PM, Jeff King wrote:\n> On Wed, May 02, 2012 at 05:31:52PM +0200, mhagger@alum.mit.edu wrote:\n>> prove(1) can write a summary of its test results and timings into a\n>> cache file, t/.prove, then use this information during later runs for\n>> various purposes.  But deleting t/.prove after every test run defeats\n>> this purpose.  So do not delete t/.prove as part of \"make\n>> DEFAILT_TEST_TARGET=prove test\".  (Continue to delete the file on\n>> \"make clean\".)\n>\n> Thanks. I had found some odd behaviors with putting \"--state\" into\n> GIT_PROVE_OPTS when I first switched my setup to use prove, but I didn't\n> investigate. I think this issue explained some of what I saw.\n>\n> Do note, though, that we only clean \".prove\" on a successful run. So\n> something like \"--state=hot,save\" will still see the cache from the last\n> run if there were failures. However, the prove cache is meant to be kept\n> across many runs, even successful ones, so I think your patch makes\n> sense.\n>\n>> ---\n>> prove(1) can be told to retain information about test runs, and to use\n>> it for interesting purposes during future invocations.  For example,\n>> it can be told to run tests that failed during the last run:\n>>\n>>      GIT_PROVE_OPTS = --state=failed,save\n>\n> I don't think this actually works, because we also feed all of the test\n> scripts to prove on the command line. So it will run them all, both\n> failed and successful.\n>\n> I'm not sure if it is worth solving for \"make test\". I suspect having\n> that not test everything would be error prone (i.e., it's too easy to\n> accidentally not run the full suite when you meant to, and unexpected\n> regressions in other tests will go unnoticed). You can always run\n> \"prove\" yourself from the command line, or we can add a new target to do\n> a partial test (with other options set up properly).\n\nI totally believe you that some of the variations that I listed in my \ncommentary don't work in the git context.  I'm not a prove expert; I \njust noticed that removing the .prove file is counterproductive and \nbreaks some other prove features.  I also agree with you that it would \nbe dangerous to encourage partial testing and that it is therefore not a \npriority to make the use case that you mentioned work in the git context.\n\nI still think my patch makes sense.  The error that Peff pointed out was \nin my commentary, not in the patch itself or in the log message.  Junio, \nis there something else keeping you from applying this patch?\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"191739","messageId":"20120519152711.GA6979@sigill.intra.peff.net","threadId":"30397","inReplyTo":"4FB7B429.80805@alum.mit.edu","subject":"Re: [PATCH] t/Makefile: retain cache t/.prove across prove runs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-19T15:27:12Z","receivedAt":"2012-05-19T15:27:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 19, 2012 at 04:54:33PM +0200, Michael Haggerty wrote:\n\n> I totally believe you that some of the variations that I listed in my\n> commentary don't work in the git context.  I'm not a prove expert; I\n> just noticed that removing the .prove file is counterproductive and\n> breaks some other prove features.  I also agree with you that it\n> would be dangerous to encourage partial testing and that it is\n> therefore not a priority to make the use case that you mentioned work\n> in the git context.\n> \n> I still think my patch makes sense.  The error that Peff pointed out\n> was in my commentary, not in the patch itself or in the log message.\n> Junio, is there something else keeping you from applying this patch?\n\nYeah, I hope my comments weren't interpreted as \"don't apply this\".\nKeeping the .prove file around is a prerequisite for lots of clever\nthings, including some useful (--state=slow) and some less so\n(--state=failed). But if the latter case does not work (if we even want\nit to), it is because this patch is only the first building block. We\nshould definitely apply it.\n\n-Peff\n"},{"id":"191772","messageId":"7vvcjqa6w6.fsf@alter.siamese.dyndns.org","threadId":"30397","inReplyTo":"4FB7B429.80805@alum.mit.edu","subject":"Re: [PATCH] t/Makefile: retain cache t/.prove across prove runs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-20T21:18:17Z","receivedAt":"2012-05-20T21:18:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> I still think my patch makes sense.  The error that Peff pointed out\n> was in my commentary, not in the patch itself or in the log message.\n> Junio, is there something else keeping you from applying this patch?\n\nNo.\n\nI was waiting just in case if anything other than \"Ah, that does indeed\nnot work but it does not mean this patch is wrong\" comes out (e.g. \"With\nthe patch it does not work as pointed out, but I made it also work by this\nreroll, which also is correct\") to avoid the trouble of applying and\nreplacing.\n\nThanks.\n"}]}