{"thread":{"id":"48940","subject":"[PATCH 0/1] t7406: avoid failures solely due to timing issues","startedAt":"2018-07-23T13:39:43Z","lastAt":"2018-07-24T09:00:15Z","messageCount":7,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Stefan Beller","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"353345","messageId":"pull.12.git.gitgitgadget@gmail.com","threadId":"48940","inReplyTo":null,"subject":"[PATCH 0/1] t7406: avoid failures solely due to timing issues","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-07-23T13:39:40Z","receivedAt":"2018-07-23T13:39:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This fixes a regression test that produces false positives occasionally: https://git-for-windows.visualstudio.com/git/_build/results?buildId=14035&view=logs\n\nJohannes Schindelin (1):\n  t7406: avoid failures solely due to timing issues\n\n t/t7406-submodule-update.sh | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\n\nbase-commit: b7bd9486b055c3f967a870311e704e3bb0654e4f\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-12%2Fdscho%2Fmore-robust-t7406-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-12/dscho/more-robust-t7406-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/12\n-- \ngitgitgadget\n"},{"id":"353346","messageId":"78f7a5e4baae162d1dff18701a25903b304adf9e.1532353179.git.gitgitgadget@gmail.com","threadId":"48940","inReplyTo":"pull.12.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] t7406: avoid failures solely due to timing issues","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-07-23T13:39:42Z","receivedAt":"2018-07-23T13:39:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nRegression tests are automated tests which try to ensure a specific\nbehavior. The idea is: if the test case fails, the behavior indicated in\nthe test case's title regressed.\n\nIf a regression test that fails, even occasionally, for any reason other\nthan to indicate the particular regression(s) it tries to catch, it is\nless useful than when it really only fails when there is a bug in the\n(non-test) code that needs to be fixed.\n\nIn the instance of the test case \"submodule update --init --recursive\nfrom subdirectory\" of the script t7406-submodule-update.sh, the exact\noutput of a recursive clone is compared with a pre-generated one. And\nthis is a racy test because the structure of the submodules only\nguarantees a *partial* order. The 'none' and the 'rebasing' submodules\n*can* be cloned in any order, which means that a mismatch with the\nhard-coded order does not necessarily indicate a bug in the tested code.\n\nSee for example:\nhttps://git-for-windows.visualstudio.com/git/_build/results?buildId=14035&view=logs\n\nTo prevent such false positives from unnecessarily costing time when\ninvestigating test failures, let's take the exact order of the lines out\nof the equation by sorting them before comparing them.\n\nThis test script seems not to have any more test cases that try to\nverify any specific order in which recursive clones process the\nsubmodules, therefore this is the only test case that is changed in this\nmanner.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t7406-submodule-update.sh | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 9e0d31700..4937ebb67 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -115,17 +115,17 @@ Submodule path '../super/submodule': checked out '$submodulesha1'\n EOF\n \n cat <<EOF >expect2\n+Cloning into '$pwd/recursivesuper/super/merging'...\n+Cloning into '$pwd/recursivesuper/super/none'...\n+Cloning into '$pwd/recursivesuper/super/rebasing'...\n+Cloning into '$pwd/recursivesuper/super/submodule'...\n Submodule 'merging' ($pwd/merging) registered for path '../super/merging'\n Submodule 'none' ($pwd/none) registered for path '../super/none'\n Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing'\n Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule'\n-Cloning into '$pwd/recursivesuper/super/merging'...\n done.\n-Cloning into '$pwd/recursivesuper/super/none'...\n done.\n-Cloning into '$pwd/recursivesuper/super/rebasing'...\n done.\n-Cloning into '$pwd/recursivesuper/super/submodule'...\n done.\n EOF\n \n@@ -137,7 +137,8 @@ test_expect_success 'submodule update --init --recursive from subdirectory' '\n \t git submodule update --init --recursive ../super >../../actual 2>../../actual2\n \t) &&\n \ttest_i18ncmp expect actual &&\n-\ttest_i18ncmp expect2 actual2\n+\tsort actual2 >actual2.sorted &&\n+\ttest_i18ncmp expect2 actual2.sorted\n '\n \n cat <<EOF >expect2\n-- \ngitgitgadget\n"},{"id":"353401","messageId":"xmqqr2jt6a5b.fsf@gitster-ct.c.googlers.com","threadId":"48940","inReplyTo":"pull.12.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] t7406: avoid failures solely due to timing issues","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-23T19:10:24Z","receivedAt":"2018-07-23T19:10:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> This fixes a regression test that produces false positives occasionally: https://git-for-windows.visualstudio.com/git/_build/results?buildId=14035&view=logs\n>\n\n[jc: forwrding to Torsten for <208b2ede-4833-f062-16f2-f35b8a8ce099@web.de>]\n\n> Johannes Schindelin (1):\n>   t7406: avoid failures solely due to timing issues\n>\n>  t/t7406-submodule-update.sh | 11 ++++++-----\n>  1 file changed, 6 insertions(+), 5 deletions(-)\n>\n>\n> base-commit: b7bd9486b055c3f967a870311e704e3bb0654e4f\n> Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-12%2Fdscho%2Fmore-robust-t7406-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-12/dscho/more-robust-t7406-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/12\n"},{"id":"353403","messageId":"xmqqmuuh69xx.fsf@gitster-ct.c.googlers.com","threadId":"48940","inReplyTo":"78f7a5e4baae162d1dff18701a25903b304adf9e.1532353179.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] t7406: avoid failures solely due to timing issues","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-23T19:14:50Z","receivedAt":"2018-07-23T19:14:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> To prevent such false positives from unnecessarily costing time when\n> investigating test failures, let's take the exact order of the lines out\n> of the equation by sorting them before comparing them.\n> ...\n>  cat <<EOF >expect2\n> +Cloning into '$pwd/recursivesuper/super/merging'...\n> +Cloning into '$pwd/recursivesuper/super/none'...\n> +Cloning into '$pwd/recursivesuper/super/rebasing'...\n> +Cloning into '$pwd/recursivesuper/super/submodule'...\n>  Submodule 'merging' ($pwd/merging) registered for path '../super/merging'\n>  Submodule 'none' ($pwd/none) registered for path '../super/none'\n>  Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing'\n>  Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule'\n\nThanks.  It obviously goes in the right direction to loosen the\nordering requirement that fundamentally cannot be met, as the log\nmessage cleanly analizes.\n\nDo we know that the write(2)s that show these lines are atomic, or\nare we merely lucky that we don't see them intermixed in the middle\nof lines (sbeller cc'ed).  I _think_ they are but just wanted to\ndouble check.\n\n\n"},{"id":"353416","messageId":"CAGZ79kag=YC6PxOnBbVAqY92dNd5yuqfvash-oPVrEktowNbVA@mail.gmail.com","threadId":"48940","inReplyTo":"xmqqmuuh69xx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] t7406: avoid failures solely due to timing issues","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-23T20:53:10Z","receivedAt":"2018-07-23T20:53:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Jul 23, 2018 at 12:14 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > To prevent such false positives from unnecessarily costing time when\n> > investigating test failures, let's take the exact order of the lines out\n> > of the equation by sorting them before comparing them.\n> > ...\n> >  cat <<EOF >expect2\n> > +Cloning into '$pwd/recursivesuper/super/merging'...\n> > +Cloning into '$pwd/recursivesuper/super/none'...\n> > +Cloning into '$pwd/recursivesuper/super/rebasing'...\n> > +Cloning into '$pwd/recursivesuper/super/submodule'...\n> >  Submodule 'merging' ($pwd/merging) registered for path '../super/merging'\n> >  Submodule 'none' ($pwd/none) registered for path '../super/none'\n> >  Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing'\n> >  Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule'\n>\n> Thanks.  It obviously goes in the right direction to loosen the\n> ordering requirement that fundamentally cannot be met, as the log\n> message cleanly analizes.\n>\n> Do we know that the write(2)s that show these lines are atomic, or\n> are we merely lucky that we don't see them intermixed in the middle\n> of lines (sbeller cc'ed).  I _think_ they are but just wanted to\n> double check.\n\nNot just the lines, but the whole message per submodule (i.e.\nthe \"Cloning... \" lione and its accompanying \"done\") is one\natomic unit. These messages are from processes invoked via\nrun_processes_parallel (in run-command.h), which buffers\nall output per process.\n\nThis test was last touched in c66410ed32a (submodule init:\nredirect stdout to stderr, 2016-05-02), merged as f2c96ceb57a\n(Merge branch 'sb/submodule-init', 2016-05-17)\n\nThe parallelizing effort was made before that\n(2335b870fa7 (submodule update: expose parallelism to the\nuser, 2016-02-29) via bdebbeb3346 (Merge branch\n'sb/submodule-parallel-update', 2016-04-06)), but did not\nreview existing tests for the newly introduced raciness.\n\nThanks for the fix!\nStefan\n"},{"id":"353422","messageId":"xmqqlga14p16.fsf@gitster-ct.c.googlers.com","threadId":"48940","inReplyTo":"CAGZ79kag=YC6PxOnBbVAqY92dNd5yuqfvash-oPVrEktowNbVA@mail.gmail.com","subject":"Re: [PATCH 1/1] t7406: avoid failures solely due to timing issues","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-23T21:31:49Z","receivedAt":"2018-07-23T21:31:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> This test was last touched in c66410ed32a (submodule init:\n> redirect stdout to stderr, 2016-05-02), merged as f2c96ceb57a\n> (Merge branch 'sb/submodule-init', 2016-05-17)\n>\n> The parallelizing effort was made before that\n> (2335b870fa7 (submodule update: expose parallelism to the\n> user, 2016-02-29) via bdebbeb3346 (Merge branch\n> 'sb/submodule-parallel-update', 2016-04-06)), but did not\n> review existing tests for the newly introduced raciness.\n\nAh, that explains what I was scratching my head about.  I somehow\nthought that the parallel update broke an otherwise working test\nwritten back in the serial days, but it seems it was the other way\naround.  The test was simply careless.\n\nNice to have the two-year old issue fixed.  Thanks, all.\n"},{"id":"353446","messageId":"20180724090005.GA25260@tor.lan","threadId":"48940","inReplyTo":"xmqqr2jt6a5b.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/1] t7406: avoid failures solely due to timing issues","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-07-24T09:00:05Z","receivedAt":"2018-07-24T09:00:15Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Mon, Jul 23, 2018 at 12:10:24PM -0700, Junio C Hamano wrote:\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n> \n> > This fixes a regression test that produces false positives occasionally: https://git-for-windows.visualstudio.com/git/_build/results?buildId=14035&view=logs\n> >\n> \n> [jc: forwrding to Torsten for <208b2ede-4833-f062-16f2-f35b8a8ce099@web.de>]\n\nYes,\nthanks for a \"fantastic fast fix\" and the integration.\n\nI downloaded and tested it OK (under MacOS) this morning.\n"}]}