{"thread":{"id":"64492","subject":"[PATCH] ci(dockerized): do show the result of failing tests again","startedAt":"2025-11-17T17:04:27Z","lastAt":"2025-11-29T18:31:31Z","messageCount":9,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Jeff King","Elijah Newren","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"530823","messageId":"pull.2003.git.1763399064983.gitgitgadget@gmail.com","threadId":"64492","inReplyTo":null,"subject":"[PATCH] ci(dockerized): do show the result of failing tests again","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-11-17T17:04:24Z","receivedAt":"2025-11-17T17:04:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe quality of tests/test suites does not show as much when there are no\nbreakages as in the amount of time required after bugs trigger test\nfailures before the bugs can be identified, analyzed and resolved.\n\nAs such, it is an unfortunate side effect of 2a21098b98a (github: adapt\ncontainerized jobs to be rootless, 2025-01-10) that the output of failed\ntest cases, which was shown before that change directly in the build\nlogs, is now no longer shown at all.\n\nThe reason is a side effect of trying to run the build and the tests\nwith permissions other than the `root` user, but without providing the\nprerequisite permissions to signal what tests failed and whose output\nhence needs to be included in the logs.\n\nThe way this signaling works is for the workflow to write into\nspecial-purpose files whose path is specific to the current workflow\nstep and which can be accessed via the `$GITHUB_ENV` environment\nvariable, which differs between workflow steps. It is this file that is\nmissing write permission for the `builder` user that was introduced in\nabove-mentioned commit.\n\nThe solution is simple: make the file world-writable.\n\nTechnically, this write permission should be removed after the step has\ncompleted, if proper security practices were to be upheld, but since\nnothing uses that file again, it does not matter, and the fix is more\nsuccinct this way.\n\nThis commit is best viewed with `--color-words`.\n\nSigned-off-by: Johannes Schindelin \n---\n ci(dockerized): do show the result of failing tests again\n \n It has become quite hard to debug CI failures when they happen in one of\n the Dockerized jobs, as the actual test failures are now hidden. This\n was most likely an oversight when 2a21098b98a (github: adapt\n containerized jobs to be rootless, 2025-01-10) was merged in 2bf3c7fab19\n (Merge branch 'ps/ci-misc-updates', 2025-02-06), v2.49.0-rc0~55, and I\n had reported this as a regression in\n https://lore.kernel.org/git/e45b9487-b3ae-ed85-fd07-c92cfbf47cbb@gmx.de/.\n Seeing no movement on my report, and having the pressure of\n newly-failing tests during the v2.52.0-rc0 rebase of Git for Windows, I\n was kind of forced into fixing this in Git for Windows. Here I upstream\n the fix.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2003%2Fdscho%2Ffix-failure-reporting-in-dockerized-ci-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2003/dscho/fix-failure-reporting-in-dockerized-ci-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2003\n\n .github/workflows/main.yml | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 816d5a34c4..ca7cc2984f 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -433,7 +433,7 @@ jobs:\n - run: ci/install-dependencies.sh\n - run: useradd builder --create-home\n - run: chown -R builder .\n- - run: sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh\n+ - run: chmod o+w $GITHUB_ENV && sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh\n - name: print test failures\n if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n run: sudo --preserve-env --set-home --user=builder ci/print-test-failures.sh\n\nbase-commit: 621415c8b5371a4734315232a780dd8282f6fe4f\n-- \ngitgitgadget\n"},{"id":"530829","messageId":"xmqqpl9gike6.fsf@gitster.g","threadId":"64492","inReplyTo":"pull.2003.git.1763399064983.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci(dockerized): do show the result of failing tests again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-17T18:28:17Z","receivedAt":"2025-11-17T18:28:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> The quality of tests/test suites does not show as much when there are no\n> breakages as in the amount of time required after bugs trigger test\n> failures before the bugs can be identified, analyzed and resolved.\n>\n> As such, it is an unfortunate side effect of 2a21098b98a (github: adapt\n> containerized jobs to be rootless, 2025-01-10) that the output of failed\n> test cases, which was shown before that change directly in the build\n> logs, is now no longer shown at all.\n>\n> The reason is a side effect of trying to run the build and the tests\n> with permissions other than the `root` user, but without providing the\n> prerequisite permissions to signal what tests failed and whose output\n> hence needs to be included in the logs.\n>\n> The way this signaling works is for the workflow to write into\n> special-purpose files whose path is specific to the current workflow\n> step and which can be accessed via the `$GITHUB_ENV` environment\n> variable, which differs between workflow steps. It is this file that is\n> missing write permission for the `builder` user that was introduced in\n> above-mentioned commit.\n>\n> The solution is simple: make the file world-writable.\n\nI expected to see a+w not o+w from this statement; as long as it\nworks I have no strong objections, but if I saw o+w without the\nabove explanation I would probably have wondered who are in the\ngroup that we do not want this file touched by.\n\n> Technically, this write permission should be removed after the step has\n> completed, if proper security practices were to be upheld, but since\n> nothing uses that file again, it does not matter, and the fix is more\n> succinct this way.\n>\n> This commit is best viewed with `--color-words`.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2003%2Fdscho%2Ffix-failure-reporting-in-dockerized-ci-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2003/dscho/fix-failure-reporting-in-dockerized-ci-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/2003\n>\n> .github/workflows/main.yml | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 816d5a34c4..ca7cc2984f 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -433,7 +433,7 @@ jobs:\n> - run: ci/install-dependencies.sh\n> - run: useradd builder --create-home\n> - run: chown -R builder .\n> - - run: sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh\n> + - run: chmod o+w $GITHUB_ENV && sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh\n> - name: print test failures\n> if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n> run: sudo --preserve-env --set-home --user=builder ci/print-test-failures.sh\n\nThanks. Will apply.\n"},{"id":"530892","messageId":"20251118093659.GA530545@coredump.intra.peff.net","threadId":"64492","inReplyTo":"pull.2003.git.1763399064983.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci(dockerized): do show the result of failing tests again","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-11-18T09:36:59Z","receivedAt":"2025-11-18T09:37:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 17, 2025 at 05:04:24PM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> The way this signaling works is for the workflow to write into\n> special-purpose files whose path is specific to the current workflow\n> step and which can be accessed via the `$GITHUB_ENV` environment\n> variable, which differs between workflow steps. It is this file that is\n> missing write permission for the `builder` user that was introduced in\n> above-mentioned commit.\n\nThanks for fixing this. It bit me recently, but I hadn't had time to\nlook at it yet.\n\nBTW, I ran into a similar issue (no useful output from a failed test) in\nthe windows-meson job, but the cause is totally different there:\n\n https://lore.kernel.org/git/20251118093221.GA530337@coredump.intra.peff.net/\n\n-Peff\n"},{"id":"531169","messageId":"xmqqqztp1nel.fsf@gitster.g","threadId":"64492","inReplyTo":"xmqqpl9gike6.fsf@gitster.g","subject":"Re: [PATCH] ci(dockerized): do show the result of failing tests again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-23T02:41:06Z","receivedAt":"2025-11-23T02:41:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> The solution is simple: make the file world-writable.\n>\n> I expected to see a+w not o+w from this statement; as long as it\n> works I have no strong objections, but if I saw o+w without the\n> above explanation I would probably have wondered who are in the\n> group that we do not want this file touched by.\n> ...\n>> - run: useradd builder --create-home\n>> - run: chown -R builder .\n>> - - run: sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh\n>> + - run: chmod o+w $GITHUB_ENV && sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh\n>> - name: print test failures\n\nUnless I hear that \"user X belongs to the same group as our user\nthat runs 'chmod' on $GITHUB_ENV, and we do not want that user to be\nwriting into the file\", I'll amend the patch text to match the\n\"solution\" described in the proposed log message to \"chmod a+w\",\nbefore we mark the topic for 'next'.\n\nThanks.\n"},{"id":"531251","messageId":"CABPp-BErdhTjbqDem4Xvc-XbhgLUEpy9-eiaaR1F_diMca--6A@mail.gmail.com","threadId":"64492","inReplyTo":"pull.2003.git.1763399064983.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci(dockerized): do show the result of failing tests again","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-11-25T06:15:07Z","receivedAt":"2025-11-25T06:15:19Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Nov 17, 2025 at 9:17 AM Johannes Schindelin via GitGitGadget\n wrote:\n>\n> From: Johannes Schindelin \n>\n> The quality of tests/test suites does not show as much when there are no\n> breakages as in the amount of time required after bugs trigger test\n> failures before the bugs can be identified, analyzed and resolved.\n\nI found this paragraph hard to parse. After re-reading a couple\ntimes, does the following convey the same meaning?:\n\nThe quality of tests and test suites is most apparent not when\neverything passes, but in how quickly bugs can be identified,\nanalyzed, and resolved after test failures occur.\n"},{"id":"531259","messageId":"xmqqjyzetc6y.fsf@gitster.g","threadId":"64492","inReplyTo":"CABPp-BErdhTjbqDem4Xvc-XbhgLUEpy9-eiaaR1F_diMca--6A@mail.gmail.com","subject":"Re: [PATCH] ci(dockerized): do show the result of failing tests again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-25T14:32:37Z","receivedAt":"2025-11-25T14:32:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> On Mon, Nov 17, 2025 at 9:17 AM Johannes Schindelin via GitGitGadget\n> wrote:\n>>\n>> From: Johannes Schindelin \n>>\n>> The quality of tests/test suites does not show as much when there are no\n>> breakages as in the amount of time required after bugs trigger test\n>> failures before the bugs can be identified, analyzed and resolved.\n>\n> I found this paragraph hard to parse. After re-reading a couple\n> times, does the following convey the same meaning?:\n>\n> The quality of tests and test suites is most apparent not when\n> everything passes, but in how quickly bugs can be identified,\n> analyzed, and resolved after test failures occur.\n\nFWIW, I had the same \"I cannot quite figure out how this paragraph\nreally wants to help readers by saying this\" reaction to the\nparagraph. Your rewrite finally helped me understand the intention\n(if that is what the originall wanted to say, that is).\n\nThanks.\n\n"},{"id":"531268","messageId":"d8054499-aacc-f697-c117-116729432c3a@gmx.de","threadId":"64492","inReplyTo":"CABPp-BErdhTjbqDem4Xvc-XbhgLUEpy9-eiaaR1F_diMca--6A@mail.gmail.com","subject":"Re: [PATCH] ci(dockerized): do show the result of failing tests again","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-11-25T17:40:39Z","receivedAt":"2025-11-25T17:40:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Mon, 24 Nov 2025, Elijah Newren wrote:\n\n> On Mon, Nov 17, 2025 at 9:17 AM Johannes Schindelin via GitGitGadget\n> wrote:\n> >\n> > From: Johannes Schindelin \n> >\n> > The quality of tests/test suites does not show as much when there are no\n> > breakages as in the amount of time required after bugs trigger test\n> > failures before the bugs can be identified, analyzed and resolved.\n> \n> I found this paragraph hard to parse. After re-reading a couple\n> times, does the following convey the same meaning?:\n> \n> The quality of tests and test suites is most apparent not when\n> everything passes, but in how quickly bugs can be identified,\n> analyzed, and resolved after test failures occur.\n\nYes, this reflects what I tried to say.\n\nCiao,\nJohannes\n"},{"id":"531272","messageId":"xmqqsee1rjyx.fsf@gitster.g","threadId":"64492","inReplyTo":"d8054499-aacc-f697-c117-116729432c3a@gmx.de","subject":"Re: [PATCH] ci(dockerized): do show the result of failing tests again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-25T19:27:34Z","receivedAt":"2025-11-25T19:27:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Elijah,\n>\n> On Mon, 24 Nov 2025, Elijah Newren wrote:\n>\n>> On Mon, Nov 17, 2025 at 9:17 AM Johannes Schindelin via GitGitGadget\n>> wrote:\n>> >\n>> > From: Johannes Schindelin \n>> >\n>> > The quality of tests/test suites does not show as much when there are no\n>> > breakages as in the amount of time required after bugs trigger test\n>> > failures before the bugs can be identified, analyzed and resolved.\n>> \n>> I found this paragraph hard to parse. After re-reading a couple\n>> times, does the following convey the same meaning?:\n>> \n>> The quality of tests and test suites is most apparent not when\n>> everything passes, but in how quickly bugs can be identified,\n>> analyzed, and resolved after test failures occur.\n>\n> Yes, this reflects what I tried to say.\n\nSo, do you mind if I locally amended the log message, or should we\nexpect an updated patch sent to the list? For a small thing like\nthis, either is fine by me.\n\nThanks.\n\n"},{"id":"531447","messageId":"920c302c-5d36-d5fe-7f19-28a1eb905a19@gmx.de","threadId":"64492","inReplyTo":"xmqqsee1rjyx.fsf@gitster.g","subject":"Re: [PATCH] ci(dockerized): do show the result of failing tests again","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-11-29T18:31:27Z","receivedAt":"2025-11-29T18:31:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 25 Nov 2025, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > On Mon, 24 Nov 2025, Elijah Newren wrote:\n> >\n> >> On Mon, Nov 17, 2025 at 9:17 AM Johannes Schindelin via GitGitGadget\n> >> wrote:\n> >> >\n> >> > From: Johannes Schindelin \n> >> >\n> >> > The quality of tests/test suites does not show as much when there are no\n> >> > breakages as in the amount of time required after bugs trigger test\n> >> > failures before the bugs can be identified, analyzed and resolved.\n> >> \n> >> I found this paragraph hard to parse. After re-reading a couple\n> >> times, does the following convey the same meaning?:\n> >> \n> >> The quality of tests and test suites is most apparent not when\n> >> everything passes, but in how quickly bugs can be identified,\n> >> analyzed, and resolved after test failures occur.\n> >\n> > Yes, this reflects what I tried to say.\n> \n> So, do you mind if I locally amended the log message, or should we\n> expect an updated patch sent to the list? For a small thing like\n> this, either is fine by me.\n\nSure, I saw that you amended the log message and also changed the `chmod`\nto be a bit more robust.\n\nCiao,\nJohannes\n"}]}