{"thread":{"id":"66392","subject":"[PATCH] ci: point leak-sanitizer failures at the actual test and error","startedAt":"2026-09-25T18:54:06Z","lastAt":"2026-10-06T06:56:38Z","messageCount":39,"participants":["Harald Nordgren via GitGitGadget","Ben Knoble","Harald Nordgren","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"553310","messageId":"pull.2419.git.git.1790362443893.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":null,"subject":"[PATCH] ci: point leak-sanitizer failures at the actual test and error","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-25T18:54:03Z","receivedAt":"2026-09-25T18:54:06Z","isPatch":true,"body":"From: Harald Nordgren \n\nA leak is only found once, at the end of a whole script, well after\nevery test already reported ok, and the failure annotation carried\nno file or line, so all a reviewer ever saw was:\n\n Process completed with exit code 1.\n\nwith nothing to click through to. Stop each leak-sanitizer script at\nits first failure instead of running the rest of an already-tainted\nscript, and have both failure and leak annotations point at the real\nfile and carry the actual error, for example:\n\n t/t1507-rev-parse-upstream.sh, line 1:\n memory leak logged around t1507.1\n ==ERROR: LeakSanitizer: detected memory leaks\n Direct leak of 60 byte(s) in 1 object(s) allocated from:\n ...\n #5 in add_branch builtin/remote.c:135\n\nSigned-off-by: Harald Nordgren \n---\n ci: point leak-sanitizer failures at the actual test and error\n \n I discovered while running CI on another GitHub pull request that it's\n very hard to see where the error is for the leak tests.\n \n This will stop each leak-sanitizer script at its first failure and\n points annotations at the real file and error.\n \n Proof that it works:\n https://github.com/git/git/actions/runs/35871180948/job/107215430244\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v1\nPull-Request: https://github.com/git/git/pull/2419\n\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 49 +++++++++++++++++++++++-----\n t/test-lib.sh | 2 ++\n 3 files changed, 44 insertions(+), 8 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex c6ccbf8c17..a89f480a78 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)\n \texport NO_CVS_TESTS=LetsSaveSomeTime\n \texport NO_SVN_TESTS=LetsSaveSomeTime\n \texport NO_P4_TESTS=LetsSaveSomeTime\n+\tGIT_TEST_OPTS=\"$GIT_TEST_OPTS --immediate\"\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex fa29a62aa3..4f6460a0ad 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -28,6 +28,7 @@ start_test_output () {\n \tgithub_markup_output=\"${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup\"\n \t>$github_markup_output\n \tGIT_TEST_TEE_OFFSET=0\n+\tgithub_markup_script_name=${0##*/}\n }\n \n # No need to override start_test_case_output\n@@ -35,22 +36,54 @@ start_test_output () {\n finalize_test_case_output () {\n \ttest_case_result=$1\n \tshift\n+\n+\tcase \"$test_case_result\" in\n+\tok|broken)\n+\t\t# Exit without printing the \"ok\" or \"broken\" tests\n+\t\treturn\n+\t\t;;\n+\tesac\n+\n+\ttest_case_line=$(find_test_case_line_ \"$1\")\n+\ttest_case_output=$(test-tool path-utils skip-n-bytes \\\n+\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET)\n+\n \tcase \"$test_case_result\" in\n \tfailure)\n-\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n+\t\ttest_case_summary=$(printf '%s\\n' \"$test_case_output\" |\n+\t\t\ttail -n 20 | github_escape_message_)\n+\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"failed: $this_test.$test_count $1%0A%0A$test_case_summary\"\n \t\t;;\n \tfixed)\n-\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n-\t\t;;\n-\tok|broken)\n-\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n-\t\treturn\n+\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"fixed: $this_test.$test_count $1\"\n \t\t;;\n \tesac\n+\n \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n-\ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n-\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n+\tprintf '%s\\n' \"$test_case_output\" >>$github_markup_output\n \techo >>$github_markup_output \"::endgroup::\"\n }\n \n+finalize_test_leak_output () {\n+\ttest_leak_summary=$(head -n 40 \"$TEST_RESULTS_SAN_FILE\".* |\n+\t\tgithub_escape_message_)\n+\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n+\t\t\"memory leak logged around $this_test.$test_count%0A%0A$test_leak_summary\"\n+}\n+\n # No need to override finalize_test_output\n+\n+github_escape_message_ () {\n+\tsed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\\r/%0D/g' -e 's/\\n/%0A/g'\n+}\n+\n+find_test_case_line_ () {\n+\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n+\thead -n 1 | cut -d: -f1\n+}\n+\n+github_annotation_ () {\n+\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n+}\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1f0505e412..a52589c6a2 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -199,6 +199,7 @@ mark_option_requires_arg () {\n start_test_output () { :; }\n start_test_case_output () { :; }\n finalize_test_case_output () { :; }\n+finalize_test_leak_output () { :; }\n finalize_test_output () { :; }\n \n parse_option () {\n@@ -1218,6 +1219,7 @@ check_test_results_san_file_ () {\n \t\treturn\n \tfi &&\n \tsay_color >&4 error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n+\tfinalize_test_leak_output &&\n \n \tif test \"$test_failure\" = 0\n \tthen\n\nbase-commit: 3bc0341126508f78f5869cbfc0005e987efdf0c7\n-- \ngitgitgadget\n"},{"id":"553320","messageId":"09549A0E-D5FF-465C-A933-F144A19D14E4@gmail.com","threadId":"66392","inReplyTo":"pull.2419.git.git.1790362443893.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: point leak-sanitizer failures at the actual test and error","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-09-25T20:26:11Z","receivedAt":"2026-09-25T20:26:24Z","isPatch":true,"body":"\n> Le 25 sept. 2026 à 14:59, Harald Nordgren via GitGitGadget a écrit :\n> \n> +finalize_test_leak_output () {\n> + test_leak_summary=$(head -n 40 \"$TEST_RESULTS_SAN_FILE\".* |\n> + github_escape_message_)\n> + github_annotation_ error \"t/$github_markup_script_name\" 1 \\\n> + \"memory leak logged around $this_test.$test_count%0A%0A$test_leak_summary\"\n> +}\n> +\n> # No need to override finalize_test_output\n> +\n> +github_escape_message_ () {\n> + sed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\\r/%0D/g' -e 's/\\n/%0A/g'\n> +}\n> +\n> +find_test_case_line_ () {\n> + grep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n> + head -n 1 | cut -d: -f1\n> +}\n> +\n> +github_annotation_ () {\n> + echo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n> +}\n\nWithout commenting on the rest, introducing the helpers first might make the important patch easier to read. "},{"id":"553333","messageId":"CAHwyqnW83nMOcYUJrh8pHT+UT59Kh7kvWOg-j0UqTany2FBvqw@mail.gmail.com","threadId":"66392","inReplyTo":"09549A0E-D5FF-465C-A933-F144A19D14E4@gmail.com","subject":"Re: [PATCH] ci: point leak-sanitizer failures at the actual test and error","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2026-09-25T21:43:15Z","receivedAt":"2026-09-25T21:43:56Z","isPatch":true,"body":"> > +github_escape_message_ () {\n> > + sed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\\r/%0D/g' -e 's/\\n/%0A/g'\n> > +}\n> > +\n> > +find_test_case_line_ () {\n> > + grep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n> > + head -n 1 | cut -d: -f1\n> > +}\n> > +\n> > +github_annotation_ () {\n> > + echo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n> > +}\n>\n> Without commenting on the rest, introducing the helpers first might make the important patch easier to read.\n\nGood point!\n\n\nHarald\n"},{"id":"553386","messageId":"fc4efe9f-69f4-4f58-9f7c-8f2e75a8e590@gmail.com","threadId":"66392","inReplyTo":"pull.2419.git.git.1790362443893.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: point leak-sanitizer failures at the actual test and error","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-09-27T15:19:22Z","receivedAt":"2026-09-27T15:19:27Z","isPatch":true,"body":"Hi Harald\n\nOn 25/09/2026 19:54, Harald Nordgren via GitGitGadget wrote:\n> From: Harald Nordgren \n> \n> ci: point leak-sanitizer failures at the actual test and error\n> \n> I discovered while running CI on another GitHub pull request that it's\n> very hard to see where the error is for the leak tests.\n> \n> This will stop each leak-sanitizer script at its first failure and\n> points annotations at the real file and error.\n\nPutting the leak output in the test results is very welcome, but does \nthis mean that if there are two leaks we only report one?\n\n> Proof that it works:\n> https://github.com/git/git/actions/runs/35871180948/job/107215430244\n\nOpening that link shows that the individual test failures are no-longer \nfolded and I see some very strange scrolling behavior in firefox - when \nthe page opens it scrolls to the bottom of the output of \n\"ci/build-and-run-tests.sh\" and if I try to scroll up it immediately \nscrolls back down as soon as my fingers leave the touchpad.\n\nThe patch below seems to do more than just changing the output to \ndisplay the leak backtrace - it adds some escaping and changes the \nannotations. There is no explanation of what these changes do or why \nthey are required.\n\nThanks\n\nPhillip\n\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v1\n> Pull-Request: https://github.com/git/git/pull/2419\n> \n> ci/lib.sh | 1 +\n> t/test-lib-github-workflow-markup.sh | 49 +++++++++++++++++++++++-----\n> t/test-lib.sh | 2 ++\n> 3 files changed, 44 insertions(+), 8 deletions(-)\n> \n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index c6ccbf8c17..a89f480a78 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)\n> \texport NO_CVS_TESTS=LetsSaveSomeTime\n> \texport NO_SVN_TESTS=LetsSaveSomeTime\n> \texport NO_P4_TESTS=LetsSaveSomeTime\n> +\tGIT_TEST_OPTS=\"$GIT_TEST_OPTS --immediate\"\n> \t;;\n> linux-asan-ubsan)\n> \texport SANITIZE=address,undefined\n> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\n> index fa29a62aa3..4f6460a0ad 100644\n> --- a/t/test-lib-github-workflow-markup.sh\n> +++ b/t/test-lib-github-workflow-markup.sh\n> @@ -28,6 +28,7 @@ start_test_output () {\n> \tgithub_markup_output=\"${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup\"\n> \t>$github_markup_output\n> \tGIT_TEST_TEE_OFFSET=0\n> +\tgithub_markup_script_name=${0##*/}\n> }\n> \n> # No need to override start_test_case_output\n> @@ -35,22 +36,54 @@ start_test_output () {\n> finalize_test_case_output () {\n> \ttest_case_result=$1\n> \tshift\n> +\n> +\tcase \"$test_case_result\" in\n> +\tok|broken)\n> +\t\t# Exit without printing the \"ok\" or \"broken\" tests\n> +\t\treturn\n> +\t\t;;\n> +\tesac\n> +\n> +\ttest_case_line=$(find_test_case_line_ \"$1\")\n> +\ttest_case_output=$(test-tool path-utils skip-n-bytes \\\n> +\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET)\n> +\n> \tcase \"$test_case_result\" in\n> \tfailure)\n> -\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n> +\t\ttest_case_summary=$(printf '%s\\n' \"$test_case_output\" |\n> +\t\t\ttail -n 20 | github_escape_message_)\n> +\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> +\t\t\t\"failed: $this_test.$test_count $1%0A%0A$test_case_summary\"\n> \t\t;;\n> \tfixed)\n> -\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n> -\t\t;;\n> -\tok|broken)\n> -\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n> -\t\treturn\n> +\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> +\t\t\t\"fixed: $this_test.$test_count $1\"\n> \t\t;;\n> \tesac\n> +\n> \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n> -\ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n> -\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n> +\tprintf '%s\\n' \"$test_case_output\" >>$github_markup_output\n> \techo >>$github_markup_output \"::endgroup::\"\n> }\n> \n> +finalize_test_leak_output () {\n> +\ttest_leak_summary=$(head -n 40 \"$TEST_RESULTS_SAN_FILE\".* |\n> +\t\tgithub_escape_message_)\n> +\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n> +\t\t\"memory leak logged around $this_test.$test_count%0A%0A$test_leak_summary\"\n> +}\n> +\n> # No need to override finalize_test_output\n> +\n> +github_escape_message_ () {\n> +\tsed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\\r/%0D/g' -e 's/\\n/%0A/g'\n> +}\n> +\n> +find_test_case_line_ () {\n> +\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n> +\thead -n 1 | cut -d: -f1\n> +}\n> +\n> +github_annotation_ () {\n> +\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n> +}\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 1f0505e412..a52589c6a2 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -199,6 +199,7 @@ mark_option_requires_arg () {\n> start_test_output () { :; }\n> start_test_case_output () { :; }\n> finalize_test_case_output () { :; }\n> +finalize_test_leak_output () { :; }\n> finalize_test_output () { :; }\n> \n> parse_option () {\n> @@ -1218,6 +1219,7 @@ check_test_results_san_file_ () {\n> \t\treturn\n> \tfi &&\n> \tsay_color >&4 error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n> +\tfinalize_test_leak_output &&\n> \n> \tif test \"$test_failure\" = 0\n> \tthen\n> \n> base-commit: 3bc0341126508f78f5869cbfc0005e987efdf0c7\n\n"},{"id":"553392","messageId":"CAHwyqnVyDeZV7-ev6+BGeD+_qG89ZY_41dfs95FnouC0B+HJ8g@mail.gmail.com","threadId":"66392","inReplyTo":"fc4efe9f-69f4-4f58-9f7c-8f2e75a8e590@gmail.com","subject":"Re: [PATCH] ci: point leak-sanitizer failures at the actual test and error","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2026-09-27T19:52:28Z","receivedAt":"2026-09-27T19:53:07Z","isPatch":true,"body":"> > ci: point leak-sanitizer failures at the actual test and error\n> >\n> > I discovered while running CI on another GitHub pull request that it's\n> > very hard to see where the error is for the leak tests.\n> >\n> > This will stop each leak-sanitizer script at its first failure and\n> > points annotations at the real file and error.\n>\n> Putting the leak output in the test results is very welcome, but does\n> this mean that if there are two leaks we only report one?\n\nIt already had a behavior where one failure made every subsequent test\nin the script report \"not ok\" too, so lots of noise burying the real\nleaks.\n\n> > Proof that it works:\n> > https://github.com/git/git/actions/runs/35871180948/job/107215430244\n>\n> Opening that link shows that the individual test failures are no-longer\n> folded and I see some very strange scrolling behavior in firefox - when\n> the page opens it scrolls to the bottom of the output of\n> \"ci/build-and-run-tests.sh\" and if I try to scroll up it immediately\n> scrolls back down as soon as my fingers leave the touchpad.\n\nI'll take a look at that.\n\n> The patch below seems to do more than just changing the output to\n> display the leak backtrace - it adds some escaping and changes the\n> annotations. There is no explanation of what these changes do or why\n> they are required.\n\nI'll expand the commit message.\n\n\nHarald\n"},{"id":"553512","messageId":"pull.2419.v2.git.git.1790621693.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.git.git.1790362443893.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] ci: link failure and leak annotations to the test script","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-28T18:54:51Z","receivedAt":"2026-09-28T18:54:57Z","isPatch":true,"body":"Link failure and leak annotations in CI to the test script, so both can be\nfound from the job summary.\n\nCI Job where failures and leaks are reported:\nhttps://github.com/git/git/actions/runs/36449319996/job/109020434364?pr=2426\n\nChanges in v2:\n\n * Split into two commits, each explaining its own reasoning.\n * Leak output is no longer capped or embedded in the message, it's now an\n uncapped fold, so multiple leaks in the same test both show in full. A\n second leak in a different test still won't show in the same run,\n --immediate stops the script at the first failure, but it no longer gets\n buried under every later test falsely reporting \"not ok\" either.\n * Drops the giant unfolded message that annotations used to carry, which is\n what probably caused the scrolling behavior.\n\nHarald Nordgren (2):\n ci: annotate leaks and stop a leak-sanitizer script at its first\n failure\n ci: point test failures and fixed known breakages at their file and\n line\n\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 57 ++++++++++++++++++++++++----\n t/test-lib.sh | 21 ++++++----\n 3 files changed, 63 insertions(+), 16 deletions(-)\n\n\nbase-commit: 34f06850c16c7f7ac822b1adc71354f11b0f2ca3\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v2\nPull-Request: https://github.com/git/git/pull/2419\n\nRange-diff vs v1:\n\n 1: 61b0355780 ! 1: 46e13a6e77 ci: point leak-sanitizer failures at the actual test and error\n @@ Metadata\n Author: Harald Nordgren \n \n ## Commit message ##\n - ci: point leak-sanitizer failures at the actual test and error\n + ci: annotate leaks and stop a leak-sanitizer script at its first failure\n \n - A leak is only found once, at the end of a whole script, well after\n - every test already reported ok, and the failure annotation carried\n - no file or line, so all a reviewer ever saw was:\n + A leak is only discovered once, at the end of a whole script, well\n + after every test has already reported ok, and it gets no annotation at\n + all, so a leak-sanitizer job's only visible failure is:\n \n Process completed with exit code 1.\n \n - with nothing to click through to. Stop each leak-sanitizer script at\n - its first failure instead of running the rest of an already-tainted\n - script, and have both failure and leak annotations point at the real\n - file and carry the actual error, for example:\n + Give a leak its own annotation. Point it at the test script, the exact\n + line isn't known, only which script the leak turned up in, and put the\n + full sanitizer report in a log group next to it, so it stays visible\n + and isn't capped to a handful of lines.\n \n - t/t1507-rev-parse-upstream.sh, line 1:\n - memory leak logged around t1507.1\n - ==ERROR: LeakSanitizer: detected memory leaks\n - Direct leak of 60 byte(s) in 1 object(s) allocated from:\n - ...\n - #5 in add_branch builtin/remote.c:135\n + Once a script has one leak, it keeps running: the sanitizer log\n + directory is never cleared between tests, so every later test in the\n + same script sees the same leftover log entries and also reports \"not\n + ok\", burying the one real failure in copies of itself. Stop a\n + leak-sanitizer script at its first failure with --immediate instead.\n +\n + A failing test already gets its own annotation once its script\n + finishes, but --immediate exits as soon as that test fails, before\n + reaching the code that writes it. Write the annotation first, so\n + turning on --immediate here does not silently drop it.\n \n Signed-off-by: Harald Nordgren \n \n @@ t/test-lib-github-workflow-markup.sh: start_test_output () {\n \t>$github_markup_output\n \tGIT_TEST_TEE_OFFSET=0\n +\tgithub_markup_script_name=${0##*/}\n ++}\n ++\n ++github_annotation_ () {\n ++\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n \n # No need to override start_test_case_output\n -@@ t/test-lib-github-workflow-markup.sh: start_test_output () {\n - finalize_test_case_output () {\n - \ttest_case_result=$1\n - \tshift\n -+\n -+\tcase \"$test_case_result\" in\n -+\tok|broken)\n -+\t\t# Exit without printing the \"ok\" or \"broken\" tests\n -+\t\treturn\n -+\t\t;;\n -+\tesac\n -+\n -+\ttest_case_line=$(find_test_case_line_ \"$1\")\n -+\ttest_case_output=$(test-tool path-utils skip-n-bytes \\\n -+\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET)\n -+\n - \tcase \"$test_case_result\" in\n - \tfailure)\n --\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n -+\t\ttest_case_summary=$(printf '%s\\n' \"$test_case_output\" |\n -+\t\t\ttail -n 20 | github_escape_message_)\n -+\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n -+\t\t\t\"failed: $this_test.$test_count $1%0A%0A$test_case_summary\"\n - \t\t;;\n - \tfixed)\n --\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n --\t\t;;\n --\tok|broken)\n --\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n --\t\treturn\n -+\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n -+\t\t\t\"fixed: $this_test.$test_count $1\"\n - \t\t;;\n - \tesac\n -+\n - \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n --\ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n --\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n -+\tprintf '%s\\n' \"$test_case_output\" >>$github_markup_output\n +@@ t/test-lib-github-workflow-markup.sh: finalize_test_case_output () {\n \techo >>$github_markup_output \"::endgroup::\"\n }\n \n +finalize_test_leak_output () {\n -+\ttest_leak_summary=$(head -n 40 \"$TEST_RESULTS_SAN_FILE\".* |\n -+\t\tgithub_escape_message_)\n ++\t# The exact line the leak turned up on isn't known, only the script,\n ++\t# so point at line 1.\n +\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n -+\t\t\"memory leak logged around $this_test.$test_count%0A%0A$test_leak_summary\"\n -+}\n ++\t\t\"memory leak logged in $this_test\"\n +\n - # No need to override finalize_test_output\n -+\n -+github_escape_message_ () {\n -+\tsed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\\r/%0D/g' -e 's/\\n/%0A/g'\n -+}\n -+\n -+find_test_case_line_ () {\n -+\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n -+\thead -n 1 | cut -d: -f1\n ++\techo >>$github_markup_output \"::group::leak: $this_test.$test_count\"\n ++\tcat \"$TEST_RESULTS_SAN_FILE\".* >>$github_markup_output\n ++\techo >>$github_markup_output \"::endgroup::\"\n +}\n +\n -+github_annotation_ () {\n -+\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n -+}\n + # No need to override finalize_test_output\n \n ## t/test-lib.sh ##\n @@ t/test-lib.sh: mark_option_requires_arg () {\n @@ t/test-lib.sh: mark_option_requires_arg () {\n finalize_test_output () { :; }\n \n parse_option () {\n +@@ t/test-lib.sh: test_failure_ () {\n + \tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n + \tshift\n + \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n ++\tif test -n \"$immediate\" && test -n \"$invert_exit_code\"\n ++\tthen\n ++\t\tsay_color error \"1..$test_count\"\n ++\t\tfinalize_test_output\n ++\t\t_invert_exit_code_failure_end_blurb\n ++\t\tGIT_EXIT_OK=t\n ++\t\texit 0\n ++\tfi\n ++\t# Write the annotation before the --immediate exit paths below,\n ++\t# which call exit and would otherwise skip it.\n ++\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n + \tif test -n \"$immediate\"\n + \tthen\n + \t\tsay_color error \"1..$test_count\"\n +-\t\tif test -n \"$invert_exit_code\"\n +-\t\tthen\n +-\t\t\tfinalize_test_output\n +-\t\t\t_invert_exit_code_failure_end_blurb\n +-\t\t\tGIT_EXIT_OK=t\n +-\t\t\texit 0\n +-\t\tfi\n + \t\tcheck_test_results_san_file_ \"$test_failure\"\n + \t\t_error_exit\n + \tfi\n +-\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n + }\n + \n + test_known_broken_ok_ () {\n @@ t/test-lib.sh: check_test_results_san_file_ () {\n \t\treturn\n \tfi &&\n -: ---------- > 2: bffa8fb030 ci: point test failures and fixed known breakages at their file and line\n\n-- \ngitgitgadget\n"},{"id":"553513","messageId":"bffa8fb0309b3698bebaa9b4d4763acdea52a7a2.1790621693.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v2.git.git.1790621693.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-28T18:54:53Z","receivedAt":"2026-09-28T18:54:57Z","isPatch":true,"body":"From: Harald Nordgren \n\nA test failure or a fixed known breakage gets an annotation that names\nthe test but carries no file or line, so there is nothing to click\nthrough to from the GitHub UI.\n\nFind the line a test is defined on by searching the script for its\ndescription as a fixed string, using the first match. A description\ncan contain characters like `[` or `*` that a regex search would\nmisread, so match it literally. Fall back to line 1 when the\ndescription is not found verbatim, which happens when a test builds\nits description at runtime instead of writing it out literally.\n\nA GitHub annotation is a single line, and a test description is always\none line too, so only a `%` or a stray carriage return in it needs\npercent-encoding to keep the annotation intact. Escape `%` first, or a\ncarriage return's own encoding would be mangled by a `%` substitution\nthat ran after it.\n\nSigned-off-by: Harald Nordgren \n---\n t/test-lib-github-workflow-markup.sh | 41 ++++++++++++++++++++++------\n 1 file changed, 33 insertions(+), 8 deletions(-)\n\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex 0d54496358..67c5c3461c 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -31,6 +31,21 @@ start_test_output () {\n \tgithub_markup_script_name=${0##*/}\n }\n \n+github_escape_message_ () {\n+\t# A test description is always one line, so only % and CR need\n+\t# escaping here. Escape % first, or CR's own %-encoding gets mangled.\n+\tsed -e 's/%/%25/g' -e 's/\\r/%0D/g'\n+}\n+\n+find_test_case_line_ () {\n+\t# A description can contain characters like [ or * that would\n+\t# corrupt a regex search, so match it literally and take the first\n+\t# hit; -- keeps a description starting with \"-\" from being read as\n+\t# an option.\n+\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n+\thead -n 1 | cut -d: -f1\n+}\n+\n github_annotation_ () {\n \techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n@@ -40,21 +55,31 @@ github_annotation_ () {\n finalize_test_case_output () {\n \ttest_case_result=$1\n \tshift\n+\n+\tcase \"$test_case_result\" in\n+\tok|broken)\n+\t\t# Exit without printing the \"ok\" or \"broken\" tests\n+\t\treturn\n+\t\t;;\n+\tesac\n+\n+\ttest_case_line=$(find_test_case_line_ \"$1\")\n+\ttest_case_description=$(printf '%s' \"$1\" | github_escape_message_)\n+\n \tcase \"$test_case_result\" in\n \tfailure)\n-\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n+\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"failed: $this_test.$test_count $test_case_description\"\n \t\t;;\n \tfixed)\n-\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n-\t\t;;\n-\tok|broken)\n-\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n-\t\treturn\n+\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"fixed: $this_test.$test_count $test_case_description\"\n \t\t;;\n \tesac\n+\n \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n-\ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n-\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n+\ttest-tool path-utils skip-n-bytes \\\n+\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET >>$github_markup_output\n \techo >>$github_markup_output \"::endgroup::\"\n }\n \n-- \ngitgitgadget\n"},{"id":"553514","messageId":"46e13a6e77f0c1c23a0dfe6183e8c3dac405da89.1790621693.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v2.git.git.1790621693.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-28T18:54:52Z","receivedAt":"2026-09-28T18:54:59Z","isPatch":true,"body":"From: Harald Nordgren \n\nA leak is only discovered once, at the end of a whole script, well\nafter every test has already reported ok, and it gets no annotation at\nall, so a leak-sanitizer job's only visible failure is:\n\n Process completed with exit code 1.\n\nGive a leak its own annotation. Point it at the test script, the exact\nline isn't known, only which script the leak turned up in, and put the\nfull sanitizer report in a log group next to it, so it stays visible\nand isn't capped to a handful of lines.\n\nOnce a script has one leak, it keeps running: the sanitizer log\ndirectory is never cleared between tests, so every later test in the\nsame script sees the same leftover log entries and also reports \"not\nok\", burying the one real failure in copies of itself. Stop a\nleak-sanitizer script at its first failure with --immediate instead.\n\nA failing test already gets its own annotation once its script\nfinishes, but --immediate exits as soon as that test fails, before\nreaching the code that writes it. Write the annotation first, so\nturning on --immediate here does not silently drop it.\n\nSigned-off-by: Harald Nordgren \n---\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++\n t/test-lib.sh | 21 +++++++++++++--------\n 3 files changed, 30 insertions(+), 8 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex c6ccbf8c17..a89f480a78 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)\n \texport NO_CVS_TESTS=LetsSaveSomeTime\n \texport NO_SVN_TESTS=LetsSaveSomeTime\n \texport NO_P4_TESTS=LetsSaveSomeTime\n+\tGIT_TEST_OPTS=\"$GIT_TEST_OPTS --immediate\"\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex fa29a62aa3..0d54496358 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -28,6 +28,11 @@ start_test_output () {\n \tgithub_markup_output=\"${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup\"\n \t>$github_markup_output\n \tGIT_TEST_TEE_OFFSET=0\n+\tgithub_markup_script_name=${0##*/}\n+}\n+\n+github_annotation_ () {\n+\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n \n # No need to override start_test_case_output\n@@ -53,4 +58,15 @@ finalize_test_case_output () {\n \techo >>$github_markup_output \"::endgroup::\"\n }\n \n+finalize_test_leak_output () {\n+\t# The exact line the leak turned up on isn't known, only the script,\n+\t# so point at line 1.\n+\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n+\t\t\"memory leak logged in $this_test\"\n+\n+\techo >>$github_markup_output \"::group::leak: $this_test.$test_count\"\n+\tcat \"$TEST_RESULTS_SAN_FILE\".* >>$github_markup_output\n+\techo >>$github_markup_output \"::endgroup::\"\n+}\n+\n # No need to override finalize_test_output\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1f0505e412..3552a19323 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -199,6 +199,7 @@ mark_option_requires_arg () {\n start_test_output () { :; }\n start_test_case_output () { :; }\n finalize_test_case_output () { :; }\n+finalize_test_leak_output () { :; }\n finalize_test_output () { :; }\n \n parse_option () {\n@@ -822,20 +823,23 @@ test_failure_ () {\n \tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n+\tif test -n \"$immediate\" && test -n \"$invert_exit_code\"\n+\tthen\n+\t\tsay_color error \"1..$test_count\"\n+\t\tfinalize_test_output\n+\t\t_invert_exit_code_failure_end_blurb\n+\t\tGIT_EXIT_OK=t\n+\t\texit 0\n+\tfi\n+\t# Write the annotation before the --immediate exit paths below,\n+\t# which call exit and would otherwise skip it.\n+\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n \tif test -n \"$immediate\"\n \tthen\n \t\tsay_color error \"1..$test_count\"\n-\t\tif test -n \"$invert_exit_code\"\n-\t\tthen\n-\t\t\tfinalize_test_output\n-\t\t\t_invert_exit_code_failure_end_blurb\n-\t\t\tGIT_EXIT_OK=t\n-\t\t\texit 0\n-\t\tfi\n \t\tcheck_test_results_san_file_ \"$test_failure\"\n \t\t_error_exit\n \tfi\n-\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n }\n \n test_known_broken_ok_ () {\n@@ -1218,6 +1222,7 @@ check_test_results_san_file_ () {\n \t\treturn\n \tfi &&\n \tsay_color >&4 error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n+\tfinalize_test_leak_output &&\n \n \tif test \"$test_failure\" = 0\n \tthen\n-- \ngitgitgadget\n\n"},{"id":"553524","messageId":"xmqqtsn9kssi.fsf@gitster.g","threadId":"66392","inReplyTo":"46e13a6e77f0c1c23a0dfe6183e8c3dac405da89.1790621693.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-28T20:46:53Z","receivedAt":"2026-09-28T20:46:56Z","isPatch":true,"body":"\"Harald Nordgren via GitGitGadget\" writes:\n\n> From: Harald Nordgren \n>\n> A leak is only discovered once, at the end of a whole script, well\n> after every test has already reported ok, and it gets no annotation at\n> all, so a leak-sanitizer job's only visible failure is:\n>\n> Process completed with exit code 1.\n>\n> Give a leak its own annotation. Point it at the test script, the exact\n> line isn't known, only which script the leak turned up in, and put the\n> full sanitizer report in a log group next to it, so it stays visible\n> and isn't capped to a handful of lines.\n>\n> Once a script has one leak, it keeps running: the sanitizer log\n> directory is never cleared between tests, so every later test in the\n> same script sees the same leftover log entries and also reports \"not\n> ok\", burying the one real failure in copies of itself. Stop a\n> leak-sanitizer script at its first failure with --immediate instead.\n\nOK. So the idea is that we do not have sanitizer report per\ntest_expect_* block but showing the single one over and over,\nwhether the next test_expect_* block has leaks, is not helpful, so\nwe just immediately kill the test script after the first leak?\n\n> @@ -53,4 +58,15 @@ finalize_test_case_output () {\n> \techo >>$github_markup_output \"::endgroup::\"\n> }\n> \n> +finalize_test_leak_output () {\n> +\t# The exact line the leak turned up on isn't known, only the script,\n> +\t# so point at line 1.\n> +\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n> +\t\t\"memory leak logged in $this_test\"\n> +\n> +\techo >>$github_markup_output \"::group::leak: $this_test.$test_count\"\n> +\tcat \"$TEST_RESULTS_SAN_FILE\".* >>$github_markup_output\n> +\techo >>$github_markup_output \"::endgroup::\"\n> +}\n> +\n\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 1f0505e412..3552a19323 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -199,6 +199,7 @@ mark_option_requires_arg () {\n> start_test_output () { :; }\n> start_test_case_output () { :; }\n> finalize_test_case_output () { :; }\n> +finalize_test_leak_output () { :; }\n> finalize_test_output () { :; }\n> \n> parse_option () {\n> @@ -822,20 +823,23 @@ test_failure_ () {\n> \tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n> \tshift\n> \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n> +\tif test -n \"$immediate\" && test -n \"$invert_exit_code\"\n> +\tthen\n> +\t\tsay_color error \"1..$test_count\"\n> +\t\tfinalize_test_output\n> +\t\t_invert_exit_code_failure_end_blurb\n> +\t\tGIT_EXIT_OK=t\n> +\t\texit 0\n> +\tfi\n> +\t# Write the annotation before the --immediate exit paths below,\n> +\t# which call exit and would otherwise skip it.\n> +\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n> \tif test -n \"$immediate\"\n> \tthen\n> \t\tsay_color error \"1..$test_count\"\n> -\t\tif test -n \"$invert_exit_code\"\n> -\t\tthen\n> -\t\t\tfinalize_test_output\n> -\t\t\t_invert_exit_code_failure_end_blurb\n> -\t\t\tGIT_EXIT_OK=t\n> -\t\t\texit 0\n> -\t\tfi\n> \t\tcheck_test_results_san_file_ \"$test_failure\"\n> \t\t_error_exit\n> \tfi\n\nThe two-line comment in the middle made me puzzled to see \"exit 0\"\njust above it. If \"--immediate\" is asked and we are checking leaks,\nshouldn't we be doing finalize_test_case_output regardless of the\n\"invert\" setting?\n\n> -\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n> }\n> \n> test_known_broken_ok_ () {\n> @@ -1218,6 +1222,7 @@ check_test_results_san_file_ () {\n> \t\treturn\n> \tfi &&\n> \tsay_color >&4 error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n> +\tfinalize_test_leak_output &&\n> \n> \tif test \"$test_failure\" = 0\n> \tthen\n"},{"id":"553526","messageId":"xmqqpkxxkshj.fsf@gitster.g","threadId":"66392","inReplyTo":"bffa8fb0309b3698bebaa9b4d4763acdea52a7a2.1790621693.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-28T20:53:28Z","receivedAt":"2026-09-28T20:53:30Z","isPatch":true,"body":"\"Harald Nordgren via GitGitGadget\" writes:\n\n> t/test-lib-github-workflow-markup.sh | 41 ++++++++++++++++++++++------\n> 1 file changed, 33 insertions(+), 8 deletions(-)\n>\n> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\n> index 0d54496358..67c5c3461c 100644\n> --- a/t/test-lib-github-workflow-markup.sh\n> +++ b/t/test-lib-github-workflow-markup.sh\n> @@ -31,6 +31,21 @@ start_test_output () {\n> \tgithub_markup_script_name=${0##*/}\n> }\n> \n> +github_escape_message_ () {\n> +\t# A test description is always one line, so only % and CR need\n> +\t# escaping here. Escape % first, or CR's own %-encoding gets mangled.\n> +\tsed -e 's/%/%25/g' -e 's/\\r/%0D/g'\n> +}\n\nIs it portable to feed a two-letter sequence \"\\r\" to \"sed\" and\nexpect it to be interpreted as Carriage Return? Implementations of\nBSD lineage \"sed\" don't grok it if I recall correctly.\n\n> +find_test_case_line_ () {\n> +\t# A description can contain characters like [ or * that would\n> +\t# corrupt a regex search, so match it literally and take the first\n> +\t# hit; -- keeps a description starting with \"-\" from being read as\n> +\t# an option.\n> +\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n> +\thead -n 1 | cut -d: -f1\n> +}\n> +\n> github_annotation_ () {\n> \techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n> }\n> @@ -40,21 +55,31 @@ github_annotation_ () {\n> finalize_test_case_output () {\n> \ttest_case_result=$1\n> \tshift\n> +\n> +\tcase \"$test_case_result\" in\n> +\tok|broken)\n> +\t\t# Exit without printing the \"ok\" or \"broken\" tests\n> +\t\treturn\n> +\t\t;;\n> +\tesac\n> +\n> +\ttest_case_line=$(find_test_case_line_ \"$1\")\n> +\ttest_case_description=$(printf '%s' \"$1\" | github_escape_message_)\n> +\n> \tcase \"$test_case_result\" in\n> \tfailure)\n> -\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n> +\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> +\t\t\t\"failed: $this_test.$test_count $test_case_description\"\n> \t\t;;\n> \tfixed)\n> -\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n> -\t\t;;\n> -\tok|broken)\n> -\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n> -\t\treturn\n> +\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> +\t\t\t\"fixed: $this_test.$test_count $test_case_description\"\n> \t\t;;\n> \tesac\n> +\n\nAll of the above may make sense, but ...\n\n> \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n> -\ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n> -\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n> +\ttest-tool path-utils skip-n-bytes \\\n> +\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET >>$github_markup_output\n> \techo >>$github_markup_output \"::endgroup::\"\n\nWhat is this change about? In the original, all surrounding code\nhas redirection early on the command line, and breaking that pattern\nis the only difference between the removed and added lines here as\nfar as I can see.\n\n> }\n"},{"id":"553558","messageId":"CAHwyqnXX-kxDmsE+uiVHZ3=6iKvNkWkd5VkK1tRmFBS7fGWmxQ@mail.gmail.com","threadId":"66392","inReplyTo":"xmqqtsn9kssi.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2026-09-29T07:47:55Z","receivedAt":"2026-09-29T07:48:34Z","isPatch":true,"body":"> > Once a script has one leak, it keeps running: the sanitizer log\n> > directory is never cleared between tests, so every later test in the\n> > same script sees the same leftover log entries and also reports \"not\n> > ok\", burying the one real failure in copies of itself. Stop a\n> > leak-sanitizer script at its first failure with --immediate instead.\n>\n> OK. So the idea is that we do not have sanitizer report per\n> test_expect_* block but showing the single one over and over,\n> whether the next test_expect_* block has leaks, is not helpful, so\n> we just immediately kill the test script after the first leak?\n\nYes that's it, one leak makes continuing pointless since every later\ntest would just see the same accumulated log, so we stop there\ninstead.\n\n> > if test -n \"$immediate\"\n> > then\n> > say_color error \"1..$test_count\"\n> > - if test -n \"$invert_exit_code\"\n> > - then\n> > - finalize_test_output\n> > - _invert_exit_code_failure_end_blurb\n> > - GIT_EXIT_OK=t\n> > - exit 0\n> > - fi\n> > check_test_results_san_file_ \"$test_failure\"\n> > _error_exit\n> > fi\n>\n> The two-line comment in the middle made me puzzled to see \"exit 0\"\n> just above it. If \"--immediate\" is asked and we are checking leaks,\n> shouldn't we be doing finalize_test_case_output regardless of the\n> \"invert\" setting?\n\nI'll take a look at that, it might be a problem.\n\n\nHarald\n"},{"id":"553670","messageId":"pull.2419.v3.git.git.1790748583.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.git.git.1790362443893.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] ci: link failure and leak annotations to the test script","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-30T06:09:41Z","receivedAt":"2026-09-30T06:09:46Z","isPatch":true,"body":"Link failure and leak annotations in CI to the test script, so both can be\nfound from the job summary.\n\nV3 CI Job where failures and leaks are reported:\nhttps://github.com/git/git/actions/runs/36537917146/job/109306215909?pr=2426\n\nChanges in v3:\n\n * Fixed bug in the --immediate exit ordering: the --immediate &&\n --invert-exit-code path called exit 0 before the test's annotation was\n written, now a single unconditional call covers both exit paths.\n * github_escape_message_ no longer relies on \\r being a portable sed escape\n sequence (not POSIX-guaranteed and BSD sed implementations can differ),\n it splices in the literal carriage-return byte via printf instead.\n * Reverted unrelated test-tool line back to its original form.\n\nChanges in v2:\n\n * Split into two commits, each explaining its own reasoning.\n * Leak output is no longer capped or embedded in the message, it's now an\n uncapped fold, so multiple leaks in the same test both show in full. A\n second leak in a different test still won't show in the same run,\n --immediate stops the script at the first failure, but it no longer gets\n buried under every later test falsely reporting \"not ok\" either.\n * Drops the giant unfolded message that annotations used to carry, which is\n what probably caused the scrolling behavior.\n\nHarald Nordgren (2):\n ci: annotate leaks and stop a leak-sanitizer script at its first\n failure\n ci: point test failures and fixed known breakages at their file and\n line\n\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 54 ++++++++++++++++++++++++----\n t/test-lib.sh | 6 +++-\n 3 files changed, 54 insertions(+), 7 deletions(-)\n\n\nbase-commit: a018953688f1b10bddf91bff8747068f5f4746a4\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v3\nPull-Request: https://github.com/git/git/pull/2419\n\nRange-diff vs v2:\n\n 1: 46e13a6e77 ! 1: b6a36820ae ci: annotate leaks and stop a leak-sanitizer script at its first failure\n @@ t/test-lib.sh: test_failure_ () {\n \tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n -+\tif test -n \"$immediate\" && test -n \"$invert_exit_code\"\n -+\tthen\n -+\t\tsay_color error \"1..$test_count\"\n -+\t\tfinalize_test_output\n -+\t\t_invert_exit_code_failure_end_blurb\n -+\t\tGIT_EXIT_OK=t\n -+\t\texit 0\n -+\tfi\n -+\t# Write the annotation before the --immediate exit paths below,\n -+\t# which call exit and would otherwise skip it.\n ++\t# Write the annotation before either --immediate exit path below,\n ++\t# both of which call exit and would otherwise skip it.\n +\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n \tif test -n \"$immediate\"\n \tthen\n \t\tsay_color error \"1..$test_count\"\n --\t\tif test -n \"$invert_exit_code\"\n --\t\tthen\n --\t\t\tfinalize_test_output\n --\t\t\t_invert_exit_code_failure_end_blurb\n --\t\t\tGIT_EXIT_OK=t\n --\t\t\texit 0\n --\t\tfi\n +@@ t/test-lib.sh: test_failure_ () {\n \t\tcheck_test_results_san_file_ \"$test_failure\"\n \t\t_error_exit\n \tfi\n 2: bffa8fb030 ! 2: 750c360512 ci: point test failures and fixed known breakages at their file and line\n @@ t/test-lib-github-workflow-markup.sh: start_test_output () {\n +github_escape_message_ () {\n +\t# A test description is always one line, so only % and CR need\n +\t# escaping here. Escape % first, or CR's own %-encoding gets mangled.\n -+\tsed -e 's/%/%25/g' -e 's/\\r/%0D/g'\n ++\t# \\r is not a portable sed escape, so splice in the actual byte.\n ++\tsed -e 's/%/%25/g' -e \"s/$(printf '\\r')/%0D/g\"\n +}\n +\n +find_test_case_line_ () {\n @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {\n \tesac\n +\n \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n --\ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n --\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n -+\ttest-tool path-utils skip-n-bytes \\\n -+\t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET >>$github_markup_output\n - \techo >>$github_markup_output \"::endgroup::\"\n - }\n - \n + \ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n + \t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n\n-- \ngitgitgadget\n"},{"id":"553671","messageId":"b6a36820ae3c50e36d71f751b7ff25b7f3275cea.1790748583.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v3.git.git.1790748583.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-30T06:09:42Z","receivedAt":"2026-09-30T06:09:47Z","isPatch":true,"body":"From: Harald Nordgren \n\nA leak is only discovered once, at the end of a whole script, well\nafter every test has already reported ok, and it gets no annotation at\nall, so a leak-sanitizer job's only visible failure is:\n\n Process completed with exit code 1.\n\nGive a leak its own annotation. Point it at the test script, the exact\nline isn't known, only which script the leak turned up in, and put the\nfull sanitizer report in a log group next to it, so it stays visible\nand isn't capped to a handful of lines.\n\nOnce a script has one leak, it keeps running: the sanitizer log\ndirectory is never cleared between tests, so every later test in the\nsame script sees the same leftover log entries and also reports \"not\nok\", burying the one real failure in copies of itself. Stop a\nleak-sanitizer script at its first failure with --immediate instead.\n\nA failing test already gets its own annotation once its script\nfinishes, but --immediate exits as soon as that test fails, before\nreaching the code that writes it. Write the annotation first, so\nturning on --immediate here does not silently drop it.\n\nSigned-off-by: Harald Nordgren \n---\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++\n t/test-lib.sh | 6 +++++-\n 3 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex c6ccbf8c17..a89f480a78 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)\n \texport NO_CVS_TESTS=LetsSaveSomeTime\n \texport NO_SVN_TESTS=LetsSaveSomeTime\n \texport NO_P4_TESTS=LetsSaveSomeTime\n+\tGIT_TEST_OPTS=\"$GIT_TEST_OPTS --immediate\"\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex fa29a62aa3..0d54496358 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -28,6 +28,11 @@ start_test_output () {\n \tgithub_markup_output=\"${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup\"\n \t>$github_markup_output\n \tGIT_TEST_TEE_OFFSET=0\n+\tgithub_markup_script_name=${0##*/}\n+}\n+\n+github_annotation_ () {\n+\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n \n # No need to override start_test_case_output\n@@ -53,4 +58,15 @@ finalize_test_case_output () {\n \techo >>$github_markup_output \"::endgroup::\"\n }\n \n+finalize_test_leak_output () {\n+\t# The exact line the leak turned up on isn't known, only the script,\n+\t# so point at line 1.\n+\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n+\t\t\"memory leak logged in $this_test\"\n+\n+\techo >>$github_markup_output \"::group::leak: $this_test.$test_count\"\n+\tcat \"$TEST_RESULTS_SAN_FILE\".* >>$github_markup_output\n+\techo >>$github_markup_output \"::endgroup::\"\n+}\n+\n # No need to override finalize_test_output\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1f0505e412..e74a12f1dd 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -199,6 +199,7 @@ mark_option_requires_arg () {\n start_test_output () { :; }\n start_test_case_output () { :; }\n finalize_test_case_output () { :; }\n+finalize_test_leak_output () { :; }\n finalize_test_output () { :; }\n \n parse_option () {\n@@ -822,6 +823,9 @@ test_failure_ () {\n \tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n+\t# Write the annotation before either --immediate exit path below,\n+\t# both of which call exit and would otherwise skip it.\n+\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n \tif test -n \"$immediate\"\n \tthen\n \t\tsay_color error \"1..$test_count\"\n@@ -835,7 +839,6 @@ test_failure_ () {\n \t\tcheck_test_results_san_file_ \"$test_failure\"\n \t\t_error_exit\n \tfi\n-\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n }\n \n test_known_broken_ok_ () {\n@@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {\n \t\treturn\n \tfi &&\n \tsay_color >&4 error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n+\tfinalize_test_leak_output &&\n \n \tif test \"$test_failure\" = 0\n \tthen\n-- \ngitgitgadget\n\n"},{"id":"553672","messageId":"750c3605128c268f331b1b9477ca0489ced75543.1790748583.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v3.git.git.1790748583.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-30T06:09:43Z","receivedAt":"2026-09-30T06:09:49Z","isPatch":true,"body":"From: Harald Nordgren \n\nA test failure or a fixed known breakage gets an annotation that names\nthe test but carries no file or line, so there is nothing to click\nthrough to from the GitHub UI.\n\nFind the line a test is defined on by searching the script for its\ndescription as a fixed string, using the first match. A description\ncan contain characters like `[` or `*` that a regex search would\nmisread, so match it literally. Fall back to line 1 when the\ndescription is not found verbatim, which happens when a test builds\nits description at runtime instead of writing it out literally.\n\nA GitHub annotation is a single line, and a test description is always\none line too, so only a `%` or a stray carriage return in it needs\npercent-encoding to keep the annotation intact. Escape `%` first, or a\ncarriage return's own encoding would be mangled by a `%` substitution\nthat ran after it.\n\nSigned-off-by: Harald Nordgren \n---\n t/test-lib-github-workflow-markup.sh | 38 +++++++++++++++++++++++-----\n 1 file changed, 32 insertions(+), 6 deletions(-)\n\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex 0d54496358..66d2ccca18 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -31,6 +31,22 @@ start_test_output () {\n \tgithub_markup_script_name=${0##*/}\n }\n \n+github_escape_message_ () {\n+\t# A test description is always one line, so only % and CR need\n+\t# escaping here. Escape % first, or CR's own %-encoding gets mangled.\n+\t# \\r is not a portable sed escape, so splice in the actual byte.\n+\tsed -e 's/%/%25/g' -e \"s/$(printf '\\r')/%0D/g\"\n+}\n+\n+find_test_case_line_ () {\n+\t# A description can contain characters like [ or * that would\n+\t# corrupt a regex search, so match it literally and take the first\n+\t# hit; -- keeps a description starting with \"-\" from being read as\n+\t# an option.\n+\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n+\thead -n 1 | cut -d: -f1\n+}\n+\n github_annotation_ () {\n \techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n@@ -40,18 +56,28 @@ github_annotation_ () {\n finalize_test_case_output () {\n \ttest_case_result=$1\n \tshift\n+\n+\tcase \"$test_case_result\" in\n+\tok|broken)\n+\t\t# Exit without printing the \"ok\" or \"broken\" tests\n+\t\treturn\n+\t\t;;\n+\tesac\n+\n+\ttest_case_line=$(find_test_case_line_ \"$1\")\n+\ttest_case_description=$(printf '%s' \"$1\" | github_escape_message_)\n+\n \tcase \"$test_case_result\" in\n \tfailure)\n-\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n+\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"failed: $this_test.$test_count $test_case_description\"\n \t\t;;\n \tfixed)\n-\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n-\t\t;;\n-\tok|broken)\n-\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n-\t\treturn\n+\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"fixed: $this_test.$test_count $test_case_description\"\n \t\t;;\n \tesac\n+\n \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n \ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n \t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n-- \ngitgitgadget\n"},{"id":"553708","messageId":"xmqqpkxudcva.fsf@gitster.g","threadId":"66392","inReplyTo":"pull.2419.v3.git.git.1790748583.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/2] ci: link failure and leak annotations to the test script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-30T14:37:13Z","receivedAt":"2026-09-30T14:37:18Z","isPatch":true,"body":"\"Harald Nordgren via GitGitGadget\" writes:\n\n> Link failure and leak annotations in CI to the test script, so both can be\n> found from the job summary.\n>\n> V3 CI Job where failures and leaks are reported:\n> https://github.com/git/git/actions/runs/36537917146/job/109306215909?pr=2426\n>\n> Changes in v3:\n>\n> * Fixed bug in the --immediate exit ordering: the --immediate &&\n> --invert-exit-code path called exit 0 before the test's annotation was\n> written, now a single unconditional call covers both exit paths.\n> * github_escape_message_ no longer relies on \\r being a portable sed escape\n> sequence (not POSIX-guaranteed and BSD sed implementations can differ),\n> it splices in the literal carriage-return byte via printf instead.\n> * Reverted unrelated test-tool line back to its original form.\n\nWith these updates, the patches look good to me. Unless others\nspot problems I failed to see, let me mark the topic for 'next'.\n\nThanks.\n"},{"id":"553712","messageId":"37628f91-eb36-426b-9f6b-b2083f917919@gmail.com","threadId":"66392","inReplyTo":"b6a36820ae3c50e36d71f751b7ff25b7f3275cea.1790748583.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-09-30T14:56:29Z","receivedAt":"2026-09-30T14:56:41Z","isPatch":true,"body":"Hi Harald\n\nOn 30/09/2026 07:09, Harald Nordgren via GitGitGadget wrote:\n> From: Harald Nordgren \n> \n> A leak is only discovered once, at the end of a whole script, well\n> after every test has already reported ok, and it gets no annotation at\n> all, so a leak-sanitizer job's only visible failure is:\n> \n> Process completed with exit code 1.\n> \n> Give a leak its own annotation. Point it at the test script, the exact\n> line isn't known, only which script the leak turned up in, and put the\n> full sanitizer report in a log group next to it, so it stays visible\n> and isn't capped to a handful of lines.\n\nIt is good that it reports the full LSAN but is it really necessary to \nemphasize that - why would anyone reading this think it might be \nabbreviated?\n\nWhat does adding the file and line number to the annotation buy us? I \nwas hoping there'd be a link in the output to the test source but I \ncan't see anything like that. The output is displayed as\n\n Error: failed: t7603.3 pull c2, c3, c4, c5 into c1\n ▶failure: t7603.3 pull c2, c3, c4, c5 into c1\n Error: memory leak logged in t7603\n ▶leak: t7603.3\n\nand clicking on the '▶' lines expands the output, but there is nothing \nabout the source file as far as I can see. Ideally, if the test is \nfailing due to a leak, it would be nice to report that as\n\n Error: leak detected in: t7603.3 pull c2, c3, c4, c5 into c1\n ▶failure: t7603.3 pull c2, c3, c4, c5 into c1\n\nand display the test output and LSAN output together when the \n\"▶failure:\" line is clicked, rather than having separate sections for \nthe test output and leak output. Having said that what you've already \nimplemented is a clear improvement so I'd be happy to take that if you \ndon't feel like devoting any more time to it.\n\n> Once a script has one leak, it keeps running: the sanitizer log\n> directory is never cleared between tests, so every later test in the\n> same script sees the same leftover log entries and also reports \"not\n> ok\", burying the one real failure in copies of itself. Stop a\n> leak-sanitizer script at its first failure with --immediate instead.\n\nI think that is probably a welcome improvement though if two different \ntests have different leaks it would be nice to be able to show both.\n\nThanks for working on this, it makes the LSAN output much more accessible.\n\nPhillip\n\n> A failing test already gets its own annotation once its script\n> finishes, but --immediate exits as soon as that test fails, before\n> reaching the code that writes it. Write the annotation first, so\n> turning on --immediate here does not silently drop it.\n> \n> Signed-off-by: Harald Nordgren \n> ---\n> ci/lib.sh | 1 +\n> t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++\n> t/test-lib.sh | 6 +++++-\n> 3 files changed, 22 insertions(+), 1 deletion(-)\n> \n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index c6ccbf8c17..a89f480a78 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)\n> \texport NO_CVS_TESTS=LetsSaveSomeTime\n> \texport NO_SVN_TESTS=LetsSaveSomeTime\n> \texport NO_P4_TESTS=LetsSaveSomeTime\n> +\tGIT_TEST_OPTS=\"$GIT_TEST_OPTS --immediate\"\n> \t;;\n> linux-asan-ubsan)\n> \texport SANITIZE=address,undefined\n> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\n> index fa29a62aa3..0d54496358 100644\n> --- a/t/test-lib-github-workflow-markup.sh\n> +++ b/t/test-lib-github-workflow-markup.sh\n> @@ -28,6 +28,11 @@ start_test_output () {\n> \tgithub_markup_output=\"${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup\"\n> \t>$github_markup_output\n> \tGIT_TEST_TEE_OFFSET=0\n> +\tgithub_markup_script_name=${0##*/}\n> +}\n> +\n> +github_annotation_ () {\n> +\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n> }\n> \n> # No need to override start_test_case_output\n> @@ -53,4 +58,15 @@ finalize_test_case_output () {\n> \techo >>$github_markup_output \"::endgroup::\"\n> }\n> \n> +finalize_test_leak_output () {\n> +\t# The exact line the leak turned up on isn't known, only the script,\n> +\t# so point at line 1.\n> +\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n> +\t\t\"memory leak logged in $this_test\"\n> +\n> +\techo >>$github_markup_output \"::group::leak: $this_test.$test_count\"\n> +\tcat \"$TEST_RESULTS_SAN_FILE\".* >>$github_markup_output\n> +\techo >>$github_markup_output \"::endgroup::\"\n> +}\n> +\n> # No need to override finalize_test_output\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 1f0505e412..e74a12f1dd 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -199,6 +199,7 @@ mark_option_requires_arg () {\n> start_test_output () { :; }\n> start_test_case_output () { :; }\n> finalize_test_case_output () { :; }\n> +finalize_test_leak_output () { :; }\n> finalize_test_output () { :; }\n> \n> parse_option () {\n> @@ -822,6 +823,9 @@ test_failure_ () {\n> \tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n> \tshift\n> \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n> +\t# Write the annotation before either --immediate exit path below,\n> +\t# both of which call exit and would otherwise skip it.\n> +\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n> \tif test -n \"$immediate\"\n> \tthen\n> \t\tsay_color error \"1..$test_count\"\n> @@ -835,7 +839,6 @@ test_failure_ () {\n> \t\tcheck_test_results_san_file_ \"$test_failure\"\n> \t\t_error_exit\n> \tfi\n> -\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n> }\n> \n> test_known_broken_ok_ () {\n> @@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {\n> \t\treturn\n> \tfi &&\n> \tsay_color >&4 error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n> +\tfinalize_test_leak_output &&\n> \n> \tif test \"$test_failure\" = 0\n> \tthen\n\n"},{"id":"553713","messageId":"5529bccf-eeb1-40f9-ae03-8fa19dc26f5a@gmail.com","threadId":"66392","inReplyTo":"750c3605128c268f331b1b9477ca0489ced75543.1790748583.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-09-30T14:56:39Z","receivedAt":"2026-09-30T14:56:48Z","isPatch":true,"body":"Hi Harald\n\nOn 30/09/2026 07:09, Harald Nordgren via GitGitGadget wrote:\n> From: Harald Nordgren \n> \n> A test failure or a fixed known breakage gets an annotation that names\n> the test but carries no file or line, so there is nothing to click\n> through to from the GitHub UI.\n\nHave you got an example of this? As I said in my last mail, I can't see \nany links in the output from the linux-leaks job.\n\n> Find the line a test is defined on by searching the script for its\n> description as a fixed string, using the first match. A description\n> can contain characters like `[` or `*` that a regex search would\n> misread, so match it literally. \n\nThis second sentence doesn't really add anything - you've already said \nwe're searching for a fixed string.\n\n> Fall back to line 1 when the\n> description is not found verbatim, which happens when a test builds\n> its description at runtime instead of writing it out literally.\n\nIronically, it is the dynamically generated tests where a line number \nwould be most useful, but there is no easy way to determine what line we \nshould be using.\n\n> A GitHub annotation is a single line, and a test description is always\n> one line too, so only a `%` or a stray carriage return in it needs\n> percent-encoding to keep the annotation intact. Escape `%` first, or a\n> carriage return's own encoding would be mangled by a `%` substitution\n> that ran after it.\n\nWhy do we need to escape the test descriptions when we haven't been \ndoing so up to now? Also if the test description is a single line why \nare we worring about '\\r'? If it is so important to escape the output \nwhy does this patch not convert the existing annotations like the \n\"group::\" on in the trailing context lines?\n\nThanks\n\nPhillip\n\n> Signed-off-by: Harald Nordgren \n> ---\n> t/test-lib-github-workflow-markup.sh | 38 +++++++++++++++++++++++-----\n> 1 file changed, 32 insertions(+), 6 deletions(-)\n> \n> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\n> index 0d54496358..66d2ccca18 100644\n> --- a/t/test-lib-github-workflow-markup.sh\n> +++ b/t/test-lib-github-workflow-markup.sh\n> @@ -31,6 +31,22 @@ start_test_output () {\n> \tgithub_markup_script_name=${0##*/}\n> }\n> \n> +github_escape_message_ () {\n> +\t# A test description is always one line, so only % and CR need\n> +\t# escaping here. Escape % first, or CR's own %-encoding gets mangled.\n> +\t# \\r is not a portable sed escape, so splice in the actual byte.\n> +\tsed -e 's/%/%25/g' -e \"s/$(printf '\\r')/%0D/g\"\n> +}\n> +\n> +find_test_case_line_ () {\n> +\t# A description can contain characters like [ or * that would\n> +\t# corrupt a regex search, so match it literally and take the first\n> +\t# hit; -- keeps a description starting with \"-\" from being read as\n> +\t# an option.\n> +\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n> +\thead -n 1 | cut -d: -f1\n> +}\n> +\n> github_annotation_ () {\n> \techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n> }\n> @@ -40,18 +56,28 @@ github_annotation_ () {\n> finalize_test_case_output () {\n> \ttest_case_result=$1\n> \tshift\n> +\n> +\tcase \"$test_case_result\" in\n> +\tok|broken)\n> +\t\t# Exit without printing the \"ok\" or \"broken\" tests\n> +\t\treturn\n> +\t\t;;\n> +\tesac\n> +\n> +\ttest_case_line=$(find_test_case_line_ \"$1\")\n> +\ttest_case_description=$(printf '%s' \"$1\" | github_escape_message_)\n> +\n> \tcase \"$test_case_result\" in\n> \tfailure)\n> -\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n> +\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> +\t\t\t\"failed: $this_test.$test_count $test_case_description\"\n> \t\t;;\n> \tfixed)\n> -\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n> -\t\t;;\n> -\tok|broken)\n> -\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n> -\t\treturn\n> +\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> +\t\t\t\"fixed: $this_test.$test_count $test_case_description\"\n> \t\t;;\n> \tesac\n> +\n> \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n> \ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n> \t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n\n"},{"id":"553722","messageId":"acc4ad5b-1ac7-4a63-b771-fc2e585a6ebf@gmail.com","threadId":"66392","inReplyTo":"xmqqpkxudcva.fsf@gitster.g","subject":"Re: [PATCH v3 0/2] ci: link failure and leak annotations to the test script","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-09-30T15:52:17Z","receivedAt":"2026-09-30T15:52:26Z","isPatch":true,"body":"On 30/09/2026 15:37, Junio C Hamano wrote:\n> \"Harald Nordgren via GitGitGadget\" writes:\n> \n> With these updates, the patches look good to me. Unless others\n> spot problems I failed to see, let me mark the topic for 'next'.\nI've left a couple of comments. This version is a nice improvement on \nthe status quo, but I'd like some clarity on what the filename and line \nnumber annotations actually do, and why we selectively escape the \nannotations.\n\nThanks\n\nPhillip\n"},{"id":"553741","messageId":"CAHwyqnVt=mSG9u6a_FtuqHDSM+8RVKRNmow7f7MtDvFV4EyOhQ@mail.gmail.com","threadId":"66392","inReplyTo":"5529bccf-eeb1-40f9-ae03-8fa19dc26f5a@gmail.com","subject":"Re: [PATCH v3 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2026-09-30T18:40:27Z","receivedAt":"2026-09-30T18:41:05Z","isPatch":true,"body":"On Wed, Sep 30, 2026 at 4:56 PM Phillip Wood wrote:\n>\n> Hi Harald\n>\n> On 30/09/2026 07:09, Harald Nordgren via GitGitGadget wrote:\n> > From: Harald Nordgren \n> >\n> > A test failure or a fixed known breakage gets an annotation that names\n> > the test but carries no file or line, so there is nothing to click\n> > through to from the GitHub UI.\n>\n> Have you got an example of this? As I said in my last mail, I can't see\n> any links in the output from the linux-leaks job.\n\nThat's poor wording on my side, I'll clarify.\n\n> Why do we need to escape the test descriptions when we haven't been\n> doing so up to now? Also if the test description is a single line why\n> are we worring about '\\r'? If it is so important to escape the output\n> why does this patch not convert the existing annotations like the\n> \"group::\" on in the trailing context lines?\n\nYeah, that can be simplified.\n\n\nHarald\n"},{"id":"553743","messageId":"xmqqld8ia86m.fsf@gitster.g","threadId":"66392","inReplyTo":"acc4ad5b-1ac7-4a63-b771-fc2e585a6ebf@gmail.com","subject":"Re: [PATCH v3 0/2] ci: link failure and leak annotations to the test script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-30T18:46:41Z","receivedAt":"2026-09-30T18:46:43Z","isPatch":true,"body":"Phillip Wood writes:\n\n> On 30/09/2026 15:37, Junio C Hamano wrote:\n>> \"Harald Nordgren via GitGitGadget\" writes:\n>> \n>> With these updates, the patches look good to me. Unless others\n>> spot problems I failed to see, let me mark the topic for 'next'.\n> I've left a couple of comments. This version is a nice improvement on \n> the status quo, but I'd like some clarity on what the filename and line \n> number annotations actually do, and why we selectively escape the \n> annotations.\n>\n> Thanks\n>\n> Phillip\n\nThanks.\n"},{"id":"553861","messageId":"pull.2419.v4.git.git.1790880255.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.git.git.1790362443893.gitgitgadget@gmail.com","subject":"[PATCH v4 0/2] ci: link failure and leak annotations to the test script","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-01T18:44:13Z","receivedAt":"2026-10-01T18:44:19Z","isPatch":true,"body":"Link failure and leak annotations in CI to the test script, so both can be\nfound from the job summary.\n\nV4 CI Job where failures and leaks are reported:\nhttps://github.com/git/git/actions/runs/36760014277/job/110039964565?pr=2426\n\nChanges in v4:\n\n * Clarify commit messages and simplify escaping logic.\n\nChanges in v3:\n\n * Fixed bug in the --immediate exit ordering: the --immediate &&\n --invert-exit-code path called exit 0 before the test's annotation was\n written, now a single unconditional call covers both exit paths.\n * github_escape_message_ no longer relies on \\r being a portable sed escape\n sequence (not POSIX-guaranteed and BSD sed implementations can differ),\n it splices in the literal carriage-return byte via printf instead.\n * Reverted unrelated test-tool line back to its original form.\n\nChanges in v2:\n\n * Split into two commits, each explaining its own reasoning.\n * Leak output is no longer capped or embedded in the message, it's now an\n uncapped fold, so multiple leaks in the same test both show in full. A\n second leak in a different test still won't show in the same run,\n --immediate stops the script at the first failure, but it no longer gets\n buried under every later test falsely reporting \"not ok\" either.\n * Drops the giant unfolded message that annotations used to carry, which is\n what probably caused the scrolling behavior.\n\nHarald Nordgren (2):\n ci: annotate leaks and stop a leak-sanitizer script at its first\n failure\n ci: point test failures and fixed known breakages at their file and\n line\n\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 54 ++++++++++++++++++++++++----\n t/test-lib.sh | 6 +++-\n 3 files changed, 54 insertions(+), 7 deletions(-)\n\n\nbase-commit: a018953688f1b10bddf91bff8747068f5f4746a4\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v4\nPull-Request: https://github.com/git/git/pull/2419\n\nRange-diff vs v3:\n\n 1: b6a36820ae ! 1: 138394b48b ci: annotate leaks and stop a leak-sanitizer script at its first failure\n @@ Commit message\n \n Give a leak its own annotation. Point it at the test script, the exact\n line isn't known, only which script the leak turned up in, and put the\n - full sanitizer report in a log group next to it, so it stays visible\n - and isn't capped to a handful of lines.\n + sanitizer report in a log group next to it, so it stays visible.\n \n Once a script has one leak, it keeps running: the sanitizer log\n directory is never cleared between tests, so every later test in the\n 2: 750c360512 ! 2: 8ec2b53d82 ci: point test failures and fixed known breakages at their file and line\n @@ Commit message\n ci: point test failures and fixed known breakages at their file and line\n \n A test failure or a fixed known breakage gets an annotation that names\n - the test but carries no file or line, so there is nothing to click\n - through to from the GitHub UI.\n + the test but says nothing about where it's defined, so a reviewer has\n + to search the script by hand to find it.\n \n Find the line a test is defined on by searching the script for its\n - description as a fixed string, using the first match. A description\n - can contain characters like `[` or `*` that a regex search would\n - misread, so match it literally. Fall back to line 1 when the\n - description is not found verbatim, which happens when a test builds\n - its description at runtime instead of writing it out literally.\n + description as a fixed string, using the first match. Fall back to\n + line 1 when the description is not found verbatim, which happens when\n + a test builds its description at runtime instead of writing it out\n + literally.\n \n - A GitHub annotation is a single line, and a test description is always\n - one line too, so only a `%` or a stray carriage return in it needs\n - percent-encoding to keep the annotation intact. Escape `%` first, or a\n - carriage return's own encoding would be mangled by a `%` substitution\n - that ran after it.\n + A GitHub annotation is a single line, so a `%` in a test description\n + has to be percent-encoded as `%25`, or GitHub misreads it as its own\n + escape sequence. for-each-ref's format atoms use plenty of them, e.g.\n + `%(raw)`.\n \n Signed-off-by: Harald Nordgren \n \n @@ t/test-lib-github-workflow-markup.sh: start_test_output () {\n }\n \n +github_escape_message_ () {\n -+\t# A test description is always one line, so only % and CR need\n -+\t# escaping here. Escape % first, or CR's own %-encoding gets mangled.\n -+\t# \\r is not a portable sed escape, so splice in the actual byte.\n -+\tsed -e 's/%/%25/g' -e \"s/$(printf '\\r')/%0D/g\"\n ++\t# % has to be escaped or GitHub misreads it as the start of its own\n ++\t# percent-encoding (e.g. a literal %(raw) in a for-each-ref test\n ++\t# description).\n ++\tsed -e 's/%/%25/g'\n +}\n +\n +find_test_case_line_ () {\n +\t# A description can contain characters like [ or * that would\n +\t# corrupt a regex search, so match it literally and take the first\n -+\t# hit; -- keeps a description starting with \"-\" from being read as\n -+\t# an option.\n ++\t# hit. The -- keeps a description starting with \"-\" from being read\n ++\t# as an option.\n +\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n +\thead -n 1 | cut -d: -f1\n +}\n\n-- \ngitgitgadget\n"},{"id":"553862","messageId":"138394b48b2422a54fe6862c8701b8869f9dcb4a.1790880255.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v4.git.git.1790880255.gitgitgadget@gmail.com","subject":"[PATCH v4 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-01T18:44:14Z","receivedAt":"2026-10-01T18:44:21Z","isPatch":true,"body":"From: Harald Nordgren \n\nA leak is only discovered once, at the end of a whole script, well\nafter every test has already reported ok, and it gets no annotation at\nall, so a leak-sanitizer job's only visible failure is:\n\n Process completed with exit code 1.\n\nGive a leak its own annotation. Point it at the test script, the exact\nline isn't known, only which script the leak turned up in, and put the\nsanitizer report in a log group next to it, so it stays visible.\n\nOnce a script has one leak, it keeps running: the sanitizer log\ndirectory is never cleared between tests, so every later test in the\nsame script sees the same leftover log entries and also reports \"not\nok\", burying the one real failure in copies of itself. Stop a\nleak-sanitizer script at its first failure with --immediate instead.\n\nA failing test already gets its own annotation once its script\nfinishes, but --immediate exits as soon as that test fails, before\nreaching the code that writes it. Write the annotation first, so\nturning on --immediate here does not silently drop it.\n\nSigned-off-by: Harald Nordgren \n---\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++\n t/test-lib.sh | 6 +++++-\n 3 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex c6ccbf8c17..a89f480a78 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)\n \texport NO_CVS_TESTS=LetsSaveSomeTime\n \texport NO_SVN_TESTS=LetsSaveSomeTime\n \texport NO_P4_TESTS=LetsSaveSomeTime\n+\tGIT_TEST_OPTS=\"$GIT_TEST_OPTS --immediate\"\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex fa29a62aa3..0d54496358 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -28,6 +28,11 @@ start_test_output () {\n \tgithub_markup_output=\"${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup\"\n \t>$github_markup_output\n \tGIT_TEST_TEE_OFFSET=0\n+\tgithub_markup_script_name=${0##*/}\n+}\n+\n+github_annotation_ () {\n+\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n \n # No need to override start_test_case_output\n@@ -53,4 +58,15 @@ finalize_test_case_output () {\n \techo >>$github_markup_output \"::endgroup::\"\n }\n \n+finalize_test_leak_output () {\n+\t# The exact line the leak turned up on isn't known, only the script,\n+\t# so point at line 1.\n+\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n+\t\t\"memory leak logged in $this_test\"\n+\n+\techo >>$github_markup_output \"::group::leak: $this_test.$test_count\"\n+\tcat \"$TEST_RESULTS_SAN_FILE\".* >>$github_markup_output\n+\techo >>$github_markup_output \"::endgroup::\"\n+}\n+\n # No need to override finalize_test_output\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1f0505e412..e74a12f1dd 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -199,6 +199,7 @@ mark_option_requires_arg () {\n start_test_output () { :; }\n start_test_case_output () { :; }\n finalize_test_case_output () { :; }\n+finalize_test_leak_output () { :; }\n finalize_test_output () { :; }\n \n parse_option () {\n@@ -822,6 +823,9 @@ test_failure_ () {\n \tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n+\t# Write the annotation before either --immediate exit path below,\n+\t# both of which call exit and would otherwise skip it.\n+\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n \tif test -n \"$immediate\"\n \tthen\n \t\tsay_color error \"1..$test_count\"\n@@ -835,7 +839,6 @@ test_failure_ () {\n \t\tcheck_test_results_san_file_ \"$test_failure\"\n \t\t_error_exit\n \tfi\n-\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n }\n \n test_known_broken_ok_ () {\n@@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {\n \t\treturn\n \tfi &&\n \tsay_color >&4 error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n+\tfinalize_test_leak_output &&\n \n \tif test \"$test_failure\" = 0\n \tthen\n-- \ngitgitgadget\n\n"},{"id":"553863","messageId":"8ec2b53d8265e1219b5f1279cadda2ac44c96ae0.1790880255.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v4.git.git.1790880255.gitgitgadget@gmail.com","subject":"[PATCH v4 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-01T18:44:15Z","receivedAt":"2026-10-01T18:44:24Z","isPatch":true,"body":"From: Harald Nordgren \n\nA test failure or a fixed known breakage gets an annotation that names\nthe test but says nothing about where it's defined, so a reviewer has\nto search the script by hand to find it.\n\nFind the line a test is defined on by searching the script for its\ndescription as a fixed string, using the first match. Fall back to\nline 1 when the description is not found verbatim, which happens when\na test builds its description at runtime instead of writing it out\nliterally.\n\nA GitHub annotation is a single line, so a `%` in a test description\nhas to be percent-encoded as `%25`, or GitHub misreads it as its own\nescape sequence. for-each-ref's format atoms use plenty of them, e.g.\n`%(raw)`.\n\nSigned-off-by: Harald Nordgren \n---\n t/test-lib-github-workflow-markup.sh | 38 +++++++++++++++++++++++-----\n 1 file changed, 32 insertions(+), 6 deletions(-)\n\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex 0d54496358..6fee4dfb22 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -31,6 +31,22 @@ start_test_output () {\n \tgithub_markup_script_name=${0##*/}\n }\n \n+github_escape_message_ () {\n+\t# % has to be escaped or GitHub misreads it as the start of its own\n+\t# percent-encoding (e.g. a literal %(raw) in a for-each-ref test\n+\t# description).\n+\tsed -e 's/%/%25/g'\n+}\n+\n+find_test_case_line_ () {\n+\t# A description can contain characters like [ or * that would\n+\t# corrupt a regex search, so match it literally and take the first\n+\t# hit. The -- keeps a description starting with \"-\" from being read\n+\t# as an option.\n+\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n+\thead -n 1 | cut -d: -f1\n+}\n+\n github_annotation_ () {\n \techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n@@ -40,18 +56,28 @@ github_annotation_ () {\n finalize_test_case_output () {\n \ttest_case_result=$1\n \tshift\n+\n+\tcase \"$test_case_result\" in\n+\tok|broken)\n+\t\t# Exit without printing the \"ok\" or \"broken\" tests\n+\t\treturn\n+\t\t;;\n+\tesac\n+\n+\ttest_case_line=$(find_test_case_line_ \"$1\")\n+\ttest_case_description=$(printf '%s' \"$1\" | github_escape_message_)\n+\n \tcase \"$test_case_result\" in\n \tfailure)\n-\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n+\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"failed: $this_test.$test_count $test_case_description\"\n \t\t;;\n \tfixed)\n-\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n-\t\t;;\n-\tok|broken)\n-\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n-\t\treturn\n+\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"fixed: $this_test.$test_count $test_case_description\"\n \t\t;;\n \tesac\n+\n \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n \ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n \t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n-- \ngitgitgadget\n"},{"id":"553865","messageId":"0e0972b7-65a2-46ce-84a9-7e403620802a@gmail.com","threadId":"66392","inReplyTo":"8ec2b53d8265e1219b5f1279cadda2ac44c96ae0.1790880255.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-10-01T19:49:08Z","receivedAt":"2026-10-01T19:49:18Z","isPatch":true,"body":"Hi Harald\n\nOn 01/10/2026 19:44, Harald Nordgren via GitGitGadget wrote:\n> From: Harald Nordgren \n> \n> A test failure or a fixed known breakage gets an annotation that names\n> the test but says nothing about where it's defined, so a reviewer has\n> to search the script by hand to find it.\n\nI'm afraid I'm still not clear what this does in practical terms. What \nappears in the test output that the user sees that didn't before?\n\n> Find the line a test is defined on by searching the script for its\n> description as a fixed string, using the first match. Fall back to\n> line 1 when the description is not found verbatim, which happens when\n> a test builds its description at runtime instead of writing it out\n> literally.\n> \n> A GitHub annotation is a single line, so a `%` in a test description\n> has to be percent-encoded as `%25`, or GitHub misreads it as its own\n> escape sequence. for-each-ref's format atoms use plenty of them, e.g.\n> `%(raw)`.\n\nThat's a useful example of why we want to escape the output which makes \nit all the more puzzling that we don't escape the existing annotations \nthat I mentioned last time.\n\nThanks\n\nPhillip\n\n> Signed-off-by: Harald Nordgren \n> ---\n> t/test-lib-github-workflow-markup.sh | 38 +++++++++++++++++++++++-----\n> 1 file changed, 32 insertions(+), 6 deletions(-)\n> \n> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\n> index 0d54496358..6fee4dfb22 100644\n> --- a/t/test-lib-github-workflow-markup.sh\n> +++ b/t/test-lib-github-workflow-markup.sh\n> @@ -31,6 +31,22 @@ start_test_output () {\n> \tgithub_markup_script_name=${0##*/}\n> }\n> \n> +github_escape_message_ () {\n> +\t# % has to be escaped or GitHub misreads it as the start of its own\n> +\t# percent-encoding (e.g. a literal %(raw) in a for-each-ref test\n> +\t# description).\n> +\tsed -e 's/%/%25/g'\n> +}\n> +\n> +find_test_case_line_ () {\n> +\t# A description can contain characters like [ or * that would\n> +\t# corrupt a regex search, so match it literally and take the first\n> +\t# hit. The -- keeps a description starting with \"-\" from being read\n> +\t# as an option.\n> +\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n> +\thead -n 1 | cut -d: -f1\n> +}\n> +\n> github_annotation_ () {\n> \techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n> }\n> @@ -40,18 +56,28 @@ github_annotation_ () {\n> finalize_test_case_output () {\n> \ttest_case_result=$1\n> \tshift\n> +\n> +\tcase \"$test_case_result\" in\n> +\tok|broken)\n> +\t\t# Exit without printing the \"ok\" or \"broken\" tests\n> +\t\treturn\n> +\t\t;;\n> +\tesac\n> +\n> +\ttest_case_line=$(find_test_case_line_ \"$1\")\n> +\ttest_case_description=$(printf '%s' \"$1\" | github_escape_message_)\n> +\n> \tcase \"$test_case_result\" in\n> \tfailure)\n> -\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n> +\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> +\t\t\t\"failed: $this_test.$test_count $test_case_description\"\n> \t\t;;\n> \tfixed)\n> -\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n> -\t\t;;\n> -\tok|broken)\n> -\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n> -\t\treturn\n> +\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> +\t\t\t\"fixed: $this_test.$test_count $test_case_description\"\n> \t\t;;\n> \tesac\n> +\n> \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n> \ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n> \t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n\n"},{"id":"553866","messageId":"xmqqld8h41vm.fsf@gitster.g","threadId":"66392","inReplyTo":"0e0972b7-65a2-46ce-84a9-7e403620802a@gmail.com","subject":"Re: [PATCH v4 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-01T20:11:41Z","receivedAt":"2026-10-01T20:11:44Z","isPatch":true,"body":"Phillip Wood writes:\n\n> Hi Harald\n>\n> On 01/10/2026 19:44, Harald Nordgren via GitGitGadget wrote:\n>> From: Harald Nordgren \n>> \n>> A test failure or a fixed known breakage gets an annotation that names\n>> the test but says nothing about where it's defined, so a reviewer has\n>> to search the script by hand to find it.\n>\n> I'm afraid I'm still not clear what this does in practical terms. What \n> appears in the test output that the user sees that didn't before?\n>\n>> Find the line a test is defined on by searching the script for its\n>> description as a fixed string, using the first match. Fall back to\n>> line 1 when the description is not found verbatim, which happens when\n>> a test builds its description at runtime instead of writing it out\n>> literally.\n\nI agree this is still hard to read. My interpretation of the above\nis\n\n We only say \"the t1234 script failed\" (in the first paragraph\n that makes an observation of the status quo), and we try to find\n the test_expect_success block and show it as the finer-grained\n clue (the second paragraph).\n\nbut that may be way off the mark.\n\n>> A GitHub annotation is a single line, so a `%` in a test description\n>> has to be percent-encoded as `%25`, or GitHub misreads it as its own\n>> escape sequence. for-each-ref's format atoms use plenty of them, e.g.\n>> `%(raw)`.\n>\n> That's a useful example of why we want to escape the output which makes \n> it all the more puzzling that we don't escape the existing annotations \n> that I mentioned last time.\n>\n> Thanks\n>\n> Phillip\n\nThanks.\n"},{"id":"553907","messageId":"CAHwyqnWyKRd_K0VfEMKRd79AJa2ygcq3wfWOPKS2+YqJ5UYqWw@mail.gmail.com","threadId":"66392","inReplyTo":"0e0972b7-65a2-46ce-84a9-7e403620802a@gmail.com","subject":"Re: [PATCH v4 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2026-10-02T08:04:07Z","receivedAt":"2026-10-02T08:04:47Z","isPatch":true,"body":"> > A GitHub annotation is a single line, so a `%` in a test description\n> > has to be percent-encoded as `%25`, or GitHub misreads it as its own\n> > escape sequence. for-each-ref's format atoms use plenty of them, e.g.\n> > `%(raw)`.\n>\n> That's a useful example of why we want to escape the output which makes\n> it all the more puzzling that we don't escape the existing annotations\n> that I mentioned last time.\n\nThis feels like a rabbit hole and probably better to just drop the\nescaping altogether. It seems that the only thing that would need\nescaping is the literal '%25', '%' in ASCII, but it doesn't even\nappear in any of our tests. See:\n\n- https://github.com/git/git/actions/runs/36976989079/job/110742888375?pr=2435\n- https://github.com/git/git/actions/runs/36977041388/job/110743045462?pr=2436\n\nI'll just drop this now. If needed, better to pick it up in a\ndifferent topic. Thanks for pursuing this!\n\n\nHarald\n"},{"id":"554047","messageId":"pull.2419.v5.git.git.1791015117.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.git.git.1790362443893.gitgitgadget@gmail.com","subject":"[PATCH v5 0/2] ci: link failure and leak annotations to the test script","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-03T08:11:55Z","receivedAt":"2026-10-03T08:12:00Z","isPatch":true,"body":"Link failure and leak annotations in CI to the test script, so both can be\nfound from the job summary.\n\nV5 CI Job where failures and leaks are reported:\nhttps://github.com/git/git/actions/runs/36979818141/job/110752027863?pr=2426\n\nChanges in v5:\n\n * Removed % escaping entirely, verified on CI that it isn't needed. Every\n existing test description that uses % renders correctly unescaped.\n * Rewrote the file/line commit message with a concrete example (failed:\n t1060.17 partial clone of corrupted repository).\n\nChanges in v4:\n\n * Clarify commit messages and simplify escaping logic.\n\nChanges in v3:\n\n * Fixed bug in the --immediate exit ordering: the --immediate &&\n --invert-exit-code path called exit 0 before the test's annotation was\n written, now a single unconditional call covers both exit paths.\n * github_escape_message_ no longer relies on \\r being a portable sed escape\n sequence (not POSIX-guaranteed and BSD sed implementations can differ),\n it splices in the literal carriage-return byte via printf instead.\n * Reverted unrelated test-tool line back to its original form.\n\nChanges in v2:\n\n * Split into two commits, each explaining its own reasoning.\n * Leak output is no longer capped or embedded in the message, it's now an\n uncapped fold, so multiple leaks in the same test both show in full. A\n second leak in a different test still won't show in the same run,\n --immediate stops the script at the first failure, but it no longer gets\n buried under every later test falsely reporting \"not ok\" either.\n * Drops the giant unfolded message that annotations used to carry, which is\n what probably caused the scrolling behavior.\n\nHarald Nordgren (2):\n ci: annotate leaks and stop a leak-sanitizer script at its first\n failure\n ci: point test failures and fixed known breakages at their file and\n line\n\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 46 ++++++++++++++++++++++++----\n t/test-lib.sh | 6 +++-\n 3 files changed, 46 insertions(+), 7 deletions(-)\n\n\nbase-commit: c46c1e37724f0478939de636ab8ea5a89086d532\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v5\nPull-Request: https://github.com/git/git/pull/2419\n\nRange-diff vs v4:\n\n 1: 138394b48b = 1: 851efeec8b ci: annotate leaks and stop a leak-sanitizer script at its first failure\n 2: 8ec2b53d82 ! 2: 46f93a9e16 ci: point test failures and fixed known breakages at their file and line\n @@ Metadata\n ## Commit message ##\n ci: point test failures and fixed known breakages at their file and line\n \n - A test failure or a fixed known breakage gets an annotation that names\n - the test but says nothing about where it's defined, so a reviewer has\n - to search the script by hand to find it.\n + When a test fails, GitHub shows an annotation naming it, for example:\n \n - Find the line a test is defined on by searching the script for its\n - description as a fixed string, using the first match. Fall back to\n + failed: t1060.17 partial clone of corrupted repository\n +\n + but the location GitHub attaches to that annotation is the CI\n + workflow file itself, not the test script, so there is nothing\n + pointing at where the test actually lives.\n +\n + Find the line a test is defined on by searching its script for the\n + test's own description as a fixed string, using the first match, and\n + attach that file and line to the annotation instead. Fall back to\n line 1 when the description is not found verbatim, which happens when\n a test builds its description at runtime instead of writing it out\n literally.\n \n - A GitHub annotation is a single line, so a `%` in a test description\n - has to be percent-encoded as `%25`, or GitHub misreads it as its own\n - escape sequence. for-each-ref's format atoms use plenty of them, e.g.\n - `%(raw)`.\n -\n Signed-off-by: Harald Nordgren \n \n ## t/test-lib-github-workflow-markup.sh ##\n @@ t/test-lib-github-workflow-markup.sh: start_test_output () {\n \tgithub_markup_script_name=${0##*/}\n }\n \n -+github_escape_message_ () {\n -+\t# % has to be escaped or GitHub misreads it as the start of its own\n -+\t# percent-encoding (e.g. a literal %(raw) in a for-each-ref test\n -+\t# description).\n -+\tsed -e 's/%/%25/g'\n -+}\n -+\n +find_test_case_line_ () {\n +\t# A description can contain characters like [ or * that would\n +\t# corrupt a regex search, so match it literally and take the first\n @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {\n +\tesac\n +\n +\ttest_case_line=$(find_test_case_line_ \"$1\")\n -+\ttest_case_description=$(printf '%s' \"$1\" | github_escape_message_)\n +\n \tcase \"$test_case_result\" in\n \tfailure)\n -\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n +\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n -+\t\t\t\"failed: $this_test.$test_count $test_case_description\"\n ++\t\t\t\"failed: $this_test.$test_count $1\"\n \t\t;;\n \tfixed)\n -\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {\n -\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n -\t\treturn\n +\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n -+\t\t\t\"fixed: $this_test.$test_count $test_case_description\"\n ++\t\t\t\"fixed: $this_test.$test_count $1\"\n \t\t;;\n \tesac\n +\n\n-- \ngitgitgadget\n"},{"id":"554048","messageId":"851efeec8b363807effe81779ff75552816fdb8f.1791015117.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v5.git.git.1791015117.gitgitgadget@gmail.com","subject":"[PATCH v5 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-03T08:11:56Z","receivedAt":"2026-10-03T08:12:02Z","isPatch":true,"body":"From: Harald Nordgren \n\nA leak is only discovered once, at the end of a whole script, well\nafter every test has already reported ok, and it gets no annotation at\nall, so a leak-sanitizer job's only visible failure is:\n\n Process completed with exit code 1.\n\nGive a leak its own annotation. Point it at the test script, the exact\nline isn't known, only which script the leak turned up in, and put the\nsanitizer report in a log group next to it, so it stays visible.\n\nOnce a script has one leak, it keeps running: the sanitizer log\ndirectory is never cleared between tests, so every later test in the\nsame script sees the same leftover log entries and also reports \"not\nok\", burying the one real failure in copies of itself. Stop a\nleak-sanitizer script at its first failure with --immediate instead.\n\nA failing test already gets its own annotation once its script\nfinishes, but --immediate exits as soon as that test fails, before\nreaching the code that writes it. Write the annotation first, so\nturning on --immediate here does not silently drop it.\n\nSigned-off-by: Harald Nordgren \n---\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++\n t/test-lib.sh | 6 +++++-\n 3 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex c6ccbf8c17..a89f480a78 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)\n \texport NO_CVS_TESTS=LetsSaveSomeTime\n \texport NO_SVN_TESTS=LetsSaveSomeTime\n \texport NO_P4_TESTS=LetsSaveSomeTime\n+\tGIT_TEST_OPTS=\"$GIT_TEST_OPTS --immediate\"\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex fa29a62aa3..0d54496358 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -28,6 +28,11 @@ start_test_output () {\n \tgithub_markup_output=\"${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup\"\n \t>$github_markup_output\n \tGIT_TEST_TEE_OFFSET=0\n+\tgithub_markup_script_name=${0##*/}\n+}\n+\n+github_annotation_ () {\n+\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n \n # No need to override start_test_case_output\n@@ -53,4 +58,15 @@ finalize_test_case_output () {\n \techo >>$github_markup_output \"::endgroup::\"\n }\n \n+finalize_test_leak_output () {\n+\t# The exact line the leak turned up on isn't known, only the script,\n+\t# so point at line 1.\n+\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n+\t\t\"memory leak logged in $this_test\"\n+\n+\techo >>$github_markup_output \"::group::leak: $this_test.$test_count\"\n+\tcat \"$TEST_RESULTS_SAN_FILE\".* >>$github_markup_output\n+\techo >>$github_markup_output \"::endgroup::\"\n+}\n+\n # No need to override finalize_test_output\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 321c2ba339..a893963f86 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -199,6 +199,7 @@ mark_option_requires_arg () {\n start_test_output () { :; }\n start_test_case_output () { :; }\n finalize_test_case_output () { :; }\n+finalize_test_leak_output () { :; }\n finalize_test_output () { :; }\n \n parse_option () {\n@@ -822,6 +823,9 @@ test_failure_ () {\n \tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n+\t# Write the annotation before either --immediate exit path below,\n+\t# both of which call exit and would otherwise skip it.\n+\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n \tif test -n \"$immediate\"\n \tthen\n \t\tsay_color error \"1..$test_count\"\n@@ -835,7 +839,6 @@ test_failure_ () {\n \t\tcheck_test_results_san_file_ \"$test_failure\"\n \t\t_error_exit\n \tfi\n-\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n }\n \n test_known_broken_ok_ () {\n@@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {\n \t\treturn\n \tfi &&\n \tsay_color >&4 error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n+\tfinalize_test_leak_output &&\n \n \tif test \"$test_failure\" = 0\n \tthen\n-- \ngitgitgadget\n\n"},{"id":"554049","messageId":"46f93a9e16e27966391a1cedb02a156b26fbd77f.1791015117.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v5.git.git.1791015117.gitgitgadget@gmail.com","subject":"[PATCH v5 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-03T08:11:57Z","receivedAt":"2026-10-03T08:12:04Z","isPatch":true,"body":"From: Harald Nordgren \n\nWhen a test fails, GitHub shows an annotation naming it, for example:\n\n failed: t1060.17 partial clone of corrupted repository\n\nbut the location GitHub attaches to that annotation is the CI\nworkflow file itself, not the test script, so there is nothing\npointing at where the test actually lives.\n\nFind the line a test is defined on by searching its script for the\ntest's own description as a fixed string, using the first match, and\nattach that file and line to the annotation instead. Fall back to\nline 1 when the description is not found verbatim, which happens when\na test builds its description at runtime instead of writing it out\nliterally.\n\nSigned-off-by: Harald Nordgren \n---\n t/test-lib-github-workflow-markup.sh | 30 ++++++++++++++++++++++------\n 1 file changed, 24 insertions(+), 6 deletions(-)\n\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex 0d54496358..ac8c536231 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -31,6 +31,15 @@ start_test_output () {\n \tgithub_markup_script_name=${0##*/}\n }\n \n+find_test_case_line_ () {\n+\t# A description can contain characters like [ or * that would\n+\t# corrupt a regex search, so match it literally and take the first\n+\t# hit. The -- keeps a description starting with \"-\" from being read\n+\t# as an option.\n+\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n+\thead -n 1 | cut -d: -f1\n+}\n+\n github_annotation_ () {\n \techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n@@ -40,18 +49,27 @@ github_annotation_ () {\n finalize_test_case_output () {\n \ttest_case_result=$1\n \tshift\n+\n+\tcase \"$test_case_result\" in\n+\tok|broken)\n+\t\t# Exit without printing the \"ok\" or \"broken\" tests\n+\t\treturn\n+\t\t;;\n+\tesac\n+\n+\ttest_case_line=$(find_test_case_line_ \"$1\")\n+\n \tcase \"$test_case_result\" in\n \tfailure)\n-\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n+\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"failed: $this_test.$test_count $1\"\n \t\t;;\n \tfixed)\n-\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n-\t\t;;\n-\tok|broken)\n-\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n-\t\treturn\n+\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n+\t\t\t\"fixed: $this_test.$test_count $1\"\n \t\t;;\n \tesac\n+\n \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n \ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n \t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n-- \ngitgitgadget\n"},{"id":"554058","messageId":"3587bf44-1b3b-422f-a926-f8481104dfd8@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v5.git.git.1791015117.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-10-03T13:03:37Z","receivedAt":"2026-10-03T13:03:40Z","isPatch":true,"body":"Hi Harald\n\nOn 03/10/2026 09:11, Harald Nordgren via GitGitGadget wrote:\n> Link failure and leak annotations in CI to the test script, so both can be\n> found from the job summary.\n> \n> V5 CI Job where failures and leaks are reported:\n> https://github.com/git/git/actions/runs/36979818141/job/110752027863?pr=2426\n\nThere does not appear to be any output relating to leaks in that job. \nThe first patch hasn't changed so I'm not sure why that is.\n\n> Changes in v5:\n> \n> * Removed % escaping entirely, verified on CI that it isn't needed. Every\n> existing test description that uses % renders correctly unescaped.\n> * Rewrote the file/line commit message with a concrete example (failed:\n> t1060.17 partial clone of corrupted repository).\n\nYou have added\n\n\n When a test fails, GitHub shows an annotation naming it, for\n example:\n\n failed: t1060.17 partial clone of corrupted repository\n\nwhich shows an example of the current output without the filename or \nline annotations. There is no example of what that output changes to, so \nthere is no way for someone reading that message to see what has \nactually changed. After spending some time clicking around in Github I \nthink what that patch changes is not the test output of individual jobs \nwhich you linked to above, but what is displayed on the summary page at\n\nhttps://github.com/git/git/actions/runs/36979818141?pr=2426\n\nThat page shows a list of annotations with links to the changes in the \nfailed test file. That is a useful improvement but how you expected \nsomeone reading the commit message to understand what had changed when \nyou did not give an example of the new output, and the changes are on a \ndifferent page to the one you linked to in the cover letter is beyond \nme. I'm pretty exasperated that I've had to spend time messing about on \nGithub trying to see what has changed because you could not provide a \nlink and write a couple of sentences explaining it. After asking what \nthis change did in v3 you replied that the commit message wasn't clear \nwithout explaining what the change actually did. When I asked what the \nchange did in practical terms in response to v4 I got no reply. As you \nalready know reviewer time is short on this list, so please, when \nsomeone asks a question answer it rather than replying with an obtuse \ncomment or simply ignoring it and sending another patch.\n\nBoth these patches are useful improvements, but trying to get an \nexplanation of what they did has been like trying getting blood out of a \nstone.\n\nThanks\n\nPhillip\n\n\n> Changes in v4:\n> \n> * Clarify commit messages and simplify escaping logic.\n> \n> Changes in v3:\n> \n> * Fixed bug in the --immediate exit ordering: the --immediate &&\n> --invert-exit-code path called exit 0 before the test's annotation was\n> written, now a single unconditional call covers both exit paths.\n> * github_escape_message_ no longer relies on \\r being a portable sed escape\n> sequence (not POSIX-guaranteed and BSD sed implementations can differ),\n> it splices in the literal carriage-return byte via printf instead.\n> * Reverted unrelated test-tool line back to its original form.\n> \n> Changes in v2:\n> \n> * Split into two commits, each explaining its own reasoning.\n> * Leak output is no longer capped or embedded in the message, it's now an\n> uncapped fold, so multiple leaks in the same test both show in full. A\n> second leak in a different test still won't show in the same run,\n> --immediate stops the script at the first failure, but it no longer gets\n> buried under every later test falsely reporting \"not ok\" either.\n> * Drops the giant unfolded message that annotations used to carry, which is\n> what probably caused the scrolling behavior.\n> \n> Harald Nordgren (2):\n> ci: annotate leaks and stop a leak-sanitizer script at its first\n> failure\n> ci: point test failures and fixed known breakages at their file and\n> line\n> \n> ci/lib.sh | 1 +\n> t/test-lib-github-workflow-markup.sh | 46 ++++++++++++++++++++++++----\n> t/test-lib.sh | 6 +++-\n> 3 files changed, 46 insertions(+), 7 deletions(-)\n> \n> \n> base-commit: c46c1e37724f0478939de636ab8ea5a89086d532\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v5\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v5\n> Pull-Request: https://github.com/git/git/pull/2419\n> \n> Range-diff vs v4:\n> \n> 1: 138394b48b = 1: 851efeec8b ci: annotate leaks and stop a leak-sanitizer script at its first failure\n> 2: 8ec2b53d82 ! 2: 46f93a9e16 ci: point test failures and fixed known breakages at their file and line\n> @@ Metadata\n> ## Commit message ##\n> ci: point test failures and fixed known breakages at their file and line\n> \n> - A test failure or a fixed known breakage gets an annotation that names\n> - the test but says nothing about where it's defined, so a reviewer has\n> - to search the script by hand to find it.\n> + When a test fails, GitHub shows an annotation naming it, for example:\n> \n> - Find the line a test is defined on by searching the script for its\n> - description as a fixed string, using the first match. Fall back to\n> + failed: t1060.17 partial clone of corrupted repository\n> +\n> + but the location GitHub attaches to that annotation is the CI\n> + workflow file itself, not the test script, so there is nothing\n> + pointing at where the test actually lives.\n> +\n> + Find the line a test is defined on by searching its script for the\n> + test's own description as a fixed string, using the first match, and\n> + attach that file and line to the annotation instead. Fall back to\n> line 1 when the description is not found verbatim, which happens when\n> a test builds its description at runtime instead of writing it out\n> literally.\n> \n> - A GitHub annotation is a single line, so a `%` in a test description\n> - has to be percent-encoded as `%25`, or GitHub misreads it as its own\n> - escape sequence. for-each-ref's format atoms use plenty of them, e.g.\n> - `%(raw)`.\n> -\n> Signed-off-by: Harald Nordgren \n> \n> ## t/test-lib-github-workflow-markup.sh ##\n> @@ t/test-lib-github-workflow-markup.sh: start_test_output () {\n> \tgithub_markup_script_name=${0##*/}\n> }\n> \n> -+github_escape_message_ () {\n> -+\t# % has to be escaped or GitHub misreads it as the start of its own\n> -+\t# percent-encoding (e.g. a literal %(raw) in a for-each-ref test\n> -+\t# description).\n> -+\tsed -e 's/%/%25/g'\n> -+}\n> -+\n> +find_test_case_line_ () {\n> +\t# A description can contain characters like [ or * that would\n> +\t# corrupt a regex search, so match it literally and take the first\n> @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {\n> +\tesac\n> +\n> +\ttest_case_line=$(find_test_case_line_ \"$1\")\n> -+\ttest_case_description=$(printf '%s' \"$1\" | github_escape_message_)\n> +\n> \tcase \"$test_case_result\" in\n> \tfailure)\n> -\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n> +\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> -+\t\t\t\"failed: $this_test.$test_count $test_case_description\"\n> ++\t\t\t\"failed: $this_test.$test_count $1\"\n> \t\t;;\n> \tfixed)\n> -\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n> @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {\n> -\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n> -\t\treturn\n> +\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n> -+\t\t\t\"fixed: $this_test.$test_count $test_case_description\"\n> ++\t\t\t\"fixed: $this_test.$test_count $1\"\n> \t\t;;\n> \tesac\n> +\n> \n\n"},{"id":"554075","messageId":"8b873f2e-b395-4044-ab15-f1eab4148447@gmail.com","threadId":"66392","inReplyTo":"3587bf44-1b3b-422f-a926-f8481104dfd8@gmail.com","subject":"Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-10-03T19:02:09Z","receivedAt":"2026-10-03T19:02:13Z","isPatch":true,"body":"On 03/10/2026 14:03, Phillip Wood wrote:\n> After spending some time clicking around in Github I \n> think what that patch changes is not the test output of individual jobs \n> which you linked to above, but what is displayed on the summary page at\n> \n> https://github.com/git/git/actions/runs/36979818141?pr=2426\n> \n> That page shows a list of annotations with links to the changes in the \n> failed test file. That is a useful improvement\n\nBut it seems it is only useful if the test changed is in that example. \nIf I look at the summary for the CI run from v3 of this series [1] then \nI can see a test failure in t1022\n\n linux-leaks(ubuntu-rolling): t/t1022-read-tree-partial-clone.sh#L8\n failed: t1022.1 read-tree in partial clone prefetches in one batch\n\nIf I click on the link [2] it does not take me to that test file though, \nbecause it was not changed. That makes this somewhat less useful than I \ninitially thought. The current behavior is that when you click on those \nlinks in the summary page it takes you to the test output for the job \nthat failed which seems more useful. For example [3] is recent test run \nthat had a leak and clicking on\n\n linux-leaks:(ubuntu-rolling):\n failed: t1092.58 submodule handling\n\nTakes me to [4] which where I can click to expand the output of the \nfailing test.\n\nThanks\n\nPhillip\n\n[1] https://github.com/git/git/actions/runs/36537917146?pr=2426\n[2] https://github.com/git/git/pull/2426/files#annotation_82189987516\n[3] https://github.com/benknoble/git/actions/runs/36033463504\n[4] \nhttps://github.com/benknoble/git/actions/runs/36033463504/job/107747745741#step:9:5333\n"},{"id":"554108","messageId":"CAHwyqnXBLiAA+aX8uLA3UvsfD4zaMTcvj96H8DX8BhMVopwcfQ@mail.gmail.com","threadId":"66392","inReplyTo":"8b873f2e-b395-4044-ab15-f1eab4148447@gmail.com","subject":"Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2026-10-04T11:51:56Z","receivedAt":"2026-10-04T11:52:35Z","isPatch":true,"body":"On Sat, Oct 3, 2026 at 9:02 PM Phillip Wood wrote:\n>\n> On 03/10/2026 14:03, Phillip Wood wrote:\n> > After spending some time clicking around in Github I\n> > think what that patch changes is not the test output of individual jobs\n> > which you linked to above, but what is displayed on the summary page at\n> >\n> > https://github.com/git/git/actions/runs/36979818141?pr=2426\n> >\n> > That page shows a list of annotations with links to the changes in the\n> > failed test file. That is a useful improvement\n>\n> But it seems it is only useful if the test changed is in that example.\n> If I look at the summary for the CI run from v3 of this series [1] then\n> I can see a test failure in t1022\n>\n> linux-leaks(ubuntu-rolling): t/t1022-read-tree-partial-clone.sh#L8\n> failed: t1022.1 read-tree in partial clone prefetches in one batch\n>\n> If I click on the link [2] it does not take me to that test file though,\n> because it was not changed.\n\nYes, unfortunately GitHub won't let us link to a line that was not\nchanged in that PR.\n\n> That makes this somewhat less useful than I\n> initially thought. The current behavior is that when you click on those\n> links in the summary page it takes you to the test output for the job\n> that failed which seems more useful. For example [3] is recent test run\n> that had a leak and clicking on\n>\n> linux-leaks:(ubuntu-rolling):\n> failed: t1092.58 submodule handling\n>\n> Takes me to [4] which where I can click to expand the output of the\n> failing test.\n> ...\n> [1] https://github.com/git/git/actions/runs/36537917146?pr=2426\n> [2] https://github.com/git/git/pull/2426/files#annotation_82189987516\n> [3] https://github.com/benknoble/git/actions/runs/36033463504\n> [4]\n> https://github.com/benknoble/git/actions/runs/36033463504/job/107747745741#step:9:5333\n\nIs this enough to call this a regression? Then maybe it's not worth\ndoing this part at all.\n\n\nHarald\n"},{"id":"554109","messageId":"CAHwyqnWU0z7wHxDnxdoviOBenYJogB7k4GLo2jdO7_M_hSMZWQ@mail.gmail.com","threadId":"66392","inReplyTo":"3587bf44-1b3b-422f-a926-f8481104dfd8@gmail.com","subject":"Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2026-10-04T12:03:31Z","receivedAt":"2026-10-04T12:04:11Z","isPatch":true,"body":"On Sat, Oct 3, 2026 at 3:03 PM Phillip Wood wrote:\n>\n> Hi Harald\n>\n> On 03/10/2026 09:11, Harald Nordgren via GitGitGadget wrote:\n> > Link failure and leak annotations in CI to the test script, so both can be\n> > found from the job summary.\n> >\n> > V5 CI Job where failures and leaks are reported:\n> > https://github.com/git/git/actions/runs/36979818141/job/110752027863?pr=2426\n>\n> There does not appear to be any output relating to leaks in that job.\n> The first patch hasn't changed so I'm not sure why that is.\n>\n> > Changes in v5:\n> >\n> > * Removed % escaping entirely, verified on CI that it isn't needed. Every\n> > existing test description that uses % renders correctly unescaped.\n> > * Rewrote the file/line commit message with a concrete example (failed:\n> > t1060.17 partial clone of corrupted repository).\n>\n> You have added\n>\n>\n> When a test fails, GitHub shows an annotation naming it, for\n> example:\n>\n> failed: t1060.17 partial clone of corrupted repository\n>\n> which shows an example of the current output without the filename or\n> line annotations. There is no example of what that output changes to, so\n> there is no way for someone reading that message to see what has\n> actually changed. After spending some time clicking around in Github I\n> think what that patch changes is not the test output of individual jobs\n> which you linked to above, but what is displayed on the summary page at\n>\n> https://github.com/git/git/actions/runs/36979818141?pr=2426\n>\n> That page shows a list of annotations with links to the changes in the\n> failed test file. That is a useful improvement but how you expected\n> someone reading the commit message to understand what had changed when\n> you did not give an example of the new output, and the changes are on a\n> different page to the one you linked to in the cover letter is beyond\n> me. I'm pretty exasperated that I've had to spend time messing about on\n> Github trying to see what has changed because you could not provide a\n> link and write a couple of sentences explaining it. After asking what\n> this change did in v3 you replied that the commit message wasn't clear\n> without explaining what the change actually did. When I asked what the\n> change did in practical terms in response to v4 I got no reply. As you\n> already know reviewer time is short on this list, so please, when\n> someone asks a question answer it rather than replying with an obtuse\n> comment or simply ignoring it and sending another patch.\n>\n> Both these patches are useful improvements, but trying to get an\n> explanation of what they did has been like trying getting blood out of a\n> stone.\n\nI don't understand why this tone is necessary at all.\n\nYes, I should clarify that it affects the summary.\n\nBut I posted a comment regarding the regression you brought up in your\nfollowing message, so we should decide what we want before\nprogressing. The point of this whole topic was to make the leak\nreporting less bad, everything else was a bonus.\n\n\nHarald\n"},{"id":"554172","messageId":"ea988ec0-ef3d-4250-a0d6-ffdf3794b9cf@gmail.com","threadId":"66392","inReplyTo":"CAHwyqnXBLiAA+aX8uLA3UvsfD4zaMTcvj96H8DX8BhMVopwcfQ@mail.gmail.com","subject":"Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-10-05T13:23:21Z","receivedAt":"2026-10-05T13:23:41Z","isPatch":true,"body":"Hi Harald\n\nOn 04/10/2026 12:51, Harald Nordgren wrote:\n> On Sat, Oct 3, 2026 at 9:02 PM Phillip Wood wrote:\n>>\n>> If I click on the link [2] it does not take me to that test file though,\n>> because it was not changed.\n> \n> Yes, unfortunately GitHub won't let us link to a line that was not\n> changed in that PR.\n> \n>> That makes this somewhat less useful than I\n>> initially thought. The current behavior is that when you click on those\n>> links in the summary page it takes you to the test output for the job\n>> that failed which seems more useful. For example [3] is recent test run\n>> that had a leak and clicking on\n>>\n>> linux-leaks:(ubuntu-rolling):\n>> failed: t1092.58 submodule handling\n>>\n>> Takes me to [4] which where I can click to expand the output of the\n>> failing test.\n>> ...\n>> [1] https://github.com/git/git/actions/runs/36537917146?pr=2426\n>> [2] https://github.com/git/git/pull/2426/files#annotation_82189987516\n>> [3] https://github.com/benknoble/git/actions/runs/36033463504\n>> [4]\n>> https://github.com/benknoble/git/actions/runs/36033463504/job/107747745741#step:9:5333\n> \n> Is this enough to call this a regression? Then maybe it's not worth\n> doing this part at all.\n\nYes, I think we should drop this patch. The first step to debugging a \ntest failure is to look at the test output, so the current behavior \nwhere clicking on the links on the summary page takes you to the test \noutput is more useful than taking you to a diff that may not even show \nthe test that failed. The first patch is definitely worth keeping as it \nmakes it much easier to see the LSAN output.\n\nThanks\n\nPhillip\n\n"},{"id":"554179","messageId":"CAHwyqnUO0zvr+hPT2t0CG-7D9vZuWBRdj1RjC356WEuXaZ9Faw@mail.gmail.com","threadId":"66392","inReplyTo":"ea988ec0-ef3d-4250-a0d6-ffdf3794b9cf@gmail.com","subject":"Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2026-10-05T13:59:48Z","receivedAt":"2026-10-05T13:59:48Z","isPatch":true,"body":"> > Is this enough to call this a regression? Then maybe it's not worth\n> > doing this part at all.\n>\n> Yes, I think we should drop this patch. The first step to debugging a\n> test failure is to look at the test output, so the current behavior\n> where clicking on the links on the summary page takes you to the test\n> output is more useful than taking you to a diff that may not even show\n> the test that failed. The first patch is definitely worth keeping as it\n> makes it much easier to see the LSAN output.\n\nI played with instead showing the file name (and line when available)\nas part of the annotation text, and leaving the linking as it is. I\nthink it could gives us the best of both worlds:\n\n memory leak logged in t1060 (t1060-object-corruption.sh)\n\nand\n\n failed: t1060.17 partial clone of corrupted repository\n(t1060-object-corruption.sh:141)\n\nWhat do you think?\n\n\nHarald\n\n"},{"id":"554185","messageId":"c991fe7d-6230-4600-a038-b051993e0e2d@gmail.com","threadId":"66392","inReplyTo":"CAHwyqnUO0zvr+hPT2t0CG-7D9vZuWBRdj1RjC356WEuXaZ9Faw@mail.gmail.com","subject":"Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-10-05T15:11:14Z","receivedAt":"2026-10-05T15:11:14Z","isPatch":true,"body":"On 05/10/2026 14:59, Harald Nordgren wrote:\n>>> Is this enough to call this a regression? Then maybe it's not worth\n>>> doing this part at all.\n>>\n>> Yes, I think we should drop this patch. The first step to debugging a\n>> test failure is to look at the test output, so the current behavior\n>> where clicking on the links on the summary page takes you to the test\n>> output is more useful than taking you to a diff that may not even show\n>> the test that failed. The first patch is definitely worth keeping as it\n>> makes it much easier to see the LSAN output.\n> > I played with instead showing the file name (and line when available)\n> as part of the annotation text, and leaving the linking as it is. I\n> think it could gives us the best of both worlds:\n> > memory leak logged in t1060 (t1060-object-corruption.sh)\n> > and\n> > failed: t1060.17 partial clone of corrupted repository\n> (t1060-object-corruption.sh:141)\n> > What do you think?\n\nI guess having a bit more detail could be useful, I certainly don't object.\n\nThanks\n\nPhillip\n\n\n"},{"id":"554238","messageId":"pull.2419.v6.git.git.1791269798.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.git.git.1790362443893.gitgitgadget@gmail.com","subject":"[PATCH v6 0/2] ci: link failure and leak annotations to the test script","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-06T06:56:36Z","receivedAt":"2026-10-06T06:56:36Z","isPatch":true,"body":"Link failure and leak annotations in CI to the test script, so both can be\nfound from the job summary.\n\nV7 CI job where failures and leaks are reported:\n\n * https://github.com/git/git/actions/runs/37205086805?pr=2426\n\nChanges in v7:\n\n * Revert the linking in annotation message, direct direct line fails when\n error pointed to an unchanged file in that PR, which regresses the\n experiences in many cases. Instead now show the file and line as plain\n text in the annotation.\n * Don't name the the line number as 1 for leaks, where it's never\n available, just omit the line number.\n\nChanges in v6:\n\n * Update commit message.\n\nChanges in v5:\n\n * Removed % escaping entirely, verified on CI that it isn't needed. Every\n existing test description that uses % renders correctly unescaped.\n * Rewrote the file/line commit message with a concrete example (failed:\n t1060.17 partial clone of corrupted repository).\n\nChanges in v4:\n\n * Clarify commit messages and simplify escaping logic.\n\nChanges in v3:\n\n * Fixed bug in the --immediate exit ordering: the --immediate &&\n --invert-exit-code path called exit 0 before the test's annotation was\n written, now a single unconditional call covers both exit paths.\n * github_escape_message_ no longer relies on \\r being a portable sed escape\n sequence (not POSIX-guaranteed and BSD sed implementations can differ),\n it splices in the literal carriage-return byte via printf instead.\n * Reverted unrelated test-tool line back to its original form.\n\nChanges in v2:\n\n * Split into two commits, each explaining its own reasoning.\n * Leak output is no longer capped or embedded in the message, it's now an\n uncapped fold, so multiple leaks in the same test both show in full. A\n second leak in a different test still won't show in the same run,\n --immediate stops the script at the first failure, but it no longer gets\n buried under every later test falsely reporting \"not ok\" either.\n * Drops the giant unfolded message that annotations used to carry, which is\n what probably caused the scrolling behavior.\n\nHarald Nordgren (2):\n ci: annotate leaks and stop a leak-sanitizer script at its first\n failure\n ci: point test failures and fixed known breakages at their file and\n line\n\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 39 +++++++++++++++++++++++-----\n t/test-lib.sh | 6 ++++-\n 3 files changed, 39 insertions(+), 7 deletions(-)\n\n\nbase-commit: 8103b446517e0c44e67561b9d0ccce56efa60a71\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v6\nPull-Request: https://github.com/git/git/pull/2419\n\nRange-diff vs v5:\n\n 1: 851efeec8b ! 1: 917f373f91 ci: annotate leaks and stop a leak-sanitizer script at its first failure\n @@ Commit message\n \n Process completed with exit code 1.\n \n - Give a leak its own annotation. Point it at the test script, the exact\n - line isn't known, only which script the leak turned up in, and put the\n - sanitizer report in a log group next to it, so it stays visible.\n + Give a leak its own annotation, naming the script it turned up in, the\n + exact line isn't known, only which script:\n +\n + memory leak logged in t1060 (t1060-object-corruption.sh)\n +\n + Put the sanitizer report in a log group next to it, so it stays\n + visible.\n \n Once a script has one leak, it keeps running: the sanitizer log\n directory is never cleared between tests, so every later test in the\n @@ t/test-lib-github-workflow-markup.sh: start_test_output () {\n \t>$github_markup_output\n \tGIT_TEST_TEE_OFFSET=0\n +\tgithub_markup_script_name=${0##*/}\n -+}\n -+\n -+github_annotation_ () {\n -+\techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n }\n \n # No need to override start_test_case_output\n @@ t/test-lib-github-workflow-markup.sh: finalize_test_case_output () {\n }\n \n +finalize_test_leak_output () {\n -+\t# The exact line the leak turned up on isn't known, only the script,\n -+\t# so point at line 1.\n -+\tgithub_annotation_ error \"t/$github_markup_script_name\" 1 \\\n -+\t\t\"memory leak logged in $this_test\"\n ++\techo >>$github_markup_output \\\n ++\t\t\"::error::memory leak logged in $this_test ($github_markup_script_name)\"\n +\n +\techo >>$github_markup_output \"::group::leak: $this_test.$test_count\"\n +\tcat \"$TEST_RESULTS_SAN_FILE\".* >>$github_markup_output\n 2: 46f93a9e16 ! 2: acf1fbd250 ci: point test failures and fixed known breakages at their file and line\n @@ Metadata\n ## Commit message ##\n ci: point test failures and fixed known breakages at their file and line\n \n - When a test fails, GitHub shows an annotation naming it, for example:\n + A failing test gets an annotation in the Annotations list on its job's\n + summary page, naming it, for example:\n \n failed: t1060.17 partial clone of corrupted repository\n \n - but the location GitHub attaches to that annotation is the CI\n - workflow file itself, not the test script, so there is nothing\n - pointing at where the test actually lives.\n + with no indication of where that test lives.\n \n Find the line a test is defined on by searching its script for the\n test's own description as a fixed string, using the first match, and\n - attach that file and line to the annotation instead. Fall back to\n - line 1 when the description is not found verbatim, which happens when\n - a test builds its description at runtime instead of writing it out\n - literally.\n + add the file and line to the annotation's own message text:\n +\n + failed: t1060.17 partial clone of corrupted repository (t1060-object-corruption.sh:141)\n +\n + Fall back to naming just the script, with no line, when the\n + description is not found verbatim, which happens when a test builds\n + its description at runtime instead of writing it out literally.\n \n Signed-off-by: Harald Nordgren \n \n @@ t/test-lib-github-workflow-markup.sh: start_test_output () {\n +\thead -n 1 | cut -d: -f1\n +}\n +\n - github_annotation_ () {\n - \techo >>$github_markup_output \"::$1 file=$2,line=$3::$4\"\n - }\n -@@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {\n + # No need to override start_test_case_output\n + \n finalize_test_case_output () {\n \ttest_case_result=$1\n \tshift\n @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {\n +\tesac\n +\n +\ttest_case_line=$(find_test_case_line_ \"$1\")\n ++\ttest_case_where=\"$github_markup_script_name${test_case_line:+:$test_case_line}\"\n +\n \tcase \"$test_case_result\" in\n \tfailure)\n -\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n -+\t\tgithub_annotation_ error \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n -+\t\t\t\"failed: $this_test.$test_count $1\"\n ++\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1 ($test_case_where)\"\n \t\t;;\n \tfixed)\n -\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {\n -\tok|broken)\n -\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n -\t\treturn\n -+\t\tgithub_annotation_ notice \"t/$github_markup_script_name\" \"${test_case_line:-1}\" \\\n -+\t\t\t\"fixed: $this_test.$test_count $1\"\n ++\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1 ($test_case_where)\"\n \t\t;;\n \tesac\n +\n\n-- \ngitgitgadget\n\n"},{"id":"554239","messageId":"917f373f916945b7a01b151756eba90f55a347fe.1791269798.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v6.git.git.1791269798.gitgitgadget@gmail.com","subject":"[PATCH v6 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-06T06:56:37Z","receivedAt":"2026-10-06T06:56:37Z","isPatch":true,"body":"From: Harald Nordgren \n\nA leak is only discovered once, at the end of a whole script, well\nafter every test has already reported ok, and it gets no annotation at\nall, so a leak-sanitizer job's only visible failure is:\n\n Process completed with exit code 1.\n\nGive a leak its own annotation, naming the script it turned up in, the\nexact line isn't known, only which script:\n\n memory leak logged in t1060 (t1060-object-corruption.sh)\n\nPut the sanitizer report in a log group next to it, so it stays\nvisible.\n\nOnce a script has one leak, it keeps running: the sanitizer log\ndirectory is never cleared between tests, so every later test in the\nsame script sees the same leftover log entries and also reports \"not\nok\", burying the one real failure in copies of itself. Stop a\nleak-sanitizer script at its first failure with --immediate instead.\n\nA failing test already gets its own annotation once its script\nfinishes, but --immediate exits as soon as that test fails, before\nreaching the code that writes it. Write the annotation first, so\nturning on --immediate here does not silently drop it.\n\nSigned-off-by: Harald Nordgren \n---\n ci/lib.sh | 1 +\n t/test-lib-github-workflow-markup.sh | 10 ++++++++++\n t/test-lib.sh | 6 +++++-\n 3 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex c6ccbf8c17..a89f480a78 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)\n \texport NO_CVS_TESTS=LetsSaveSomeTime\n \texport NO_SVN_TESTS=LetsSaveSomeTime\n \texport NO_P4_TESTS=LetsSaveSomeTime\n+\tGIT_TEST_OPTS=\"$GIT_TEST_OPTS --immediate\"\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex fa29a62aa3..3fa7859f0b 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -28,6 +28,7 @@ start_test_output () {\n \tgithub_markup_output=\"${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup\"\n \t>$github_markup_output\n \tGIT_TEST_TEE_OFFSET=0\n+\tgithub_markup_script_name=${0##*/}\n }\n \n # No need to override start_test_case_output\n@@ -53,4 +54,13 @@ finalize_test_case_output () {\n \techo >>$github_markup_output \"::endgroup::\"\n }\n \n+finalize_test_leak_output () {\n+\techo >>$github_markup_output \\\n+\t\t\"::error::memory leak logged in $this_test ($github_markup_script_name)\"\n+\n+\techo >>$github_markup_output \"::group::leak: $this_test.$test_count\"\n+\tcat \"$TEST_RESULTS_SAN_FILE\".* >>$github_markup_output\n+\techo >>$github_markup_output \"::endgroup::\"\n+}\n+\n # No need to override finalize_test_output\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 321c2ba339..a893963f86 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -199,6 +199,7 @@ mark_option_requires_arg () {\n start_test_output () { :; }\n start_test_case_output () { :; }\n finalize_test_case_output () { :; }\n+finalize_test_leak_output () { :; }\n finalize_test_output () { :; }\n \n parse_option () {\n@@ -822,6 +823,9 @@ test_failure_ () {\n \tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n+\t# Write the annotation before either --immediate exit path below,\n+\t# both of which call exit and would otherwise skip it.\n+\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n \tif test -n \"$immediate\"\n \tthen\n \t\tsay_color error \"1..$test_count\"\n@@ -835,7 +839,6 @@ test_failure_ () {\n \t\tcheck_test_results_san_file_ \"$test_failure\"\n \t\t_error_exit\n \tfi\n-\tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n }\n \n test_known_broken_ok_ () {\n@@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {\n \t\treturn\n \tfi &&\n \tsay_color >&4 error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n+\tfinalize_test_leak_output &&\n \n \tif test \"$test_failure\" = 0\n \tthen\n-- \ngitgitgadget\n\n\n"},{"id":"554240","messageId":"acf1fbd250c347a0f1afc295e603f3463c14217f.1791269798.git.gitgitgadget@gmail.com","threadId":"66392","inReplyTo":"pull.2419.v6.git.git.1791269798.gitgitgadget@gmail.com","subject":"[PATCH v6 2/2] ci: point test failures and fixed known breakages at their file and line","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-10-06T06:56:38Z","receivedAt":"2026-10-06T06:56:38Z","isPatch":true,"body":"From: Harald Nordgren \n\nA failing test gets an annotation in the Annotations list on its job's\nsummary page, naming it, for example:\n\n failed: t1060.17 partial clone of corrupted repository\n\nwith no indication of where that test lives.\n\nFind the line a test is defined on by searching its script for the\ntest's own description as a fixed string, using the first match, and\nadd the file and line to the annotation's own message text:\n\n failed: t1060.17 partial clone of corrupted repository (t1060-object-corruption.sh:141)\n\nFall back to naming just the script, with no line, when the\ndescription is not found verbatim, which happens when a test builds\nits description at runtime instead of writing it out literally.\n\nSigned-off-by: Harald Nordgren \n---\n t/test-lib-github-workflow-markup.sh | 29 ++++++++++++++++++++++------\n 1 file changed, 23 insertions(+), 6 deletions(-)\n\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex 3fa7859f0b..826c4ac902 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -31,23 +31,40 @@ start_test_output () {\n \tgithub_markup_script_name=${0##*/}\n }\n \n+find_test_case_line_ () {\n+\t# A description can contain characters like [ or * that would\n+\t# corrupt a regex search, so match it literally and take the first\n+\t# hit. The -- keeps a description starting with \"-\" from being read\n+\t# as an option.\n+\tgrep -n -F -- \"$1\" \"$TEST_DIRECTORY/$github_markup_script_name\" |\n+\thead -n 1 | cut -d: -f1\n+}\n+\n # No need to override start_test_case_output\n \n finalize_test_case_output () {\n \ttest_case_result=$1\n \tshift\n+\n+\tcase \"$test_case_result\" in\n+\tok|broken)\n+\t\t# Exit without printing the \"ok\" or \"broken\" tests\n+\t\treturn\n+\t\t;;\n+\tesac\n+\n+\ttest_case_line=$(find_test_case_line_ \"$1\")\n+\ttest_case_where=\"$github_markup_script_name${test_case_line:+:$test_case_line}\"\n+\n \tcase \"$test_case_result\" in\n \tfailure)\n-\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1\"\n+\t\techo >>$github_markup_output \"::error::failed: $this_test.$test_count $1 ($test_case_where)\"\n \t\t;;\n \tfixed)\n-\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n-\t\t;;\n-\tok|broken)\n-\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n-\t\treturn\n+\t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1 ($test_case_where)\"\n \t\t;;\n \tesac\n+\n \techo >>$github_markup_output \"::group::$test_case_result: $this_test.$test_count $*\"\n \ttest-tool >>$github_markup_output path-utils skip-n-bytes \\\n \t\t\"$GIT_TEST_TEE_OUTPUT_FILE\" $GIT_TEST_TEE_OFFSET\n-- \ngitgitgadget\n\n"}]}