{"thread":{"id":"42741","subject":"[PATCH] t/Makefile: add a rule to re-run previously-failed tests","startedAt":"2016-06-29T07:02:49Z","lastAt":"2017-01-30T15:35:47Z","messageCount":32,"participants":["Johannes Schindelin","Junio C Hamano","Jeff King","Ævar Arnfjörð Bjarmason","Sverre Rabbelier","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"290459","messageId":"b2d016e44fa04e8a318967c43762d6933faf7956.1467183740.git.johannes.schindelin@gmx.de","threadId":"42741","inReplyTo":null,"subject":"[PATCH] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T07:02:37Z","receivedAt":"2016-06-29T07:02:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"While developing patch series, it is a good practice to run the test\nsuite from time to time, just to make sure that obvious bugs are caught\nearly. With complex patch series, it is common to run `make -j15 -k\ntest`, i.e. run the tests in parallel and not stop at the first failing\ntest but continue. This has the advantage of identifying possibly\nmultiple problems without having to wait for the complete test suite to\nfinish.\n\nIt is particularly important to reduce the turn-around time thusly on\nWindows, where the test suite spends 45 minutes on the computer on which\nthis patch was developed.\n\nIt is the most convenient way to determine which tests failed after\nrunning the entire test suite, in parallel, to look for left-over \"trash\ndirectory.t*\" subdirectories in the t/ subdirectory.\n\nThis patch automates the process of determinig which tests failed\npreviously and re-running them; It turned out to be quite convenient\nwhen trying to squash bugs that crept in during rebases.\n\nSigned-off-by: Johannes Schindelin \n---\nPublished-As: https://github.com/dscho/git/releases/tag/failing-tests-v1\n t/Makefile | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 18e2b28..1459a7f 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -35,6 +35,8 @@ all: $(DEFAULT_TEST_TARGET)\n test: pre-clean $(TEST_LINT)\n \t$(MAKE) aggregate-results-and-cleanup\n \n+failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\\ directory.t[0-9]*)))\n+\n prove: pre-clean $(TEST_LINT)\n \t@echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n \t$(MAKE) clean-except-prove-cache\n-- \n2.9.0.118.g0e1a633\n\nbase-commit: cf4c2cfe52be5bd973a4838f73a35d3959ce2f43\n"},{"id":"290512","messageId":"xmqqy45n52xp.fsf@gitster.mtv.corp.google.com","threadId":"42741","inReplyTo":"b2d016e44fa04e8a318967c43762d6933faf7956.1467183740.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-29T17:18:26Z","receivedAt":"2016-06-29T17:19:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> While developing patch series, it is a good practice to run the test\n> suite from time to time, just to make sure that obvious bugs are caught\n> early. With complex patch series, it is common to run `make -j15 -k\n> test`, i.e. run the tests in parallel and not stop at the first failing\n> test but continue.\n\nHmmm, my tests run in parallel and do not stop at the first one\nwithout '-k'. What are we doing differently?\n\n> It is the most convenient way to determine which tests failed after\n> running the entire test suite, in parallel, to look for left-over \"trash\n> directory.t*\" subdirectories in the t/ subdirectory.\n\nGood idea, but I'd drop \"in the t/ subdirectory\" from the\ndescription.\n\n> +failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\\ directory.t[0-9]*)))\n> +\n\nThis would not work if you use --root= in GIT_TEST_OPTS, I am\nafraid.\n"},{"id":"290566","messageId":"20160630063725.GC15380@sigill.intra.peff.net","threadId":"42741","inReplyTo":"b2d016e44fa04e8a318967c43762d6933faf7956.1467183740.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-30T06:37:25Z","receivedAt":"2016-06-30T06:44:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 29, 2016 at 09:02:37AM +0200, Johannes Schindelin wrote:\n\n> It is the most convenient way to determine which tests failed after\n> running the entire test suite, in parallel, to look for left-over \"trash\n> directory.t*\" subdirectories in the t/ subdirectory.\n\nAs Junio noted, this doesn't work with --root. I have sometimes used:\n\n grep 'failed [^0]' test-results/*\n\nfor this purpose.\n\n> This patch automates the process of determinig which tests failed\n> previously and re-running them; It turned out to be quite convenient\n> when trying to squash bugs that crept in during rebases.\n\nI suspect your response will be \"perl tools on Windows are too painful\nto use\", but the \"prove\" tool which comes with perl can do this and more\n(e.g., running the failed tests first, and then following up with the\nothers to double-check), and our test suite supports it quite well.\n\n $ grep -B1 PROVE config.mak\n # run tests in parallel, with slow ones first to keep pipelines full\n GIT_PROVE_OPTS = -j16 --state=slow,save\n\n $ cd t\n $ make prove\n ... reports some test failed ...\n $ prove --state=failed\n ... re-runs just the failed test ...\n\n-Peff\n"},{"id":"290664","messageId":"alpine.DEB.2.20.1607011551550.12947@virtualbox","threadId":"42741","inReplyTo":"xmqqy45n52xp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-01T13:57:42Z","receivedAt":"2016-07-01T13:57:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 29 Jun 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > While developing patch series, it is a good practice to run the test\n> > suite from time to time, just to make sure that obvious bugs are caught\n> > early. With complex patch series, it is common to run `make -j15 -k\n> > test`, i.e. run the tests in parallel and not stop at the first failing\n> > test but continue.\n> \n> Hmmm, my tests run in parallel and do not stop at the first one\n> without '-k'. What are we doing differently?\n\nProbably none of your tests are failing... When I run tests with -j15 and\nwithout -k, as soon as *any* test fails, the other 14 jobs stop after\nrunning their respective current tests.\n\nThis is particularly annoying when some early test fails and a subsequent\ntest run reveals that *also* one of those pesky SVN tests failed.\n\n> > It is the most convenient way to determine which tests failed after\n> > running the entire test suite, in parallel, to look for left-over \"trash\n> > directory.t*\" subdirectories in the t/ subdirectory.\n> \n> Good idea, but I'd drop \"in the t/ subdirectory\" from the\n> description.\n\nOkay.\n\n> > +failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\\ directory.t[0-9]*)))\n> > +\n> \n> This would not work if you use --root= in GIT_TEST_OPTS, I am\n> afraid.\n\nBah. You're correct. Would it be okay with you if I simply punted, like\nthis:\n\nifeq (,$(findstring --root,$(GIT_TEST_OPTS)))\nfailed: ...\nelse\nfailed:\n\techo \"Sorry, the 'failed' rule is incompatible with --root=...\" >&2\nendif\n\n? I really do not have time to spend more time on this right now...\n\nCiao,\nDscho\n"},{"id":"290665","messageId":"alpine.DEB.2.20.1607011558010.12947@virtualbox","threadId":"42741","inReplyTo":"20160630063725.GC15380@sigill.intra.peff.net","subject":"Re: [PATCH] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-01T14:00:59Z","receivedAt":"2016-07-01T14:01:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Thu, 30 Jun 2016, Jeff King wrote:\n\n> On Wed, Jun 29, 2016 at 09:02:37AM +0200, Johannes Schindelin wrote:\n> \n> > It is the most convenient way to determine which tests failed after\n> > running the entire test suite, in parallel, to look for left-over \"trash\n> > directory.t*\" subdirectories in the t/ subdirectory.\n> \n> As Junio noted, this doesn't work with --root. I have sometimes used:\n> \n> grep 'failed [^0]' test-results/*\n> \n> for this purpose.\n\nTrue, I could also do that. Looking for directories rather than spawning a\nfull-fledged grep is more light-weight, though.\n\n> > This patch automates the process of determinig which tests failed\n> > previously and re-running them; It turned out to be quite convenient\n> > when trying to squash bugs that crept in during rebases.\n> \n> I suspect your response will be \"perl tools on Windows are too painful\n> to use\", but the \"prove\" tool which comes with perl can do this and more\n> (e.g., running the failed tests first, and then following up with the\n> others to double-check), and our test suite supports it quite well.\n\nIt will surprise you to learn that I did use `prove` extensively. There\nhave been enough problems with it, though, that I stopped it.\n\nModern Windows does not have too many problems with it, but it appears as\nif Windows Server 2008 R2 (which I used for quite some time for my\nprincipal development) requires too many work-arounds for Perl to work\nreliably so that every once in a while, `prove` hangs without any real\nreason.\n\nThat is when I stopped using it.\n\nCiao,\nDscho\n"},{"id":"300459","messageId":"0dfa96b17edfe84ba19c7e57fe0b017c77943e0c.1472478285.git.johannes.schindelin@gmx.de","threadId":"42741","inReplyTo":"b2d016e44fa04e8a318967c43762d6933faf7956.1467183740.git.johannes.schindelin@gmx.de","subject":"[PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-29T13:46:05Z","receivedAt":"2016-08-29T13:46:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"While developing patch series, it is a good practice to run the test\nsuite from time to time, just to make sure that obvious bugs are caught\nearly. With complex patch series, it is common to run `make -j15 -k\ntest`, i.e. run the tests in parallel and *not* stop at the first\nfailing test but continue. This has the advantage of identifying\npossibly multiple problems in one big test run.\n\nIt is particularly important to reduce the turn-around time thusly on\nWindows, where the test suite spends 45 minutes on the computer on which\nthis patch was developed.\n\nIt is the most convenient way to determine which tests failed after\nrunning the entire test suite, in parallel, to look for left-over \"trash\ndirectory.t*\" subdirectories in the t/ subdirectory. However, as was\npointed out by Jeff King, those directories might live outside t/ when\noverridden using the --root= option, to which the Makefile\nhas no access. The next best method is to grep explicitly for failed\ntests in the test-results/ directory, which the Makefile *can* access.\n\nThis patch automates the process of determinig which tests failed\npreviously and re-running them.\n\nNote that we need to be careful to inspect only the *newest* entries in\ntest-results/: this directory contains files of the form\ntNNNN--.counts and is only removed wholesale when running the\n*entire* test suite, not when running individual tests. We ensure that\nwith a little sed magic on `ls -t`'s output that simply skips lines\nwhen the file name was seen earlier.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tThe patch is unfortunately no longer as trivial as before, but it\n\tnow also works with --root=..., i.e. when the user overrode the\n\tlocation of the trash directories.\n\nPublished-As: https://github.com/dscho/git/releases/tag/failing-tests-v2\nFetch-It-Via: git fetch https://github.com/dscho/git failing-tests-v2\nInterdiff vs v1:\n\n diff --git a/t/Makefile b/t/Makefile\n index c402a9ec..8aa6a72 100644\n --- a/t/Makefile\n +++ b/t/Makefile\n @@ -35,7 +35,12 @@ all: $(DEFAULT_TEST_TARGET)\n test: pre-clean $(TEST_LINT)\n \t$(MAKE) aggregate-results-and-cleanup\n \n -failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\\ directory.t[0-9]*)))\n +failed:\n +\t@failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n +\t\tgrep -l '^failed [1-9]' $$(ls -t *.counts | \\\n +\t\t\tsed 'G;h;/^\\(t[^.]*\\)-[0-9]*\\..*\\n\\1-[0-9]*\\./d;P;d') | \\\n +\t\tsed -n 's/-[0-9]*\\.counts$$/.sh/p') && \\\n +\ttest -z \"$$failed\" || $(MAKE) $$failed\n \n prove: pre-clean $(TEST_LINT)\n \t@echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n\n\n t/Makefile | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex d613935..8aa6a72 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -35,6 +35,13 @@ all: $(DEFAULT_TEST_TARGET)\n test: pre-clean $(TEST_LINT)\n \t$(MAKE) aggregate-results-and-cleanup\n \n+failed:\n+\t@failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n+\t\tgrep -l '^failed [1-9]' $$(ls -t *.counts | \\\n+\t\t\tsed 'G;h;/^\\(t[^.]*\\)-[0-9]*\\..*\\n\\1-[0-9]*\\./d;P;d') | \\\n+\t\tsed -n 's/-[0-9]*\\.counts$$/.sh/p') && \\\n+\ttest -z \"$$failed\" || $(MAKE) $$failed\n+\n prove: pre-clean $(TEST_LINT)\n \t@echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n \t$(MAKE) clean-except-prove-cache\n-- \n2.10.0.rc1.114.g2bd6b38\n\nbase-commit: d5cb9cbd64165153a318e1049f8bf14b09a16b11\n"},{"id":"300544","messageId":"20160830084357.rdmt2ehngrz6rqaq@sigill.intra.peff.net","threadId":"42741","inReplyTo":"0dfa96b17edfe84ba19c7e57fe0b017c77943e0c.1472478285.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-30T08:43:57Z","receivedAt":"2016-08-30T08:44:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 29, 2016 at 03:46:05PM +0200, Johannes Schindelin wrote:\n\n> Note that we need to be careful to inspect only the *newest* entries in\n> test-results/: this directory contains files of the form\n> tNNNN--.counts and is only removed wholesale when running the\n> *entire* test suite, not when running individual tests. We ensure that\n> with a little sed magic on `ls -t`'s output that simply skips lines\n> when the file name was seen earlier.\n\nHmm, interesting. Your approach seems reasonable, but I have to wonder\nif writing the pid in the first place is sane.\n\nI started to write up my reasoning in this email, but realized it was\nrapidly becoming the content of a commit message. So here is that\ncommit.\n\n-- >8 --\nSubject: [PATCH] test-lib: drop PID from test-results/*.count\n\nEach test run generates a \"count\" file in t/test-results\nthat stores the number of successful, failed, etc tests.\nIf you run \"t1234-foo.sh\", that file is named as\n\"t/test-results/t1234-foo-$$.count\"\n\nThe addition of the PID there is serving no purpose, and\nmakes analysis of the count files harder.\n\nThe presence of the PID dates back to 2d84e9f (Modify\ntest-lib.sh to output stats to t/test-results/*,\n2008-06-08), but no reasoning is given there. Looking at the\ncurrent code, we can see that other files we write to\ntest-results (like *.exit and *.out) do _not_ have the PID\nincluded. So the presence of the PID does not meaningfully\nallow one to store the results from multiple runs anyway.\n\nMoreover, anybody wishing to read the *.count files to\naggregate results has to deal with the presence of multiple\nfiles for a given test (and figure out which one is the most\nrecent based on their timestamps!). The only consumer of\nthese files is the aggregate.sh script, which arguably gets\nthis wrong. If a test is run multiple times, its counts will\nappear multiple times in the total (I say arguably only\nbecause the desired semantics aren't documented anywhere,\nbut I have trouble seeing how this behavior could be\nuseful).\n\nSo let's just drop the PID, which fixes aggregate.sh, and\nwill make new features based around the count files easier\nto write.\n\nNote that since the count-file may already exist (when\nre-running a test), we also switch the \"cat\" from appending\nto truncating. The use of append here was pointless in the\nfirst place, as we expected to always write to a unique file.\n\nSigned-off-by: Jeff King \n---\nThe presence of the append, combined with the way aggregate.sh is\nwritten makes me wonder if the intent was to store multiple run results\nfor each test in a single file (and aggregate would just report the last\none). Which _still_ makes the use of the PID wrong. But again, I don't\nsee much use for it.\n\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex d731d66..eada492 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -687,9 +687,9 @@ test_done () {\n \t\ttest_results_dir=\"$TEST_OUTPUT_DIRECTORY/test-results\"\n \t\tmkdir -p \"$test_results_dir\"\n \t\tbase=${0##*/}\n-\t\ttest_results_path=\"$test_results_dir/${base%.sh}-$$.counts\"\n+\t\ttest_results_path=\"$test_results_dir/${base%.sh}.counts\"\n \n-\t\tcat >>\"$test_results_path\" <<-EOF\n+\t\tcat >\"$test_results_path\" <<-EOF\n \t\ttotal $test_count\n \t\tsuccess $test_success\n \t\tfixed $test_fixed\n-- \n2.10.0.rc2.123.ga991f9e\n\n"},{"id":"300599","messageId":"xmqq37lm3w6u.fsf@gitster.mtv.corp.google.com","threadId":"42741","inReplyTo":"20160830084357.rdmt2ehngrz6rqaq@sigill.intra.peff.net","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-30T19:15:21Z","receivedAt":"2016-08-30T19:15:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Hmm, interesting. Your approach seems reasonable, but I have to wonder\n> if writing the pid in the first place is sane.\n>\n> I started to write up my reasoning in this email, but realized it was\n> rapidly becoming the content of a commit message. So here is that\n> commit.\n\nSounds sensible; if this makes Dscho's \"which ones failed in the\nprevious run\" simpler, that is even better ;-)\n\n"},{"id":"300607","messageId":"CACBZZX6iEmbb68tzRKNAryp5qmt=iU9FMuOe2ONV=2ojcazoEg@mail.gmail.com","threadId":"42741","inReplyTo":"0dfa96b17edfe84ba19c7e57fe0b017c77943e0c.1472478285.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-08-30T20:48:19Z","receivedAt":"2016-08-30T20:50:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Aug 29, 2016 at 3:46 PM, Johannes Schindelin\n wrote:\n> While developing patch series, it is a good practice to run the test\n> suite from time to time, just to make sure that obvious bugs are caught\n> early. With complex patch series, it is common to run `make -j15 -k\n> test`, i.e. run the tests in parallel and *not* stop at the first\n> failing test but continue. This has the advantage of identifying\n> possibly multiple problems in one big test run.\n>\n> It is particularly important to reduce the turn-around time thusly on\n> Windows, where the test suite spends 45 minutes on the computer on which\n> this patch was developed.\n>\n> It is the most convenient way to determine which tests failed after\n> running the entire test suite, in parallel, to look for left-over \"trash\n> directory.t*\" subdirectories in the t/ subdirectory. However, as was\n> pointed out by Jeff King, those directories might live outside t/ when\n> overridden using the --root= option, to which the Makefile\n> has no access. The next best method is to grep explicitly for failed\n> tests in the test-results/ directory, which the Makefile *can* access.\n>\n> This patch automates the process of determinig which tests failed\n> previously and re-running them.\n>\n> Note that we need to be careful to inspect only the *newest* entries in\n> test-results/: this directory contains files of the form\n> tNNNN--.counts and is only removed wholesale when running the\n> *entire* test suite, not when running individual tests. We ensure that\n> with a little sed magic on `ls -t`'s output that simply skips lines\n> when the file name was seen earlier.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n>\n> The patch is unfortunately no longer as trivial as before, but it\n> now also works with --root=..., i.e. when the user overrode the\n> location of the trash directories.\n>\n> Published-As: https://github.com/dscho/git/releases/tag/failing-tests-v2\n> Fetch-It-Via: git fetch https://github.com/dscho/git failing-tests-v2\n> Interdiff vs v1:\n>\n> diff --git a/t/Makefile b/t/Makefile\n> index c402a9ec..8aa6a72 100644\n> --- a/t/Makefile\n> +++ b/t/Makefile\n> @@ -35,7 +35,12 @@ all: $(DEFAULT_TEST_TARGET)\n> test: pre-clean $(TEST_LINT)\n> $(MAKE) aggregate-results-and-cleanup\n>\n> -failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\\ directory.t[0-9]*)))\n> +failed:\n> + @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n> + grep -l '^failed [1-9]' $$(ls -t *.counts | \\\n> + sed 'G;h;/^\\(t[^.]*\\)-[0-9]*\\..*\\n\\1-[0-9]*\\./d;P;d') | \\\n> + sed -n 's/-[0-9]*\\.counts$$/.sh/p') && \\\n> + test -z \"$$failed\" || $(MAKE) $$failed\n>\n> prove: pre-clean $(TEST_LINT)\n> @echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n\nI don't at all mind this solution to the problem, if it works for that's cool.\n\nBut FWIW something you may have missed is that you can just use\nprove(1) for this, which is why I initially patched git.git to support\nTAP, so I didn't have to implement stuff like this.\n\nI.e.:\n\n $ prove --state=save t[0-9]*.sh\n $ prove --state=failed,save t[0-9]*.sh\n\nDoes exactly what you're trying to do here with existing tools we support.\n\nI.e. your new target could just be implemented in terms of calling prove.\n\nCheck out its man page, it may have other stuff you want to use, e.g.\nyou can run the slowest tests first etc.\n"},{"id":"300608","messageId":"20160830205151.k6ufhfzl6gh4uuog@sigill.intra.peff.net","threadId":"42741","inReplyTo":"CACBZZX6iEmbb68tzRKNAryp5qmt=iU9FMuOe2ONV=2ojcazoEg@mail.gmail.com","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-30T20:51:52Z","receivedAt":"2016-08-30T20:52:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 30, 2016 at 10:48:19PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > -failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\\ directory.t[0-9]*)))\n> > +failed:\n> > + @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n> > + grep -l '^failed [1-9]' $$(ls -t *.counts | \\\n> > + sed 'G;h;/^\\(t[^.]*\\)-[0-9]*\\..*\\n\\1-[0-9]*\\./d;P;d') | \\\n> > + sed -n 's/-[0-9]*\\.counts$$/.sh/p') && \\\n> > + test -z \"$$failed\" || $(MAKE) $$failed\n> >\n> > prove: pre-clean $(TEST_LINT)\n> > @echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n> \n> I don't at all mind this solution to the problem, if it works for that's cool.\n> \n> But FWIW something you may have missed is that you can just use\n> prove(1) for this, which is why I initially patched git.git to support\n> TAP, so I didn't have to implement stuff like this.\n\nHeh. I think each iteration of this patch will be destined to have\nsomebody[1] point Johannes at prove. ;)\n\n(But I really do recommend prove if you can use it).\n\n-Peff\n\n[1] http://public-inbox.org/git/20160630063725.GC15380@sigill.intra.peff.net/\n"},{"id":"300609","messageId":"CACBZZX4NyjkK0Nf1JVGFRhc0xnLYg2YX6ctO5OxK3Pi60r5KaA@mail.gmail.com","threadId":"42741","inReplyTo":"20160830205151.k6ufhfzl6gh4uuog@sigill.intra.peff.net","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-08-30T20:58:30Z","receivedAt":"2016-08-30T20:58:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Aug 30, 2016 at 10:51 PM, Jeff King wrote:\n> On Tue, Aug 30, 2016 at 10:48:19PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > -failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\\ directory.t[0-9]*)))\n>> > +failed:\n>> > + @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n>> > + grep -l '^failed [1-9]' $$(ls -t *.counts | \\\n>> > + sed 'G;h;/^\\(t[^.]*\\)-[0-9]*\\..*\\n\\1-[0-9]*\\./d;P;d') | \\\n>> > + sed -n 's/-[0-9]*\\.counts$$/.sh/p') && \\\n>> > + test -z \"$$failed\" || $(MAKE) $$failed\n>> >\n>> > prove: pre-clean $(TEST_LINT)\n>> > @echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n>>\n>> I don't at all mind this solution to the problem, if it works for that's cool.\n>>\n>> But FWIW something you may have missed is that you can just use\n>> prove(1) for this, which is why I initially patched git.git to support\n>> TAP, so I didn't have to implement stuff like this.\n>\n> Heh. I think each iteration of this patch will be destined to have\n> somebody[1] point Johannes at prove. ;)\n>\n> (But I really do recommend prove if you can use it).\n>\n> -Peff\n>\n> [1] http://public-inbox.org/git/20160630063725.GC15380@sigill.intra.peff.net/\n\nSorry about that, I see it's been mentioned already. My only excuse is\nthat I don't know how to operate my E-Mail client :)\n"},{"id":"300671","messageId":"alpine.DEB.2.20.1608311227150.129229@virtualbox","threadId":"42741","inReplyTo":"CACBZZX4NyjkK0Nf1JVGFRhc0xnLYg2YX6ctO5OxK3Pi60r5KaA@mail.gmail.com","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-31T10:29:12Z","receivedAt":"2016-08-31T10:29:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Tue, 30 Aug 2016, Ævar Arnfjörð Bjarmason wrote:\n\n> On Tue, Aug 30, 2016 at 10:51 PM, Jeff King wrote:\n> > On Tue, Aug 30, 2016 at 10:48:19PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> >\n> >> > -failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\\ directory.t[0-9]*)))\n> >> > +failed:\n> >> > + @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n> >> > + grep -l '^failed [1-9]' $$(ls -t *.counts | \\\n> >> > + sed 'G;h;/^\\(t[^.]*\\)-[0-9]*\\..*\\n\\1-[0-9]*\\./d;P;d') | \\\n> >> > + sed -n 's/-[0-9]*\\.counts$$/.sh/p') && \\\n> >> > + test -z \"$$failed\" || $(MAKE) $$failed\n> >> >\n> >> > prove: pre-clean $(TEST_LINT)\n> >> > @echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n> >>\n> >> I don't at all mind this solution to the problem, if it works for that's cool.\n> >>\n> >> But FWIW something you may have missed is that you can just use\n> >> prove(1) for this, which is why I initially patched git.git to support\n> >> TAP, so I didn't have to implement stuff like this.\n> >\n> > Heh. I think each iteration of this patch will be destined to have\n> > somebody[1] point Johannes at prove. ;)\n> >\n> > (But I really do recommend prove if you can use it).\n> >\n> > -Peff\n> >\n> > [1] http://public-inbox.org/git/20160630063725.GC15380@sigill.intra.peff.net/\n> \n> Sorry about that, I see it's been mentioned already.\n\nYeah, it is true that prove(1) would be able to help. If it worked\nreliably on Windows. (Probably Perl's fault, not prove's.)\n\n> My only excuse is that I don't know how to operate my E-Mail client :)\n\nBut we use email to discuss all things Git because it makes everything so\neasy and convenient... ;-)\n\nCiao,\nDscho"},{"id":"300672","messageId":"alpine.DEB.2.20.1608311233440.129229@virtualbox","threadId":"42741","inReplyTo":"xmqq37lm3w6u.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-31T10:36:05Z","receivedAt":"2016-08-31T10:38:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\n[Sverre: we are considering to remove the - suffix in test-results/,\nsee more below.]\n\nOn Tue, 30 Aug 2016, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Hmm, interesting. Your approach seems reasonable, but I have to wonder\n> > if writing the pid in the first place is sane.\n> >\n> > I started to write up my reasoning in this email, but realized it was\n> > rapidly becoming the content of a commit message. So here is that\n> > commit.\n> \n> Sounds sensible; if this makes Dscho's \"which ones failed in the\n> previous run\" simpler, that is even better ;-)\n\nI did not have the time to dig further before now. There must have been a\ngood reason why we append the PID.\n\nSverre, you added that code in 2d84e9f (Modify test-lib.sh to output stats\nto t/test-results/*, 2008-06-08): any idea why the - suffix was\nneeded?\n\nCiao,\nDscho\n\n"},{"id":"300690","messageId":"CACBZZX6exynt_9_wVtEN19HQt_rPJdo5Ck3jujdQ-hLdMAGdmg@mail.gmail.com","threadId":"42741","inReplyTo":"alpine.DEB.2.20.1608311227150.129229@virtualbox","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-08-31T13:42:04Z","receivedAt":"2016-08-31T13:42:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Aug 31, 2016 at 12:29 PM, Johannes Schindelin\n wrote:\n> Hi Ævar,\n>\n> On Tue, 30 Aug 2016, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Tue, Aug 30, 2016 at 10:51 PM, Jeff King wrote:\n>> > On Tue, Aug 30, 2016 at 10:48:19PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> >\n>> >> > -failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\\ directory.t[0-9]*)))\n>> >> > +failed:\n>> >> > + @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n>> >> > + grep -l '^failed [1-9]' $$(ls -t *.counts | \\\n>> >> > + sed 'G;h;/^\\(t[^.]*\\)-[0-9]*\\..*\\n\\1-[0-9]*\\./d;P;d') | \\\n>> >> > + sed -n 's/-[0-9]*\\.counts$$/.sh/p') && \\\n>> >> > + test -z \"$$failed\" || $(MAKE) $$failed\n>> >> >\n>> >> > prove: pre-clean $(TEST_LINT)\n>> >> > @echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n>> >>\n>> >> I don't at all mind this solution to the problem, if it works for that's cool.\n>> >>\n>> >> But FWIW something you may have missed is that you can just use\n>> >> prove(1) for this, which is why I initially patched git.git to support\n>> >> TAP, so I didn't have to implement stuff like this.\n>> >\n>> > Heh. I think each iteration of this patch will be destined to have\n>> > somebody[1] point Johannes at prove. ;)\n>> >\n>> > (But I really do recommend prove if you can use it).\n>> >\n>> > -Peff\n>> >\n>> > [1] http://public-inbox.org/git/20160630063725.GC15380@sigill.intra.peff.net/\n>>\n>> Sorry about that, I see it's been mentioned already.\n>\n> Yeah, it is true that prove(1) would be able to help. If it worked\n> reliably on Windows. (Probably Perl's fault, not prove's.)\n\nI haven't used it myself (or any Windows thing) but people say good\nthings about http://strawberryperl.com\n"},{"id":"300693","messageId":"alpine.DEB.2.20.1608311702440.129229@virtualbox","threadId":"42741","inReplyTo":"CACBZZX6exynt_9_wVtEN19HQt_rPJdo5Ck3jujdQ-hLdMAGdmg@mail.gmail.com","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-31T15:05:31Z","receivedAt":"2016-08-31T15:05:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Wed, 31 Aug 2016, Ævar Arnfjörð Bjarmason wrote:\n\n> I haven't used it myself (or any Windows thing) but people say good\n> things about http://strawberryperl.com\n\nAh yes. This comes up frequently. Many a Git for Windows user pointed me\ninto that direction.\n\nThe biggest problem with Strawberry Perl is that it is virtually\nimpossible to build the Subversion-Perl bindings using the Git for Windows\nSDK when using Strawberry Perl.\n\nWhich pretty much precludes it from being used in Git for Windows.\n\nAnd then there are the path issues... Git's Perl scripts are pretty\ncertain that they live in a POSIX-y environment. Which MSYS2 Perl\nprovides. Strawberry Perl not.\n\nCiao,\nJohannes"},{"id":"300766","messageId":"CAGdFq_iJeziyXBPL2GVHNXZcjGAwQVN2EhJs4AtJCSx7ghn32Q@mail.gmail.com","threadId":"42741","inReplyTo":"alpine.DEB.2.20.1608311233440.129229@virtualbox","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2016-09-01T03:59:50Z","receivedAt":"2016-09-01T04:00:37Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Wed, Aug 31, 2016 at 3:36 AM Johannes Schindelin\n wrote:\n> On Tue, 30 Aug 2016, Junio C Hamano wrote:\n> > Jeff King writes:\n> > > Hmm, interesting. Your approach seems reasonable, but I have to wonder\n> > > if writing the pid in the first place is sane.\n> > >\n> > > I started to write up my reasoning in this email, but realized it was\n> > > rapidly becoming the content of a commit message. So here is that\n> > > commit.\n> >\n> > Sounds sensible; if this makes Dscho's \"which ones failed in the\n> > previous run\" simpler, that is even better ;-)\n>\n> I did not have the time to dig further before now. There must have been a\n> good reason why we append the PID.\n>\n> Sverre, you added that code in 2d84e9f (Modify test-lib.sh to output stats\n> to t/test-results/*, 2008-06-08): any idea why the - suffix was\n> needed?\n\nI can't really recall, but I think it may have been related to me\ndoing something like this:\n1. Make a change, and start running tests (this takes a long time)\n2. Notice a failure, start fixing it, leave tests running to find\nfurther failures\n3. Finish fix, first tests are still running, start another run in a\nnew terminal (possibly of just the one failed test I was fixing) to\nsee if the fix worked.\n\nWithout the pid, the second run would clobber the results from the first run.\n\n\nIf only past-me was more rigorous about writing good commit messages :P.\n"},{"id":"300781","messageId":"alpine.DEB.2.20.1609011027210.129229@virtualbox","threadId":"42741","inReplyTo":"CAGdFq_iJeziyXBPL2GVHNXZcjGAwQVN2EhJs4AtJCSx7ghn32Q@mail.gmail.com","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-01T08:27:53Z","receivedAt":"2016-09-01T08:28:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Sverre,\n\nOn Wed, 31 Aug 2016, Sverre Rabbelier wrote:\n\n> On Wed, Aug 31, 2016 at 3:36 AM Johannes Schindelin\n> wrote:\n> > On Tue, 30 Aug 2016, Junio C Hamano wrote:\n> > > Jeff King writes:\n> > > > Hmm, interesting. Your approach seems reasonable, but I have to wonder\n> > > > if writing the pid in the first place is sane.\n> > > >\n> > > > I started to write up my reasoning in this email, but realized it was\n> > > > rapidly becoming the content of a commit message. So here is that\n> > > > commit.\n> > >\n> > > Sounds sensible; if this makes Dscho's \"which ones failed in the\n> > > previous run\" simpler, that is even better ;-)\n> >\n> > I did not have the time to dig further before now. There must have been a\n> > good reason why we append the PID.\n> >\n> > Sverre, you added that code in 2d84e9f (Modify test-lib.sh to output stats\n> > to t/test-results/*, 2008-06-08): any idea why the - suffix was\n> > needed?\n> \n> I can't really recall, but I think it may have been related to me\n> doing something like this:\n> 1. Make a change, and start running tests (this takes a long time)\n> 2. Notice a failure, start fixing it, leave tests running to find\n> further failures\n> 3. Finish fix, first tests are still running, start another run in a\n> new terminal (possibly of just the one failed test I was fixing) to\n> see if the fix worked.\n> \n> Without the pid, the second run would clobber the results from the first run.\n> \n> \n> If only past-me was more rigorous about writing good commit messages :P.\n\n:-)\n\nWould present-you disagree with stripping off the - suffix, based on\nyour recollections?\n\nCiao,\nDscho\n"},{"id":"300844","messageId":"CAGdFq_h3UuW7wX0-=SuS22mX_C086HRZZ=i1sYVya80dd+qMYQ@mail.gmail.com","threadId":"42741","inReplyTo":"alpine.DEB.2.20.1609011027210.129229@virtualbox","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2016-09-01T16:57:04Z","receivedAt":"2016-09-01T22:11:30Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Thu, Sep 1, 2016 at 1:27 AM, Johannes Schindelin\n wrote:\n> On Wed, 31 Aug 2016, Sverre Rabbelier wrote:\n>> On Wed, Aug 31, 2016 at 3:36 AM Johannes Schindelin\n>> wrote:\n>> > On Tue, 30 Aug 2016, Junio C Hamano wrote:\n>> > > Jeff King writes:\n>> > > > Hmm, interesting. Your approach seems reasonable, but I have to wonder\n>> > > > if writing the pid in the first place is sane.\n>> > > >\n>> > > > I started to write up my reasoning in this email, but realized it was\n>> > > > rapidly becoming the content of a commit message. So here is that\n>> > > > commit.\n>> > >\n>> > > Sounds sensible; if this makes Dscho's \"which ones failed in the\n>> > > previous run\" simpler, that is even better ;-)\n>> >\n>> > I did not have the time to dig further before now. There must have been a\n>> > good reason why we append the PID.\n>> >\n>> > Sverre, you added that code in 2d84e9f (Modify test-lib.sh to output stats\n>> > to t/test-results/*, 2008-06-08): any idea why the - suffix was\n>> > needed?\n>>\n>> I can't really recall, but I think it may have been related to me\n>> doing something like this:\n>> 1. Make a change, and start running tests (this takes a long time)\n>> 2. Notice a failure, start fixing it, leave tests running to find\n>> further failures\n>> 3. Finish fix, first tests are still running, start another run in a\n>> new terminal (possibly of just the one failed test I was fixing) to\n>> see if the fix worked.\n>>\n>> Without the pid, the second run would clobber the results from the first run.\n>>\n>>\n>> If only past-me was more rigorous about writing good commit messages :P.\n>\n> :-)\n>\n> Would present-you disagree with stripping off the - suffix, based on\n> your recollections?\n\nNo objections, I think it should be fine. If anyone uncovers a\nparticularly compelling reason later on, it's only a commit away :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"300853","messageId":"xmqqzinrteql.fsf@gitster.mtv.corp.google.com","threadId":"42741","inReplyTo":"CAGdFq_h3UuW7wX0-=SuS22mX_C086HRZZ=i1sYVya80dd+qMYQ@mail.gmail.com","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-01T22:52:18Z","receivedAt":"2016-09-01T22:52:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n>>> I can't really recall, but I think it may have been related to me\n>>> doing something like this:\n>>> 1. Make a change, and start running tests (this takes a long time)\n>>> 2. Notice a failure, start fixing it, leave tests running to find\n>>> further failures\n>>> 3. Finish fix, first tests are still running, start another run in a\n>>> new terminal (possibly of just the one failed test I was fixing) to\n>>> see if the fix worked.\n>>>\n>>> Without the pid, the second run would clobber the results from the first run.\n>>>\n>> Would present-you disagree with stripping off the - suffix, based on\n>> your recollections?\n>\n> No objections, I think it should be fine. If anyone uncovers a\n> particularly compelling reason later on, it's only a commit away :).\n\nOK, especially with the earlier observation made by Peff in the log\nmessage:\n\n ... we can see that other files we write to test-results (like\n *.exit and *.out) do _not_ have the PID included. So the\n presence of the PID does not meaningfully allow one to store the\n results from multiple runs anyway.\n\neven if we wanted to, keeping the current code with suffix is not\nsufficient, so I suspect it won't be just \"a commit\" away, but we\nshould be able to lose it for now. Hopefully that would help making\nDscho's \"what are the failed tests?\" logic simpler.\n\nThanks.\n\n"},{"id":"300868","messageId":"alpine.DEB.2.20.1609020933430.129229@virtualbox","threadId":"42741","inReplyTo":"xmqqzinrteql.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-02T07:35:14Z","receivedAt":"2016-09-02T07:35:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 1 Sep 2016, Junio C Hamano wrote:\n\n> Hopefully that [patch removing the - suffix] would help making\n> Dscho's \"what are the failed tests?\" logic simpler.\n\nOf course.\n\nIt also makes sure that those 2 hours I spent on writing and perfecting\nthe sed magic were spent in vain... ;-)\n\nCiao,\nDscho\n"},{"id":"300879","messageId":"CACBZZX56fjJZydnBrWUYtU6V3xyQyaLL4MYzVVF0yD4dRdducw@mail.gmail.com","threadId":"42741","inReplyTo":"alpine.DEB.2.20.1608311702440.129229@virtualbox","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-09-02T10:25:21Z","receivedAt":"2016-09-02T10:25:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Aug 31, 2016 at 5:05 PM, Johannes Schindelin\n wrote:\n> Hi Ævar,\n>\n> On Wed, 31 Aug 2016, Ævar Arnfjörð Bjarmason wrote:\n>\n>> I haven't used it myself (or any Windows thing) but people say good\n>> things about http://strawberryperl.com\n>\n> Ah yes. This comes up frequently. Many a Git for Windows user pointed me\n> into that direction.\n>\n> The biggest problem with Strawberry Perl is that it is virtually\n> impossible to build the Subversion-Perl bindings using the Git for Windows\n> SDK when using Strawberry Perl.\n>\n> Which pretty much precludes it from being used in Git for Windows.\n>\n> And then there are the path issues... Git's Perl scripts are pretty\n> certain that they live in a POSIX-y environment. Which MSYS2 Perl\n> provides. Strawberry Perl not.\n\nThis might be me missing the point, and I'm really just trying to be\nhelpful here and make \"prove\" work for you because it's awesome, but\nas far as just you running this for development purposes does any of\nthis SVN stuff matter? I.e. you can build Git itself not with\nStrawberry, but just use Strawberry to get a working copy of \"prove\".\n"},{"id":"300882","messageId":"alpine.DEB.2.20.1609021406340.129229@virtualbox","threadId":"42741","inReplyTo":"CACBZZX56fjJZydnBrWUYtU6V3xyQyaLL4MYzVVF0yD4dRdducw@mail.gmail.com","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-02T12:08:06Z","receivedAt":"2016-09-02T12:08:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Fri, 2 Sep 2016, Ævar Arnfjörð Bjarmason wrote:\n\n> On Wed, Aug 31, 2016 at 5:05 PM, Johannes Schindelin\n> wrote:\n>\n> > The biggest problem with Strawberry Perl is that it is virtually\n> > impossible to build the Subversion-Perl bindings using the Git for\n> > Windows SDK when using Strawberry Perl.\n> >\n> > Which pretty much precludes it from being used in Git for Windows.\n> >\n> > And then there are the path issues... Git's Perl scripts are pretty\n> > certain that they live in a POSIX-y environment. Which MSYS2 Perl\n> > provides. Strawberry Perl not.\n> \n> This might be me missing the point, and I'm really just trying to be\n> helpful here and make \"prove\" work for you because it's awesome, but\n> as far as just you running this for development purposes does any of\n> this SVN stuff matter? I.e. you can build Git itself not with\n> Strawberry, but just use Strawberry to get a working copy of \"prove\".\n\nYes, the SVN stuff matters, because of the many t9*svn* tests (which, BTW\ntake a substantial time to run). So if I run the test suite, I better do\nit with a perl.exe in the PATH that can run the SVN tests. Otherwise I\nmight just as well not bother with running the entire test suite...\n\nCiao,\nDscho"},{"id":"300898","messageId":"vpq8tva1cou.fsf@anie.imag.fr","threadId":"42741","inReplyTo":"alpine.DEB.2.20.1609021406340.129229@virtualbox","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-09-02T16:36:17Z","receivedAt":"2016-09-02T16:36:38Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Johannes Schindelin writes:\n\n> Hi Ævar,\n>\n> On Fri, 2 Sep 2016, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Wed, Aug 31, 2016 at 5:05 PM, Johannes Schindelin\n>> wrote:\n>>\n>> > The biggest problem with Strawberry Perl is that it is virtually\n>> > impossible to build the Subversion-Perl bindings using the Git for\n>> > Windows SDK when using Strawberry Perl.\n>> >\n>> > Which pretty much precludes it from being used in Git for Windows.\n>> >\n>> > And then there are the path issues... Git's Perl scripts are pretty\n>> > certain that they live in a POSIX-y environment. Which MSYS2 Perl\n>> > provides. Strawberry Perl not.\n>> \n>> This might be me missing the point, and I'm really just trying to be\n>> helpful here and make \"prove\" work for you because it's awesome, but\n>> as far as just you running this for development purposes does any of\n>> this SVN stuff matter? I.e. you can build Git itself not with\n>> Strawberry, but just use Strawberry to get a working copy of \"prove\".\n>\n> Yes, the SVN stuff matters, because of the many t9*svn* tests (which, BTW\n> take a substantial time to run). So if I run the test suite, I better do\n> it with a perl.exe in the PATH that can run the SVN tests. Otherwise I\n> might just as well not bother with running the entire test suite...\n\nMaybe something like\n\n\\path\\to\\strawberry-perl\\perl.exe \\path\\to\\prove ...\n\nwithout changing the PATH would work. I wouldn't call that convenient\nthough.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"300975","messageId":"alpine.DEB.2.20.1609040952110.129229@virtualbox","threadId":"42741","inReplyTo":"vpq8tva1cou.fsf@anie.imag.fr","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-04T07:55:39Z","receivedAt":"2016-09-04T08:02:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 2 Sep 2016, Matthieu Moy wrote:\n\n> Johannes Schindelin writes:\n> \n> > Hi Ævar,\n> >\n> > On Fri, 2 Sep 2016, Ævar Arnfjörð Bjarmason wrote:\n> >\n> >> On Wed, Aug 31, 2016 at 5:05 PM, Johannes Schindelin\n> >> wrote:\n> >>\n> >> > The biggest problem with Strawberry Perl is that it is virtually\n> >> > impossible to build the Subversion-Perl bindings using the Git for\n> >> > Windows SDK when using Strawberry Perl.\n> >> >\n> >> > Which pretty much precludes it from being used in Git for Windows.\n> >> >\n> >> > And then there are the path issues... Git's Perl scripts are pretty\n> >> > certain that they live in a POSIX-y environment. Which MSYS2 Perl\n> >> > provides. Strawberry Perl not.\n> >> \n> >> This might be me missing the point, and I'm really just trying to be\n> >> helpful here and make \"prove\" work for you because it's awesome, but\n> >> as far as just you running this for development purposes does any of\n> >> this SVN stuff matter? I.e. you can build Git itself not with\n> >> Strawberry, but just use Strawberry to get a working copy of \"prove\".\n> >\n> > Yes, the SVN stuff matters, because of the many t9*svn* tests (which, BTW\n> > take a substantial time to run). So if I run the test suite, I better do\n> > it with a perl.exe in the PATH that can run the SVN tests. Otherwise I\n> > might just as well not bother with running the entire test suite...\n> \n> Maybe something like\n> \n> \\path\\to\\strawberry-perl\\perl.exe \\path\\to\\prove ...\n> \n> without changing the PATH would work. I wouldn't call that convenient\n> though.\n\nWouldn't Perl-specific environment variables set by Strawberry Perl (such\nas PERL_PATH bleed through to the spawned child processes?\n\nWe're dancing around the issue, really. Rather than piling workaround on\nworkaround with no end in sight, I think it is time to admit that using\nprove(1) on Windows is just not a good solution for the problem to re-run\nfailed tests.\n\nCiao,\nJohannes"},{"id":"300978","messageId":"vpqoa44xbs8.fsf@anie.imag.fr","threadId":"42741","inReplyTo":"alpine.DEB.2.20.1609040952110.129229@virtualbox","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-09-04T09:19:19Z","receivedAt":"2016-09-04T09:20:36Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Johannes Schindelin writes:\n\n> Hi,\n>\n> On Fri, 2 Sep 2016, Matthieu Moy wrote:\n>\n>> Johannes Schindelin writes:\n>> \n>> > Hi Ævar,\n>> >\n>> > On Fri, 2 Sep 2016, Ævar Arnfjörð Bjarmason wrote:\n>> >\n>> >> This might be me missing the point, and I'm really just trying to be\n>> >> helpful here and make \"prove\" work for you because it's awesome, but\n>> >> as far as just you running this for development purposes does any of\n>> >> this SVN stuff matter? I.e. you can build Git itself not with\n>> >> Strawberry, but just use Strawberry to get a working copy of \"prove\".\n>> >\n>> > Yes, the SVN stuff matters, because of the many t9*svn* tests (which, BTW\n>> > take a substantial time to run). So if I run the test suite, I better do\n>> > it with a perl.exe in the PATH that can run the SVN tests. Otherwise I\n>> > might just as well not bother with running the entire test suite...\n>> \n>> Maybe something like\n>> \n>> \\path\\to\\strawberry-perl\\perl.exe \\path\\to\\prove ...\n>> \n>> without changing the PATH would work. I wouldn't call that convenient\n>> though.\n>\n> Wouldn't Perl-specific environment variables set by Strawberry Perl (such\n> as PERL_PATH bleed through to the spawned child processes?\n>\n> We're dancing around the issue, really. Rather than piling workaround on\n> workaround with no end in sight, I think it is time to admit that using\n> prove(1) on Windows is just not a good solution for the problem to re-run\n> failed tests.\n\nI didn't re-add Ævar's disclaimer, but my message was really not\nintended to be an objection to your patch, just a (not necessarily good)\nidea in case you or someone else on windows wanted to give one more\nchance to prove.\n\nI'm all for adding \"make failed\". Actually, we could even make the\nfeature more discoverable by echoing \"You may run 'make failed' to\nre-run failed tests\" at the end of the tests when one of them failed.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"301443","messageId":"xmqqy432f7wl.fsf@gitster.mtv.corp.google.com","threadId":"42741","inReplyTo":"alpine.DEB.2.20.1609020933430.129229@virtualbox","subject":"Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-08T20:34:02Z","receivedAt":"2016-09-08T20:34:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Thu, 1 Sep 2016, Junio C Hamano wrote:\n>\n>> Hopefully that [patch removing the - suffix] would help making\n>> Dscho's \"what are the failed tests?\" logic simpler.\n>\n> Of course.\n>\n> It also makes sure that those 2 hours I spent on writing and perfecting\n> the sed magic were spent in vain... ;-)\n\nWell it is either\n\n * the sed magic is so arcane that you'd need to spend a long time,\n comparable to 2 hours you already spent, if you ever need to look\n at it and figure out what it does next time you need to change\n something in it.\n\nor\n\n * you are not familiar with the sed magic and you would be able to\n write the same thing in 2 minutes next time if you need to adjust\n it when we add -pid back later.\n\nEither way, those 2 hours are not wasted.\n\nI personally fall into the former category. Any sed script that\nneeds G, h, and x together I need to spend at least 15 minutes just\nto warm myself up, as I do not work with the language that often.\n\nThanks ;-)\n\n"},{"id":"310408","messageId":"85044791cfcba35c1ad3d8138051f3f075cb0646.1485526641.git.johannes.schindelin@gmx.de","threadId":"42741","inReplyTo":"0dfa96b17edfe84ba19c7e57fe0b017c77943e0c.1472478285.git.johannes.schindelin@gmx.de","subject":"[PATCH v3] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-27T14:17:36Z","receivedAt":"2017-01-27T14:18:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This patch automates the process of determinig which tests failed\npreviously and re-running them.\n\nWhile developing patch series, it is a good practice to run the test\nsuite from time to time, just to make sure that obvious bugs are caught\nearly. With complex patch series, it is common to run `make -j15 -k\ntest`, i.e. run the tests in parallel and *not* stop at the first\nfailing test but continue. This has the advantage of identifying\npossibly multiple problems in one big test run.\n\nIt is particularly important to reduce the turn-around time thusly on\nWindows, where the test suite spends 45 minutes on the computer on which\nthis patch was developed.\n\nIt is the most convenient way to determine which tests failed after\nrunning the entire test suite, in parallel, to look for left-over \"trash\ndirectory.t*\" subdirectories in the t/ subdirectory. However, those\ndirectories might live outside t/ when overridden using the\n--root= option, to which the Makefile has no access. The next\nbest method is to grep explicitly for failed tests in the test-results/\ndirectory, which the Makefile *can* access.\n\nPlease note that the often-recommended `prove` tool requires Perl, and\nthat opens a whole new can of worms on Windows. As no native Windows Perl\ncomes with Subversion bindings, we have to use a Perl in Git for Windows\nthat uses the POSIX emulation layer named MSYS2 (which is a portable\nversion of Cygwin). When using this emulation layer under stress, e.g.\nwhen running massively-parallel tests, unexplicable crashes occur quite\nfrequently, and instead of having a solution to the original problem, the\ndeveloper now has an additional, quite huge problem. For that reason, this\ndeveloper rejected `prove` as a solution and went with this patch instead.\n\nSigned-off-by: Johannes Schindelin \n---\nPublished-As: https://github.com/dscho/git/releases/tag/failing-tests-v3\nFetch-It-Via: git fetch https://github.com/dscho/git failing-tests-v3\nInterdiff vs v2:\n\n diff --git a/t/Makefile b/t/Makefile\n index 8aa6a72a70..1bb06c36f2 100644\n --- a/t/Makefile\n +++ b/t/Makefile\n @@ -37,9 +37,8 @@ test: pre-clean $(TEST_LINT)\n \n failed:\n \t@failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n -\t\tgrep -l '^failed [1-9]' $$(ls -t *.counts | \\\n -\t\t\tsed 'G;h;/^\\(t[^.]*\\)-[0-9]*\\..*\\n\\1-[0-9]*\\./d;P;d') | \\\n -\t\tsed -n 's/-[0-9]*\\.counts$$/.sh/p') && \\\n +\t\tgrep -l '^failed [1-9]' *.counts | \\\n +\t\tsed -n 's/\\.counts$$/.sh/p') && \\\n \ttest -z \"$$failed\" || $(MAKE) $$failed\n \n prove: pre-clean $(TEST_LINT)\n\n\n t/Makefile | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex d613935f14..1bb06c36f2 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -35,6 +35,12 @@ all: $(DEFAULT_TEST_TARGET)\n test: pre-clean $(TEST_LINT)\n \t$(MAKE) aggregate-results-and-cleanup\n \n+failed:\n+\t@failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n+\t\tgrep -l '^failed [1-9]' *.counts | \\\n+\t\tsed -n 's/\\.counts$$/.sh/p') && \\\n+\ttest -z \"$$failed\" || $(MAKE) $$failed\n+\n prove: pre-clean $(TEST_LINT)\n \t@echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n \t$(MAKE) clean-except-prove-cache\n\nbase-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe\n-- \n2.11.1.windows.prerelease.2.9.g3014b57\n"},{"id":"310411","messageId":"20170127170713.kn35br4xsdco7xth@sigill.intra.peff.net","threadId":"42741","inReplyTo":"85044791cfcba35c1ad3d8138051f3f075cb0646.1485526641.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v3] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-27T17:07:13Z","receivedAt":"2017-01-27T17:21:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 27, 2017 at 03:17:36PM +0100, Johannes Schindelin wrote:\n\n> This patch automates the process of determinig which tests failed\n> previously and re-running them.\n\ns/determinig/determining/\n\nPatch otherwise looks good, and I'm happy to be rid of the sed\ncomplexity from v2.\n\n-Peff\n"},{"id":"310413","messageId":"0563f07117e828c072ba542c1a57441e2e8efb81.1485537593.git.johannes.schindelin@gmx.de","threadId":"42741","inReplyTo":"85044791cfcba35c1ad3d8138051f3f075cb0646.1485526641.git.johannes.schindelin@gmx.de","subject":"[PATCH v4] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-27T17:21:30Z","receivedAt":"2017-01-27T17:28:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This patch automates the process of determining which tests failed\npreviously and re-running them.\n\nWhile developing patch series, it is a good practice to run the test\nsuite from time to time, just to make sure that obvious bugs are caught\nearly. With complex patch series, it is common to run `make -j15 -k\ntest`, i.e. run the tests in parallel and *not* stop at the first\nfailing test but continue. This has the advantage of identifying\npossibly multiple problems in one big test run.\n\nIt is particularly important to reduce the turn-around time thusly on\nWindows, where the test suite spends 45 minutes on the computer on which\nthis patch was developed.\n\nIt is the most convenient way to determine which tests failed after\nrunning the entire test suite, in parallel, to look for left-over \"trash\ndirectory.t*\" subdirectories in the t/ subdirectory. However, those\ndirectories might live outside t/ when overridden using the\n--root= option, to which the Makefile has no access. The next\nbest method is to grep explicitly for failed tests in the test-results/\ndirectory, which the Makefile *can* access.\n\nPlease note that the often-recommended `prove` tool requires Perl, and\nthat opens a whole new can of worms on Windows. As no native Windows Perl\ncomes with Subversion bindings, we have to use a Perl in Git for Windows\nthat uses the POSIX emulation layer named MSYS2 (which is a portable\nversion of Cygwin). When using this emulation layer under stress, e.g.\nwhen running massively-parallel tests, unexplicable crashes occur quite\nfrequently, and instead of having a solution to the original problem, the\ndeveloper now has an additional, quite huge problem. For that reason, this\ndeveloper rejected `prove` as a solution and went with this patch instead.\n\nSigned-off-by: Johannes Schindelin \n---\nPublished-As: https://github.com/dscho/git/releases/tag/failing-tests-v4\nFetch-It-Via: git fetch https://github.com/dscho/git failing-tests-v4\n\n t/Makefile | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex d613935f14..1bb06c36f2 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -35,6 +35,12 @@ all: $(DEFAULT_TEST_TARGET)\n test: pre-clean $(TEST_LINT)\n \t$(MAKE) aggregate-results-and-cleanup\n \n+failed:\n+\t@failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n+\t\tgrep -l '^failed [1-9]' *.counts | \\\n+\t\tsed -n 's/\\.counts$$/.sh/p') && \\\n+\ttest -z \"$$failed\" || $(MAKE) $$failed\n+\n prove: pre-clean $(TEST_LINT)\n \t@echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n \t$(MAKE) clean-except-prove-cache\n\nbase-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe\n-- \n2.11.1.windows.prerelease.2.9.g3014b57\n"},{"id":"310424","messageId":"alpine.DEB.2.20.1701271821380.3469@virtualbox","threadId":"42741","inReplyTo":"20170127170713.kn35br4xsdco7xth@sigill.intra.peff.net","subject":"Re: [PATCH v3] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-27T17:21:58Z","receivedAt":"2017-01-27T18:02:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 27 Jan 2017, Jeff King wrote:\n\n> On Fri, Jan 27, 2017 at 03:17:36PM +0100, Johannes Schindelin wrote:\n> \n> > This patch automates the process of determinig which tests failed\n> > previously and re-running them.\n> \n> s/determinig/determining/\n\nFixed in v4,\nJohannes\n"},{"id":"310432","messageId":"xmqq4m0kz65d.fsf@gitster.mtv.corp.google.com","threadId":"42741","inReplyTo":"0563f07117e828c072ba542c1a57441e2e8efb81.1485537593.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v4] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-27T18:53:18Z","receivedAt":"2017-01-27T18:57:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> This patch automates the process of determining which tests failed\n> previously and re-running them.\n> ...\n>\n> Signed-off-by: Johannes Schindelin \n\nI stored both versions in files and compared them, and it seems the\nsingle word change in the proposed commit log message is the only\ndifference. I would have written \"Automate the process...\", though.\n\nIf you are resending, touching up to cover all points raised by a\nreviewer and doing nothing else, having \"Reviewed-by: Jeff King\n\" would have been nicer. \n\nWill queue. Thanks.\n\n> ---\n> Published-As: https://github.com/dscho/git/releases/tag/failing-tests-v4\n> Fetch-It-Via: git fetch https://github.com/dscho/git failing-tests-v4\n>\n> t/Makefile | 6 ++++++\n> 1 file changed, 6 insertions(+)\n>\n> diff --git a/t/Makefile b/t/Makefile\n> index d613935f14..1bb06c36f2 100644\n> --- a/t/Makefile\n> +++ b/t/Makefile\n> @@ -35,6 +35,12 @@ all: $(DEFAULT_TEST_TARGET)\n> test: pre-clean $(TEST_LINT)\n> \t$(MAKE) aggregate-results-and-cleanup\n> \n> +failed:\n> +\t@failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \\\n> +\t\tgrep -l '^failed [1-9]' *.counts | \\\n> +\t\tsed -n 's/\\.counts$$/.sh/p') && \\\n> +\ttest -z \"$$failed\" || $(MAKE) $$failed\n> +\n> prove: pre-clean $(TEST_LINT)\n> \t@echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n> \t$(MAKE) clean-except-prove-cache\n>\n> base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe\n"},{"id":"310525","messageId":"alpine.DEB.2.20.1701301627260.3469@virtualbox","threadId":"42741","inReplyTo":"xmqq4m0kz65d.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4] t/Makefile: add a rule to re-run previously-failed tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-30T15:35:05Z","receivedAt":"2017-01-30T15:35:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 27 Jan 2017, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > This patch automates the process of determining which tests failed\n> > previously and re-running them.\n> > ...\n> >\n> > Signed-off-by: Johannes Schindelin \n> \n> I stored both versions in files and compared them, and it seems the\n> single word change in the proposed commit log message is the only\n> difference. I would have written \"Automate the process...\", though.\n\nYes, we have different styles. Thanks for letting my commit keep my commit\nmessage this time ;-)\n\n> If you are resending, touching up to cover all points raised by a\n> reviewer and doing nothing else, having \"Reviewed-by: Jeff King\n> \" would have been nicer.\n\nTBH I am not at all sure that I know when to add those footers and when\nnot. After having been asked to remove such a footer, I decided to *not*\ninclude them by default.\n\nHaving gray zones about the footers strikes me as similar to having gray\nzones in the coding style guidelines: it sure gives the contributors more\nfreedom, but it also creates uncertainty and as a consequence takes up a\nlot of reviewing space and time (hence taking away space and time from\nreviewing the code for bugs).\n\nIn other words: while I appreciate the idea of giving contributors such as\nmyself a lot of leeway, I would love even more to be able to automate away\ntedious and boring tasks (such as adding Tested-by: or Reviewed-by:\nfooters, or for that matter, addressing code style issues before any\nreviewer has to shed bikes so that they can focus on the parts of the\nreview that no machine can do for them).\n\nCiao,\nJohannes\n"}]}