[PATCH] ci: point leak-sanitizer failures at the actual test and error

[PATCH] ci: point leak-sanitizer failures at the actual test and error

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A leak is only found once, at the end of a whole script, well after
every test already reported ok, and the failure annotation carried
no file or line, so all a reviewer ever saw was:

    Process completed with exit code 1.

with nothing to click through to. Stop each leak-sanitizer script at
its first failure instead of running the rest of an already-tainted
script, and have both failure and leak annotations point at the real
file and carry the actual error, for example:

    t/t1507-rev-parse-upstream.sh, line 1:
    memory leak logged around t1507.1
    ==ERROR: LeakSanitizer: detected memory leaks
    Direct leak of 60 byte(s) in 1 object(s) allocated from:
        ...
        #5 in add_branch builtin/remote.c:135

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    ci: point leak-sanitizer failures at the actual test and error
    
    I discovered while running CI on another GitHub pull request that it's
    very hard to see where the error is for the leak tests.
    
    This will stop each leak-sanitizer script at its first failure and
    points annotations at the real file and error.
    
    Proof that it works:
    https://github.com/git/git/actions/runs/35871180948/job/107215430244

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v1
Pull-Request: https://github.com/git/git/pull/2419

 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 49 +++++++++++++++++++++++-----
 t/test-lib.sh                        |  2 ++
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index c6ccbf8c17..a89f480a78 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)
 	export NO_CVS_TESTS=LetsSaveSomeTime
 	export NO_SVN_TESTS=LetsSaveSomeTime
 	export NO_P4_TESTS=LetsSaveSomeTime
+	GIT_TEST_OPTS="$GIT_TEST_OPTS --immediate"
 	;;
 linux-asan-ubsan)
 	export SANITIZE=address,undefined
diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index fa29a62aa3..4f6460a0ad 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -28,6 +28,7 @@ start_test_output () {
 	github_markup_output="${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup"
 	>$github_markup_output
 	GIT_TEST_TEE_OFFSET=0
+	github_markup_script_name=${0##*/}
 }
 
 # No need to override start_test_case_output
@@ -35,22 +36,54 @@ start_test_output () {
 finalize_test_case_output () {
 	test_case_result=$1
 	shift
+
+	case "$test_case_result" in
+	ok|broken)
+		# Exit without printing the "ok" or "broken" tests
+		return
+		;;
+	esac
+
+	test_case_line=$(find_test_case_line_ "$1")
+	test_case_output=$(test-tool path-utils skip-n-bytes \
+		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET)
+
 	case "$test_case_result" in
 	failure)
-		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
+		test_case_summary=$(printf '%s\n' "$test_case_output" |
+			tail -n 20 | github_escape_message_)
+		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"failed: $this_test.$test_count $1%0A%0A$test_case_summary"
 		;;
 	fixed)
-		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
-		;;
-	ok|broken)
-		# Exit without printing the "ok" or ""broken" tests
-		return
+		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"fixed: $this_test.$test_count $1"
 		;;
 	esac
+
 	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
-	test-tool >>$github_markup_output path-utils skip-n-bytes \
-		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
+	printf '%s\n' "$test_case_output" >>$github_markup_output
 	echo >>$github_markup_output "::endgroup::"
 }
 
+finalize_test_leak_output () {
+	test_leak_summary=$(head -n 40 "$TEST_RESULTS_SAN_FILE".* |
+		github_escape_message_)
+	github_annotation_ error "t/$github_markup_script_name" 1 \
+		"memory leak logged around $this_test.$test_count%0A%0A$test_leak_summary"
+}
+
 # No need to override finalize_test_output
+
+github_escape_message_ () {
+	sed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\r/%0D/g' -e 's/\n/%0A/g'
+}
+
+find_test_case_line_ () {
+	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
+	head -n 1 | cut -d: -f1
+}
+
+github_annotation_ () {
+	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
+}
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1f0505e412..a52589c6a2 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -199,6 +199,7 @@ mark_option_requires_arg () {
 start_test_output () { :; }
 start_test_case_output () { :; }
 finalize_test_case_output () { :; }
+finalize_test_leak_output () { :; }
 finalize_test_output () { :; }
 
 parse_option () {
@@ -1218,6 +1219,7 @@ check_test_results_san_file_ () {
 		return
 	fi &&
 	say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
+	finalize_test_leak_output &&
 
 	if test "$test_failure" = 0
 	then

base-commit: 3bc0341126508f78f5869cbfc0005e987efdf0c7
-- 
gitgitgadget

Re: [PATCH] ci: point leak-sanitizer failures at the actual test and error

[Ben Knoble]

> Le 25 sept. 2026 à 14:59, Harald Nordgren via GitGitGadget <gitgitgadget@gmail.com> a écrit :
> 
> +finalize_test_leak_output () {
> +    test_leak_summary=$(head -n 40 "$TEST_RESULTS_SAN_FILE".* |
> +        github_escape_message_)
> +    github_annotation_ error "t/$github_markup_script_name" 1 \
> +        "memory leak logged around $this_test.$test_count%0A%0A$test_leak_summary"
> +}
> +
> # No need to override finalize_test_output
> +
> +github_escape_message_ () {
> +    sed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\r/%0D/g' -e 's/\n/%0A/g'
> +}
> +
> +find_test_case_line_ () {
> +    grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
> +    head -n 1 | cut -d: -f1
> +}
> +
> +github_annotation_ () {
> +    echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
> +}

Without commenting on the rest, introducing the helpers first might make the important patch easier to read.  

Re: [PATCH] ci: point leak-sanitizer failures at the actual test and error

[Harald Nordgren]

> > +github_escape_message_ () {
> > +    sed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\r/%0D/g' -e 's/\n/%0A/g'
> > +}
> > +
> > +find_test_case_line_ () {
> > +    grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
> > +    head -n 1 | cut -d: -f1
> > +}
> > +
> > +github_annotation_ () {
> > +    echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
> > +}
>
> Without commenting on the rest, introducing the helpers first might make the important patch easier to read.

Good point!


Harald

Re: [PATCH] ci: point leak-sanitizer failures at the actual test and error

[Phillip Wood]

Hi Harald

On 25/09/2026 19:54, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
>      ci: point leak-sanitizer failures at the actual test and error
>      
>      I discovered while running CI on another GitHub pull request that it's
>      very hard to see where the error is for the leak tests.
>      
>      This will stop each leak-sanitizer script at its first failure and
>      points annotations at the real file and error.

Putting the leak output in the test results is very welcome, but does 
this mean that if there are two leaks we only report one?

>      Proof that it works:
>      https://github.com/git/git/actions/runs/35871180948/job/107215430244

Opening that link shows that the individual test failures are no-longer 
folded and I see some very strange scrolling behavior in firefox - when 
the page opens it scrolls to the bottom of the output of 
"ci/build-and-run-tests.sh" and if I try to scroll up it immediately 
scrolls back down as soon as my fingers leave the touchpad.

The patch below seems to do more than just changing the output to 
display the leak backtrace - it adds some escaping and changes the 
annotations. There is no explanation of what these changes do or why 
they are required.

Thanks

Phillip

> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v1
> Pull-Request: https://github.com/git/git/pull/2419
> 
>   ci/lib.sh                            |  1 +
>   t/test-lib-github-workflow-markup.sh | 49 +++++++++++++++++++++++-----
>   t/test-lib.sh                        |  2 ++
>   3 files changed, 44 insertions(+), 8 deletions(-)
> 
> diff --git a/ci/lib.sh b/ci/lib.sh
> index c6ccbf8c17..a89f480a78 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)
>   	export NO_CVS_TESTS=LetsSaveSomeTime
>   	export NO_SVN_TESTS=LetsSaveSomeTime
>   	export NO_P4_TESTS=LetsSaveSomeTime
> +	GIT_TEST_OPTS="$GIT_TEST_OPTS --immediate"
>   	;;
>   linux-asan-ubsan)
>   	export SANITIZE=address,undefined
> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
> index fa29a62aa3..4f6460a0ad 100644
> --- a/t/test-lib-github-workflow-markup.sh
> +++ b/t/test-lib-github-workflow-markup.sh
> @@ -28,6 +28,7 @@ start_test_output () {
>   	github_markup_output="${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup"
>   	>$github_markup_output
>   	GIT_TEST_TEE_OFFSET=0
> +	github_markup_script_name=${0##*/}
>   }
>   
>   # No need to override start_test_case_output
> @@ -35,22 +36,54 @@ start_test_output () {
>   finalize_test_case_output () {
>   	test_case_result=$1
>   	shift
> +
> +	case "$test_case_result" in
> +	ok|broken)
> +		# Exit without printing the "ok" or "broken" tests
> +		return
> +		;;
> +	esac
> +
> +	test_case_line=$(find_test_case_line_ "$1")
> +	test_case_output=$(test-tool path-utils skip-n-bytes \
> +		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET)
> +
>   	case "$test_case_result" in
>   	failure)
> -		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
> +		test_case_summary=$(printf '%s\n' "$test_case_output" |
> +			tail -n 20 | github_escape_message_)
> +		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
> +			"failed: $this_test.$test_count $1%0A%0A$test_case_summary"
>   		;;
>   	fixed)
> -		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
> -		;;
> -	ok|broken)
> -		# Exit without printing the "ok" or ""broken" tests
> -		return
> +		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
> +			"fixed: $this_test.$test_count $1"
>   		;;
>   	esac
> +
>   	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
> -	test-tool >>$github_markup_output path-utils skip-n-bytes \
> -		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
> +	printf '%s\n' "$test_case_output" >>$github_markup_output
>   	echo >>$github_markup_output "::endgroup::"
>   }
>   
> +finalize_test_leak_output () {
> +	test_leak_summary=$(head -n 40 "$TEST_RESULTS_SAN_FILE".* |
> +		github_escape_message_)
> +	github_annotation_ error "t/$github_markup_script_name" 1 \
> +		"memory leak logged around $this_test.$test_count%0A%0A$test_leak_summary"
> +}
> +
>   # No need to override finalize_test_output
> +
> +github_escape_message_ () {
> +	sed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\r/%0D/g' -e 's/\n/%0A/g'
> +}
> +
> +find_test_case_line_ () {
> +	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
> +	head -n 1 | cut -d: -f1
> +}
> +
> +github_annotation_ () {
> +	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
> +}
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 1f0505e412..a52589c6a2 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -199,6 +199,7 @@ mark_option_requires_arg () {
>   start_test_output () { :; }
>   start_test_case_output () { :; }
>   finalize_test_case_output () { :; }
> +finalize_test_leak_output () { :; }
>   finalize_test_output () { :; }
>   
>   parse_option () {
> @@ -1218,6 +1219,7 @@ check_test_results_san_file_ () {
>   		return
>   	fi &&
>   	say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
> +	finalize_test_leak_output &&
>   
>   	if test "$test_failure" = 0
>   	then
> 
> base-commit: 3bc0341126508f78f5869cbfc0005e987efdf0c7

Re: [PATCH] ci: point leak-sanitizer failures at the actual test and error

[Harald Nordgren]

> >      ci: point leak-sanitizer failures at the actual test and error
> >
> >      I discovered while running CI on another GitHub pull request that it's
> >      very hard to see where the error is for the leak tests.
> >
> >      This will stop each leak-sanitizer script at its first failure and
> >      points annotations at the real file and error.
>
> Putting the leak output in the test results is very welcome, but does
> this mean that if there are two leaks we only report one?

It already had a behavior where one failure made every subsequent test
in the script report "not ok" too, so lots of noise burying the real
leaks.

> >      Proof that it works:
> >      https://github.com/git/git/actions/runs/35871180948/job/107215430244
>
> Opening that link shows that the individual test failures are no-longer
> folded and I see some very strange scrolling behavior in firefox - when
> the page opens it scrolls to the bottom of the output of
> "ci/build-and-run-tests.sh" and if I try to scroll up it immediately
> scrolls back down as soon as my fingers leave the touchpad.

I'll take a look at that.

> The patch below seems to do more than just changing the output to
> display the leak backtrace - it adds some escaping and changes the
> annotations. There is no explanation of what these changes do or why
> they are required.

I'll expand the commit message.


Harald

[PATCH v2 0/2] ci: link failure and leak annotations to the test script

[Harald Nordgren via GitGitGadget]

Link failure and leak annotations in CI to the test script, so both can be
found from the job summary.

CI Job where failures and leaks are reported:
https://github.com/git/git/actions/runs/36449319996/job/109020434364?pr=2426

Changes in v2:

 * Split into two commits, each explaining its own reasoning.
 * Leak output is no longer capped or embedded in the message, it's now an
   uncapped fold, so multiple leaks in the same test both show in full. A
   second leak in a different test still won't show in the same run,
   --immediate stops the script at the first failure, but it no longer gets
   buried under every later test falsely reporting "not ok" either.
 * Drops the giant unfolded message that annotations used to carry, which is
   what probably caused the scrolling behavior.

Harald Nordgren (2):
  ci: annotate leaks and stop a leak-sanitizer script at its first
    failure
  ci: point test failures and fixed known breakages at their file and
    line

 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 57 ++++++++++++++++++++++++----
 t/test-lib.sh                        | 21 ++++++----
 3 files changed, 63 insertions(+), 16 deletions(-)


base-commit: 34f06850c16c7f7ac822b1adc71354f11b0f2ca3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v2
Pull-Request: https://github.com/git/git/pull/2419

Range-diff vs v1:

 1:  61b0355780 ! 1:  46e13a6e77 ci: point leak-sanitizer failures at the actual test and error
     @@ Metadata
      Author: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## Commit message ##
     -    ci: point leak-sanitizer failures at the actual test and error
     +    ci: annotate leaks and stop a leak-sanitizer script at its first failure
      
     -    A leak is only found once, at the end of a whole script, well after
     -    every test already reported ok, and the failure annotation carried
     -    no file or line, so all a reviewer ever saw was:
     +    A leak is only discovered once, at the end of a whole script, well
     +    after every test has already reported ok, and it gets no annotation at
     +    all, so a leak-sanitizer job's only visible failure is:
      
              Process completed with exit code 1.
      
     -    with nothing to click through to. Stop each leak-sanitizer script at
     -    its first failure instead of running the rest of an already-tainted
     -    script, and have both failure and leak annotations point at the real
     -    file and carry the actual error, for example:
     +    Give a leak its own annotation. Point it at the test script, the exact
     +    line isn't known, only which script the leak turned up in, and put the
     +    full sanitizer report in a log group next to it, so it stays visible
     +    and isn't capped to a handful of lines.
      
     -        t/t1507-rev-parse-upstream.sh, line 1:
     -        memory leak logged around t1507.1
     -        ==ERROR: LeakSanitizer: detected memory leaks
     -        Direct leak of 60 byte(s) in 1 object(s) allocated from:
     -            ...
     -            #5 in add_branch builtin/remote.c:135
     +    Once a script has one leak, it keeps running: the sanitizer log
     +    directory is never cleared between tests, so every later test in the
     +    same script sees the same leftover log entries and also reports "not
     +    ok", burying the one real failure in copies of itself. Stop a
     +    leak-sanitizer script at its first failure with --immediate instead.
     +
     +    A failing test already gets its own annotation once its script
     +    finishes, but --immediate exits as soon as that test fails, before
     +    reaching the code that writes it. Write the annotation first, so
     +    turning on --immediate here does not silently drop it.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
       	>$github_markup_output
       	GIT_TEST_TEE_OFFSET=0
      +	github_markup_script_name=${0##*/}
     ++}
     ++
     ++github_annotation_ () {
     ++	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
       }
       
       # No need to override start_test_case_output
     -@@ t/test-lib-github-workflow-markup.sh: start_test_output () {
     - finalize_test_case_output () {
     - 	test_case_result=$1
     - 	shift
     -+
     -+	case "$test_case_result" in
     -+	ok|broken)
     -+		# Exit without printing the "ok" or "broken" tests
     -+		return
     -+		;;
     -+	esac
     -+
     -+	test_case_line=$(find_test_case_line_ "$1")
     -+	test_case_output=$(test-tool path-utils skip-n-bytes \
     -+		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET)
     -+
     - 	case "$test_case_result" in
     - 	failure)
     --		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
     -+		test_case_summary=$(printf '%s\n' "$test_case_output" |
     -+			tail -n 20 | github_escape_message_)
     -+		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
     -+			"failed: $this_test.$test_count $1%0A%0A$test_case_summary"
     - 		;;
     - 	fixed)
     --		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
     --		;;
     --	ok|broken)
     --		# Exit without printing the "ok" or ""broken" tests
     --		return
     -+		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
     -+			"fixed: $this_test.$test_count $1"
     - 		;;
     - 	esac
     -+
     - 	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
     --	test-tool >>$github_markup_output path-utils skip-n-bytes \
     --		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
     -+	printf '%s\n' "$test_case_output" >>$github_markup_output
     +@@ t/test-lib-github-workflow-markup.sh: finalize_test_case_output () {
       	echo >>$github_markup_output "::endgroup::"
       }
       
      +finalize_test_leak_output () {
     -+	test_leak_summary=$(head -n 40 "$TEST_RESULTS_SAN_FILE".* |
     -+		github_escape_message_)
     ++	# The exact line the leak turned up on isn't known, only the script,
     ++	# so point at line 1.
      +	github_annotation_ error "t/$github_markup_script_name" 1 \
     -+		"memory leak logged around $this_test.$test_count%0A%0A$test_leak_summary"
     -+}
     ++		"memory leak logged in $this_test"
      +
     - # No need to override finalize_test_output
     -+
     -+github_escape_message_ () {
     -+	sed -e ':a' -e 'N' -e '$!ba' -e 's/%/%25/g' -e 's/\r/%0D/g' -e 's/\n/%0A/g'
     -+}
     -+
     -+find_test_case_line_ () {
     -+	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
     -+	head -n 1 | cut -d: -f1
     ++	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
     ++	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
     ++	echo >>$github_markup_output "::endgroup::"
      +}
      +
     -+github_annotation_ () {
     -+	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
     -+}
     + # No need to override finalize_test_output
      
       ## t/test-lib.sh ##
      @@ t/test-lib.sh: mark_option_requires_arg () {
     @@ t/test-lib.sh: mark_option_requires_arg () {
       finalize_test_output () { :; }
       
       parse_option () {
     +@@ t/test-lib.sh: test_failure_ () {
     + 	say_color error "not ok $test_count - ${pfx:+$pfx }$1"
     + 	shift
     + 	printf '%s\n' "$*" | sed -e 's/^/#	/'
     ++	if test -n "$immediate" && test -n "$invert_exit_code"
     ++	then
     ++		say_color error "1..$test_count"
     ++		finalize_test_output
     ++		_invert_exit_code_failure_end_blurb
     ++		GIT_EXIT_OK=t
     ++		exit 0
     ++	fi
     ++	# Write the annotation before the --immediate exit paths below,
     ++	# which call exit and would otherwise skip it.
     ++	finalize_test_case_output failure "$failure_label" "$@"
     + 	if test -n "$immediate"
     + 	then
     + 		say_color error "1..$test_count"
     +-		if test -n "$invert_exit_code"
     +-		then
     +-			finalize_test_output
     +-			_invert_exit_code_failure_end_blurb
     +-			GIT_EXIT_OK=t
     +-			exit 0
     +-		fi
     + 		check_test_results_san_file_ "$test_failure"
     + 		_error_exit
     + 	fi
     +-	finalize_test_case_output failure "$failure_label" "$@"
     + }
     + 
     + test_known_broken_ok_ () {
      @@ t/test-lib.sh: check_test_results_san_file_ () {
       		return
       	fi &&
 -:  ---------- > 2:  bffa8fb030 ci: point test failures and fixed known breakages at their file and line

-- 
gitgitgadget

[PATCH v2 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A leak is only discovered once, at the end of a whole script, well
after every test has already reported ok, and it gets no annotation at
all, so a leak-sanitizer job's only visible failure is:

    Process completed with exit code 1.

Give a leak its own annotation. Point it at the test script, the exact
line isn't known, only which script the leak turned up in, and put the
full sanitizer report in a log group next to it, so it stays visible
and isn't capped to a handful of lines.

Once a script has one leak, it keeps running: the sanitizer log
directory is never cleared between tests, so every later test in the
same script sees the same leftover log entries and also reports "not
ok", burying the one real failure in copies of itself. Stop a
leak-sanitizer script at its first failure with --immediate instead.

A failing test already gets its own annotation once its script
finishes, but --immediate exits as soon as that test fails, before
reaching the code that writes it. Write the annotation first, so
turning on --immediate here does not silently drop it.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++
 t/test-lib.sh                        | 21 +++++++++++++--------
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index c6ccbf8c17..a89f480a78 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)
 	export NO_CVS_TESTS=LetsSaveSomeTime
 	export NO_SVN_TESTS=LetsSaveSomeTime
 	export NO_P4_TESTS=LetsSaveSomeTime
+	GIT_TEST_OPTS="$GIT_TEST_OPTS --immediate"
 	;;
 linux-asan-ubsan)
 	export SANITIZE=address,undefined
diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index fa29a62aa3..0d54496358 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -28,6 +28,11 @@ start_test_output () {
 	github_markup_output="${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup"
 	>$github_markup_output
 	GIT_TEST_TEE_OFFSET=0
+	github_markup_script_name=${0##*/}
+}
+
+github_annotation_ () {
+	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
 }
 
 # No need to override start_test_case_output
@@ -53,4 +58,15 @@ finalize_test_case_output () {
 	echo >>$github_markup_output "::endgroup::"
 }
 
+finalize_test_leak_output () {
+	# The exact line the leak turned up on isn't known, only the script,
+	# so point at line 1.
+	github_annotation_ error "t/$github_markup_script_name" 1 \
+		"memory leak logged in $this_test"
+
+	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
+	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
+	echo >>$github_markup_output "::endgroup::"
+}
+
 # No need to override finalize_test_output
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1f0505e412..3552a19323 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -199,6 +199,7 @@ mark_option_requires_arg () {
 start_test_output () { :; }
 start_test_case_output () { :; }
 finalize_test_case_output () { :; }
+finalize_test_leak_output () { :; }
 finalize_test_output () { :; }
 
 parse_option () {
@@ -822,20 +823,23 @@ test_failure_ () {
 	say_color error "not ok $test_count - ${pfx:+$pfx }$1"
 	shift
 	printf '%s\n' "$*" | sed -e 's/^/#	/'
+	if test -n "$immediate" && test -n "$invert_exit_code"
+	then
+		say_color error "1..$test_count"
+		finalize_test_output
+		_invert_exit_code_failure_end_blurb
+		GIT_EXIT_OK=t
+		exit 0
+	fi
+	# Write the annotation before the --immediate exit paths below,
+	# which call exit and would otherwise skip it.
+	finalize_test_case_output failure "$failure_label" "$@"
 	if test -n "$immediate"
 	then
 		say_color error "1..$test_count"
-		if test -n "$invert_exit_code"
-		then
-			finalize_test_output
-			_invert_exit_code_failure_end_blurb
-			GIT_EXIT_OK=t
-			exit 0
-		fi
 		check_test_results_san_file_ "$test_failure"
 		_error_exit
 	fi
-	finalize_test_case_output failure "$failure_label" "$@"
 }
 
 test_known_broken_ok_ () {
@@ -1218,6 +1222,7 @@ check_test_results_san_file_ () {
 		return
 	fi &&
 	say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
+	finalize_test_leak_output &&
 
 	if test "$test_failure" = 0
 	then
-- 
gitgitgadget

Re: [PATCH v2 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> A leak is only discovered once, at the end of a whole script, well
> after every test has already reported ok, and it gets no annotation at
> all, so a leak-sanitizer job's only visible failure is:
>
>     Process completed with exit code 1.
>
> Give a leak its own annotation. Point it at the test script, the exact
> line isn't known, only which script the leak turned up in, and put the
> full sanitizer report in a log group next to it, so it stays visible
> and isn't capped to a handful of lines.
>
> Once a script has one leak, it keeps running: the sanitizer log
> directory is never cleared between tests, so every later test in the
> same script sees the same leftover log entries and also reports "not
> ok", burying the one real failure in copies of itself. Stop a
> leak-sanitizer script at its first failure with --immediate instead.

OK.  So the idea is that we do not have sanitizer report per
test_expect_* block but showing the single one over and over,
whether the next test_expect_* block has leaks, is not helpful, so
we just immediately kill the test script after the first leak?

> @@ -53,4 +58,15 @@ finalize_test_case_output () {
>  	echo >>$github_markup_output "::endgroup::"
>  }
>  
> +finalize_test_leak_output () {
> +	# The exact line the leak turned up on isn't known, only the script,
> +	# so point at line 1.
> +	github_annotation_ error "t/$github_markup_script_name" 1 \
> +		"memory leak logged in $this_test"
> +
> +	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
> +	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
> +	echo >>$github_markup_output "::endgroup::"
> +}
> +


> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 1f0505e412..3552a19323 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -199,6 +199,7 @@ mark_option_requires_arg () {
>  start_test_output () { :; }
>  start_test_case_output () { :; }
>  finalize_test_case_output () { :; }
> +finalize_test_leak_output () { :; }
>  finalize_test_output () { :; }
>  
>  parse_option () {
> @@ -822,20 +823,23 @@ test_failure_ () {
>  	say_color error "not ok $test_count - ${pfx:+$pfx }$1"
>  	shift
>  	printf '%s\n' "$*" | sed -e 's/^/#	/'
> +	if test -n "$immediate" && test -n "$invert_exit_code"
> +	then
> +		say_color error "1..$test_count"
> +		finalize_test_output
> +		_invert_exit_code_failure_end_blurb
> +		GIT_EXIT_OK=t
> +		exit 0
> +	fi
> +	# Write the annotation before the --immediate exit paths below,
> +	# which call exit and would otherwise skip it.
> +	finalize_test_case_output failure "$failure_label" "$@"
>  	if test -n "$immediate"
>  	then
>  		say_color error "1..$test_count"
> -		if test -n "$invert_exit_code"
> -		then
> -			finalize_test_output
> -			_invert_exit_code_failure_end_blurb
> -			GIT_EXIT_OK=t
> -			exit 0
> -		fi
>  		check_test_results_san_file_ "$test_failure"
>  		_error_exit
>  	fi

The two-line comment in the middle made me puzzled to see "exit 0"
just above it.  If "--immediate" is asked and we are checking leaks,
shouldn't we be doing finalize_test_case_output regardless of the
"invert" setting?

> -	finalize_test_case_output failure "$failure_label" "$@"
>  }
>  
>  test_known_broken_ok_ () {
> @@ -1218,6 +1222,7 @@ check_test_results_san_file_ () {
>  		return
>  	fi &&
>  	say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
> +	finalize_test_leak_output &&
>  
>  	if test "$test_failure" = 0
>  	then

Re: [PATCH v2 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure

[Harald Nordgren]

> > Once a script has one leak, it keeps running: the sanitizer log
> > directory is never cleared between tests, so every later test in the
> > same script sees the same leftover log entries and also reports "not
> > ok", burying the one real failure in copies of itself. Stop a
> > leak-sanitizer script at its first failure with --immediate instead.
>
> OK.  So the idea is that we do not have sanitizer report per
> test_expect_* block but showing the single one over and over,
> whether the next test_expect_* block has leaks, is not helpful, so
> we just immediately kill the test script after the first leak?

Yes that's it, one leak makes continuing pointless since every later
test would just see the same accumulated log, so we stop there
instead.

> >       if test -n "$immediate"
> >       then
> >               say_color error "1..$test_count"
> > -             if test -n "$invert_exit_code"
> > -             then
> > -                     finalize_test_output
> > -                     _invert_exit_code_failure_end_blurb
> > -                     GIT_EXIT_OK=t
> > -                     exit 0
> > -             fi
> >               check_test_results_san_file_ "$test_failure"
> >               _error_exit
> >       fi
>
> The two-line comment in the middle made me puzzled to see "exit 0"
> just above it.  If "--immediate" is asked and we are checking leaks,
> shouldn't we be doing finalize_test_case_output regardless of the
> "invert" setting?

I'll take a look at that, it might be a problem.


Harald

[PATCH v2 2/2] ci: point test failures and fixed known breakages at their file and line

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A test failure or a fixed known breakage gets an annotation that names
the test but carries no file or line, so there is nothing to click
through to from the GitHub UI.

Find the line a test is defined on by searching the script for its
description as a fixed string, using the first match. A description
can contain characters like `[` or `*` that a regex search would
misread, so match it literally. Fall back to line 1 when the
description is not found verbatim, which happens when a test builds
its description at runtime instead of writing it out literally.

A GitHub annotation is a single line, and a test description is always
one line too, so only a `%` or a stray carriage return in it needs
percent-encoding to keep the annotation intact. Escape `%` first, or a
carriage return's own encoding would be mangled by a `%` substitution
that ran after it.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 t/test-lib-github-workflow-markup.sh | 41 ++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index 0d54496358..67c5c3461c 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -31,6 +31,21 @@ start_test_output () {
 	github_markup_script_name=${0##*/}
 }
 
+github_escape_message_ () {
+	# A test description is always one line, so only % and CR need
+	# escaping here. Escape % first, or CR's own %-encoding gets mangled.
+	sed -e 's/%/%25/g' -e 's/\r/%0D/g'
+}
+
+find_test_case_line_ () {
+	# A description can contain characters like [ or * that would
+	# corrupt a regex search, so match it literally and take the first
+	# hit; -- keeps a description starting with "-" from being read as
+	# an option.
+	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
+	head -n 1 | cut -d: -f1
+}
+
 github_annotation_ () {
 	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
 }
@@ -40,21 +55,31 @@ github_annotation_ () {
 finalize_test_case_output () {
 	test_case_result=$1
 	shift
+
+	case "$test_case_result" in
+	ok|broken)
+		# Exit without printing the "ok" or "broken" tests
+		return
+		;;
+	esac
+
+	test_case_line=$(find_test_case_line_ "$1")
+	test_case_description=$(printf '%s' "$1" | github_escape_message_)
+
 	case "$test_case_result" in
 	failure)
-		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
+		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"failed: $this_test.$test_count $test_case_description"
 		;;
 	fixed)
-		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
-		;;
-	ok|broken)
-		# Exit without printing the "ok" or ""broken" tests
-		return
+		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"fixed: $this_test.$test_count $test_case_description"
 		;;
 	esac
+
 	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
-	test-tool >>$github_markup_output path-utils skip-n-bytes \
-		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
+	test-tool path-utils skip-n-bytes \
+		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET >>$github_markup_output
 	echo >>$github_markup_output "::endgroup::"
 }
 
-- 
gitgitgadget

Re: [PATCH v2 2/2] ci: point test failures and fixed known breakages at their file and line

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  t/test-lib-github-workflow-markup.sh | 41 ++++++++++++++++++++++------
>  1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
> index 0d54496358..67c5c3461c 100644
> --- a/t/test-lib-github-workflow-markup.sh
> +++ b/t/test-lib-github-workflow-markup.sh
> @@ -31,6 +31,21 @@ start_test_output () {
>  	github_markup_script_name=${0##*/}
>  }
>  
> +github_escape_message_ () {
> +	# A test description is always one line, so only % and CR need
> +	# escaping here. Escape % first, or CR's own %-encoding gets mangled.
> +	sed -e 's/%/%25/g' -e 's/\r/%0D/g'
> +}

Is it portable to feed a two-letter sequence "\r" to "sed" and
expect it to be interpreted as Carriage Return?  Implementations of
BSD lineage "sed" don't grok it if I recall correctly.

> +find_test_case_line_ () {
> +	# A description can contain characters like [ or * that would
> +	# corrupt a regex search, so match it literally and take the first
> +	# hit; -- keeps a description starting with "-" from being read as
> +	# an option.
> +	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
> +	head -n 1 | cut -d: -f1
> +}
> +
>  github_annotation_ () {
>  	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
>  }
> @@ -40,21 +55,31 @@ github_annotation_ () {
>  finalize_test_case_output () {
>  	test_case_result=$1
>  	shift
> +
> +	case "$test_case_result" in
> +	ok|broken)
> +		# Exit without printing the "ok" or "broken" tests
> +		return
> +		;;
> +	esac
> +
> +	test_case_line=$(find_test_case_line_ "$1")
> +	test_case_description=$(printf '%s' "$1" | github_escape_message_)
> +
>  	case "$test_case_result" in
>  	failure)
> -		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
> +		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
> +			"failed: $this_test.$test_count $test_case_description"
>  		;;
>  	fixed)
> -		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
> -		;;
> -	ok|broken)
> -		# Exit without printing the "ok" or ""broken" tests
> -		return
> +		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
> +			"fixed: $this_test.$test_count $test_case_description"
>  		;;
>  	esac
> +

All of the above may make sense, but ...

>  	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
> -	test-tool >>$github_markup_output path-utils skip-n-bytes \
> -		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
> +	test-tool path-utils skip-n-bytes \
> +		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET >>$github_markup_output
>  	echo >>$github_markup_output "::endgroup::"

What is this change about?  In the original, all surrounding code
has redirection early on the command line, and breaking that pattern
is the only difference between the removed and added lines here as
far as I can see.

>  }

[PATCH v3 0/2] ci: link failure and leak annotations to the test script

[Harald Nordgren via GitGitGadget]

Link failure and leak annotations in CI to the test script, so both can be
found from the job summary.

V3 CI Job where failures and leaks are reported:
https://github.com/git/git/actions/runs/36537917146/job/109306215909?pr=2426

Changes in v3:

 * Fixed bug in the --immediate exit ordering: the --immediate &&
   --invert-exit-code path called exit 0 before the test's annotation was
   written, now a single unconditional call covers both exit paths.
 * github_escape_message_ no longer relies on \r being a portable sed escape
   sequence (not POSIX-guaranteed and BSD sed implementations can differ),
   it splices in the literal carriage-return byte via printf instead.
 * Reverted unrelated test-tool line back to its original form.

Changes in v2:

 * Split into two commits, each explaining its own reasoning.
 * Leak output is no longer capped or embedded in the message, it's now an
   uncapped fold, so multiple leaks in the same test both show in full. A
   second leak in a different test still won't show in the same run,
   --immediate stops the script at the first failure, but it no longer gets
   buried under every later test falsely reporting "not ok" either.
 * Drops the giant unfolded message that annotations used to carry, which is
   what probably caused the scrolling behavior.

Harald Nordgren (2):
  ci: annotate leaks and stop a leak-sanitizer script at its first
    failure
  ci: point test failures and fixed known breakages at their file and
    line

 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 54 ++++++++++++++++++++++++----
 t/test-lib.sh                        |  6 +++-
 3 files changed, 54 insertions(+), 7 deletions(-)


base-commit: a018953688f1b10bddf91bff8747068f5f4746a4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v3
Pull-Request: https://github.com/git/git/pull/2419

Range-diff vs v2:

 1:  46e13a6e77 ! 1:  b6a36820ae ci: annotate leaks and stop a leak-sanitizer script at its first failure
     @@ t/test-lib.sh: test_failure_ () {
       	say_color error "not ok $test_count - ${pfx:+$pfx }$1"
       	shift
       	printf '%s\n' "$*" | sed -e 's/^/#	/'
     -+	if test -n "$immediate" && test -n "$invert_exit_code"
     -+	then
     -+		say_color error "1..$test_count"
     -+		finalize_test_output
     -+		_invert_exit_code_failure_end_blurb
     -+		GIT_EXIT_OK=t
     -+		exit 0
     -+	fi
     -+	# Write the annotation before the --immediate exit paths below,
     -+	# which call exit and would otherwise skip it.
     ++	# Write the annotation before either --immediate exit path below,
     ++	# both of which call exit and would otherwise skip it.
      +	finalize_test_case_output failure "$failure_label" "$@"
       	if test -n "$immediate"
       	then
       		say_color error "1..$test_count"
     --		if test -n "$invert_exit_code"
     --		then
     --			finalize_test_output
     --			_invert_exit_code_failure_end_blurb
     --			GIT_EXIT_OK=t
     --			exit 0
     --		fi
     +@@ t/test-lib.sh: test_failure_ () {
       		check_test_results_san_file_ "$test_failure"
       		_error_exit
       	fi
 2:  bffa8fb030 ! 2:  750c360512 ci: point test failures and fixed known breakages at their file and line
     @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
      +github_escape_message_ () {
      +	# A test description is always one line, so only % and CR need
      +	# escaping here. Escape % first, or CR's own %-encoding gets mangled.
     -+	sed -e 's/%/%25/g' -e 's/\r/%0D/g'
     ++	# \r is not a portable sed escape, so splice in the actual byte.
     ++	sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"
      +}
      +
      +find_test_case_line_ () {
     @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
       	esac
      +
       	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
     --	test-tool >>$github_markup_output path-utils skip-n-bytes \
     --		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
     -+	test-tool path-utils skip-n-bytes \
     -+		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET >>$github_markup_output
     - 	echo >>$github_markup_output "::endgroup::"
     - }
     - 
     + 	test-tool >>$github_markup_output path-utils skip-n-bytes \
     + 		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET

-- 
gitgitgadget

[PATCH v3 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A leak is only discovered once, at the end of a whole script, well
after every test has already reported ok, and it gets no annotation at
all, so a leak-sanitizer job's only visible failure is:

    Process completed with exit code 1.

Give a leak its own annotation. Point it at the test script, the exact
line isn't known, only which script the leak turned up in, and put the
full sanitizer report in a log group next to it, so it stays visible
and isn't capped to a handful of lines.

Once a script has one leak, it keeps running: the sanitizer log
directory is never cleared between tests, so every later test in the
same script sees the same leftover log entries and also reports "not
ok", burying the one real failure in copies of itself. Stop a
leak-sanitizer script at its first failure with --immediate instead.

A failing test already gets its own annotation once its script
finishes, but --immediate exits as soon as that test fails, before
reaching the code that writes it. Write the annotation first, so
turning on --immediate here does not silently drop it.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++
 t/test-lib.sh                        |  6 +++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index c6ccbf8c17..a89f480a78 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)
 	export NO_CVS_TESTS=LetsSaveSomeTime
 	export NO_SVN_TESTS=LetsSaveSomeTime
 	export NO_P4_TESTS=LetsSaveSomeTime
+	GIT_TEST_OPTS="$GIT_TEST_OPTS --immediate"
 	;;
 linux-asan-ubsan)
 	export SANITIZE=address,undefined
diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index fa29a62aa3..0d54496358 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -28,6 +28,11 @@ start_test_output () {
 	github_markup_output="${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup"
 	>$github_markup_output
 	GIT_TEST_TEE_OFFSET=0
+	github_markup_script_name=${0##*/}
+}
+
+github_annotation_ () {
+	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
 }
 
 # No need to override start_test_case_output
@@ -53,4 +58,15 @@ finalize_test_case_output () {
 	echo >>$github_markup_output "::endgroup::"
 }
 
+finalize_test_leak_output () {
+	# The exact line the leak turned up on isn't known, only the script,
+	# so point at line 1.
+	github_annotation_ error "t/$github_markup_script_name" 1 \
+		"memory leak logged in $this_test"
+
+	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
+	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
+	echo >>$github_markup_output "::endgroup::"
+}
+
 # No need to override finalize_test_output
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1f0505e412..e74a12f1dd 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -199,6 +199,7 @@ mark_option_requires_arg () {
 start_test_output () { :; }
 start_test_case_output () { :; }
 finalize_test_case_output () { :; }
+finalize_test_leak_output () { :; }
 finalize_test_output () { :; }
 
 parse_option () {
@@ -822,6 +823,9 @@ test_failure_ () {
 	say_color error "not ok $test_count - ${pfx:+$pfx }$1"
 	shift
 	printf '%s\n' "$*" | sed -e 's/^/#	/'
+	# Write the annotation before either --immediate exit path below,
+	# both of which call exit and would otherwise skip it.
+	finalize_test_case_output failure "$failure_label" "$@"
 	if test -n "$immediate"
 	then
 		say_color error "1..$test_count"
@@ -835,7 +839,6 @@ test_failure_ () {
 		check_test_results_san_file_ "$test_failure"
 		_error_exit
 	fi
-	finalize_test_case_output failure "$failure_label" "$@"
 }
 
 test_known_broken_ok_ () {
@@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {
 		return
 	fi &&
 	say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
+	finalize_test_leak_output &&
 
 	if test "$test_failure" = 0
 	then
-- 
gitgitgadget

Re: [PATCH v3 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure

[Phillip Wood]

Hi Harald

On 30/09/2026 07:09, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> A leak is only discovered once, at the end of a whole script, well
> after every test has already reported ok, and it gets no annotation at
> all, so a leak-sanitizer job's only visible failure is:
> 
>      Process completed with exit code 1.
> 
> Give a leak its own annotation. Point it at the test script, the exact
> line isn't known, only which script the leak turned up in, and put the
> full sanitizer report in a log group next to it, so it stays visible
> and isn't capped to a handful of lines.

It is good that it reports the full LSAN but is it really necessary to 
emphasize that - why would anyone reading this think it might be 
abbreviated?

What does adding the file and line number to the annotation buy us? I 
was hoping there'd be a link in the output to the test source but I 
can't see anything like that. The output is displayed as

     Error: failed: t7603.3 pull c2, c3, c4, c5 into c1
     ▶failure: t7603.3 pull c2, c3, c4, c5 into c1
     Error: memory leak logged in t7603
     ▶leak: t7603.3

and clicking on the '▶' lines expands the output, but there is nothing 
about the source file as far as I can see. Ideally, if the test is 
failing due to a leak, it would be nice to report that as

     Error: leak detected in: t7603.3 pull c2, c3, c4, c5 into c1
     ▶failure: t7603.3 pull c2, c3, c4, c5 into c1

and display the test output and LSAN output together when the 
"▶failure:" line is clicked, rather than having separate sections for 
the test output and leak output. Having said that what you've already 
implemented is a clear improvement so I'd be happy to take that if you 
don't feel like devoting any more time to it.

> Once a script has one leak, it keeps running: the sanitizer log
> directory is never cleared between tests, so every later test in the
> same script sees the same leftover log entries and also reports "not
> ok", burying the one real failure in copies of itself. Stop a
> leak-sanitizer script at its first failure with --immediate instead.

I think that is probably a welcome improvement though if two different 
tests have different leaks it would be nice to be able to show both.

Thanks for working on this, it makes the LSAN output much more accessible.

Phillip

> A failing test already gets its own annotation once its script
> finishes, but --immediate exits as soon as that test fails, before
> reaching the code that writes it. Write the annotation first, so
> turning on --immediate here does not silently drop it.
> 
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>   ci/lib.sh                            |  1 +
>   t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++
>   t/test-lib.sh                        |  6 +++++-
>   3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/ci/lib.sh b/ci/lib.sh
> index c6ccbf8c17..a89f480a78 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)
>   	export NO_CVS_TESTS=LetsSaveSomeTime
>   	export NO_SVN_TESTS=LetsSaveSomeTime
>   	export NO_P4_TESTS=LetsSaveSomeTime
> +	GIT_TEST_OPTS="$GIT_TEST_OPTS --immediate"
>   	;;
>   linux-asan-ubsan)
>   	export SANITIZE=address,undefined
> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
> index fa29a62aa3..0d54496358 100644
> --- a/t/test-lib-github-workflow-markup.sh
> +++ b/t/test-lib-github-workflow-markup.sh
> @@ -28,6 +28,11 @@ start_test_output () {
>   	github_markup_output="${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup"
>   	>$github_markup_output
>   	GIT_TEST_TEE_OFFSET=0
> +	github_markup_script_name=${0##*/}
> +}
> +
> +github_annotation_ () {
> +	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
>   }
>   
>   # No need to override start_test_case_output
> @@ -53,4 +58,15 @@ finalize_test_case_output () {
>   	echo >>$github_markup_output "::endgroup::"
>   }
>   
> +finalize_test_leak_output () {
> +	# The exact line the leak turned up on isn't known, only the script,
> +	# so point at line 1.
> +	github_annotation_ error "t/$github_markup_script_name" 1 \
> +		"memory leak logged in $this_test"
> +
> +	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
> +	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
> +	echo >>$github_markup_output "::endgroup::"
> +}
> +
>   # No need to override finalize_test_output
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 1f0505e412..e74a12f1dd 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -199,6 +199,7 @@ mark_option_requires_arg () {
>   start_test_output () { :; }
>   start_test_case_output () { :; }
>   finalize_test_case_output () { :; }
> +finalize_test_leak_output () { :; }
>   finalize_test_output () { :; }
>   
>   parse_option () {
> @@ -822,6 +823,9 @@ test_failure_ () {
>   	say_color error "not ok $test_count - ${pfx:+$pfx }$1"
>   	shift
>   	printf '%s\n' "$*" | sed -e 's/^/#	/'
> +	# Write the annotation before either --immediate exit path below,
> +	# both of which call exit and would otherwise skip it.
> +	finalize_test_case_output failure "$failure_label" "$@"
>   	if test -n "$immediate"
>   	then
>   		say_color error "1..$test_count"
> @@ -835,7 +839,6 @@ test_failure_ () {
>   		check_test_results_san_file_ "$test_failure"
>   		_error_exit
>   	fi
> -	finalize_test_case_output failure "$failure_label" "$@"
>   }
>   
>   test_known_broken_ok_ () {
> @@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {
>   		return
>   	fi &&
>   	say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
> +	finalize_test_leak_output &&
>   
>   	if test "$test_failure" = 0
>   	then

[PATCH v3 2/2] ci: point test failures and fixed known breakages at their file and line

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A test failure or a fixed known breakage gets an annotation that names
the test but carries no file or line, so there is nothing to click
through to from the GitHub UI.

Find the line a test is defined on by searching the script for its
description as a fixed string, using the first match. A description
can contain characters like `[` or `*` that a regex search would
misread, so match it literally. Fall back to line 1 when the
description is not found verbatim, which happens when a test builds
its description at runtime instead of writing it out literally.

A GitHub annotation is a single line, and a test description is always
one line too, so only a `%` or a stray carriage return in it needs
percent-encoding to keep the annotation intact. Escape `%` first, or a
carriage return's own encoding would be mangled by a `%` substitution
that ran after it.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 t/test-lib-github-workflow-markup.sh | 38 +++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index 0d54496358..66d2ccca18 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -31,6 +31,22 @@ start_test_output () {
 	github_markup_script_name=${0##*/}
 }
 
+github_escape_message_ () {
+	# A test description is always one line, so only % and CR need
+	# escaping here. Escape % first, or CR's own %-encoding gets mangled.
+	# \r is not a portable sed escape, so splice in the actual byte.
+	sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"
+}
+
+find_test_case_line_ () {
+	# A description can contain characters like [ or * that would
+	# corrupt a regex search, so match it literally and take the first
+	# hit; -- keeps a description starting with "-" from being read as
+	# an option.
+	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
+	head -n 1 | cut -d: -f1
+}
+
 github_annotation_ () {
 	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
 }
@@ -40,18 +56,28 @@ github_annotation_ () {
 finalize_test_case_output () {
 	test_case_result=$1
 	shift
+
+	case "$test_case_result" in
+	ok|broken)
+		# Exit without printing the "ok" or "broken" tests
+		return
+		;;
+	esac
+
+	test_case_line=$(find_test_case_line_ "$1")
+	test_case_description=$(printf '%s' "$1" | github_escape_message_)
+
 	case "$test_case_result" in
 	failure)
-		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
+		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"failed: $this_test.$test_count $test_case_description"
 		;;
 	fixed)
-		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
-		;;
-	ok|broken)
-		# Exit without printing the "ok" or ""broken" tests
-		return
+		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"fixed: $this_test.$test_count $test_case_description"
 		;;
 	esac
+
 	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
 	test-tool >>$github_markup_output path-utils skip-n-bytes \
 		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
-- 
gitgitgadget

Re: [PATCH v3 2/2] ci: point test failures and fixed known breakages at their file and line

[Phillip Wood]

Hi Harald

On 30/09/2026 07:09, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> A test failure or a fixed known breakage gets an annotation that names
> the test but carries no file or line, so there is nothing to click
> through to from the GitHub UI.

Have you got an example of this? As I said in my last mail, I can't see 
any links in the output from the linux-leaks job.

> Find the line a test is defined on by searching the script for its
> description as a fixed string, using the first match. A description
> can contain characters like `[` or `*` that a regex search would
> misread, so match it literally. 

This second sentence doesn't really add anything - you've already said 
we're searching for a fixed string.

> Fall back to line 1 when the
> description is not found verbatim, which happens when a test builds
> its description at runtime instead of writing it out literally.

Ironically, it is the dynamically generated tests where a line number 
would be most useful, but there is no easy way to determine what line we 
should be using.

> A GitHub annotation is a single line, and a test description is always
> one line too, so only a `%` or a stray carriage return in it needs
> percent-encoding to keep the annotation intact. Escape `%` first, or a
> carriage return's own encoding would be mangled by a `%` substitution
> that ran after it.

Why do we need to escape the test descriptions when we haven't been 
doing so up to now? Also if the test description is a single line why 
are we worring about '\r'? If it is so important to escape the output 
why does this patch not convert the existing annotations like the 
"group::" on in the trailing context lines?

Thanks

Phillip

> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>   t/test-lib-github-workflow-markup.sh | 38 +++++++++++++++++++++++-----
>   1 file changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
> index 0d54496358..66d2ccca18 100644
> --- a/t/test-lib-github-workflow-markup.sh
> +++ b/t/test-lib-github-workflow-markup.sh
> @@ -31,6 +31,22 @@ start_test_output () {
>   	github_markup_script_name=${0##*/}
>   }
>   
> +github_escape_message_ () {
> +	# A test description is always one line, so only % and CR need
> +	# escaping here. Escape % first, or CR's own %-encoding gets mangled.
> +	# \r is not a portable sed escape, so splice in the actual byte.
> +	sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"
> +}
> +
> +find_test_case_line_ () {
> +	# A description can contain characters like [ or * that would
> +	# corrupt a regex search, so match it literally and take the first
> +	# hit; -- keeps a description starting with "-" from being read as
> +	# an option.
> +	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
> +	head -n 1 | cut -d: -f1
> +}
> +
>   github_annotation_ () {
>   	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
>   }
> @@ -40,18 +56,28 @@ github_annotation_ () {
>   finalize_test_case_output () {
>   	test_case_result=$1
>   	shift
> +
> +	case "$test_case_result" in
> +	ok|broken)
> +		# Exit without printing the "ok" or "broken" tests
> +		return
> +		;;
> +	esac
> +
> +	test_case_line=$(find_test_case_line_ "$1")
> +	test_case_description=$(printf '%s' "$1" | github_escape_message_)
> +
>   	case "$test_case_result" in
>   	failure)
> -		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
> +		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
> +			"failed: $this_test.$test_count $test_case_description"
>   		;;
>   	fixed)
> -		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
> -		;;
> -	ok|broken)
> -		# Exit without printing the "ok" or ""broken" tests
> -		return
> +		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
> +			"fixed: $this_test.$test_count $test_case_description"
>   		;;
>   	esac
> +
>   	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
>   	test-tool >>$github_markup_output path-utils skip-n-bytes \
>   		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET

Re: [PATCH v3 2/2] ci: point test failures and fixed known breakages at their file and line

[Harald Nordgren]

On Wed, Sep 30, 2026 at 4:56 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Harald
>
> On 30/09/2026 07:09, Harald Nordgren via GitGitGadget wrote:
> > From: Harald Nordgren <haraldnordgren@gmail.com>
> >
> > A test failure or a fixed known breakage gets an annotation that names
> > the test but carries no file or line, so there is nothing to click
> > through to from the GitHub UI.
>
> Have you got an example of this? As I said in my last mail, I can't see
> any links in the output from the linux-leaks job.

That's poor wording on my side, I'll clarify.

> Why do we need to escape the test descriptions when we haven't been
> doing so up to now? Also if the test description is a single line why
> are we worring about '\r'? If it is so important to escape the output
> why does this patch not convert the existing annotations like the
> "group::" on in the trailing context lines?

Yeah, that can be simplified.


Harald

Re: [PATCH v3 0/2] ci: link failure and leak annotations to the test script

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Link failure and leak annotations in CI to the test script, so both can be
> found from the job summary.
>
> V3 CI Job where failures and leaks are reported:
> https://github.com/git/git/actions/runs/36537917146/job/109306215909?pr=2426
>
> Changes in v3:
>
>  * Fixed bug in the --immediate exit ordering: the --immediate &&
>    --invert-exit-code path called exit 0 before the test's annotation was
>    written, now a single unconditional call covers both exit paths.
>  * github_escape_message_ no longer relies on \r being a portable sed escape
>    sequence (not POSIX-guaranteed and BSD sed implementations can differ),
>    it splices in the literal carriage-return byte via printf instead.
>  * Reverted unrelated test-tool line back to its original form.

With these updates, the patches look good to me.  Unless others
spot problems I failed to see, let me mark the topic for 'next'.

Thanks.

Re: [PATCH v3 0/2] ci: link failure and leak annotations to the test script

[Phillip Wood]

On 30/09/2026 15:37, Junio C Hamano wrote:
> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> With these updates, the patches look good to me.  Unless others
> spot problems I failed to see, let me mark the topic for 'next'.
I've left a couple of comments. This version is a nice improvement on 
the status quo, but I'd like some clarity on what the filename and line 
number annotations actually do, and why we selectively escape the 
annotations.

Thanks

Phillip

Re: [PATCH v3 0/2] ci: link failure and leak annotations to the test script

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 30/09/2026 15:37, Junio C Hamano wrote:
>> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> 
>> With these updates, the patches look good to me.  Unless others
>> spot problems I failed to see, let me mark the topic for 'next'.
> I've left a couple of comments. This version is a nice improvement on 
> the status quo, but I'd like some clarity on what the filename and line 
> number annotations actually do, and why we selectively escape the 
> annotations.
>
> Thanks
>
> Phillip

Thanks.

[PATCH v4 0/2] ci: link failure and leak annotations to the test script

[Harald Nordgren via GitGitGadget]

Link failure and leak annotations in CI to the test script, so both can be
found from the job summary.

V4 CI Job where failures and leaks are reported:
https://github.com/git/git/actions/runs/36760014277/job/110039964565?pr=2426

Changes in v4:

 * Clarify commit messages and simplify escaping logic.

Changes in v3:

 * Fixed bug in the --immediate exit ordering: the --immediate &&
   --invert-exit-code path called exit 0 before the test's annotation was
   written, now a single unconditional call covers both exit paths.
 * github_escape_message_ no longer relies on \r being a portable sed escape
   sequence (not POSIX-guaranteed and BSD sed implementations can differ),
   it splices in the literal carriage-return byte via printf instead.
 * Reverted unrelated test-tool line back to its original form.

Changes in v2:

 * Split into two commits, each explaining its own reasoning.
 * Leak output is no longer capped or embedded in the message, it's now an
   uncapped fold, so multiple leaks in the same test both show in full. A
   second leak in a different test still won't show in the same run,
   --immediate stops the script at the first failure, but it no longer gets
   buried under every later test falsely reporting "not ok" either.
 * Drops the giant unfolded message that annotations used to carry, which is
   what probably caused the scrolling behavior.

Harald Nordgren (2):
  ci: annotate leaks and stop a leak-sanitizer script at its first
    failure
  ci: point test failures and fixed known breakages at their file and
    line

 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 54 ++++++++++++++++++++++++----
 t/test-lib.sh                        |  6 +++-
 3 files changed, 54 insertions(+), 7 deletions(-)


base-commit: a018953688f1b10bddf91bff8747068f5f4746a4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v4
Pull-Request: https://github.com/git/git/pull/2419

Range-diff vs v3:

 1:  b6a36820ae ! 1:  138394b48b ci: annotate leaks and stop a leak-sanitizer script at its first failure
     @@ Commit message
      
          Give a leak its own annotation. Point it at the test script, the exact
          line isn't known, only which script the leak turned up in, and put the
     -    full sanitizer report in a log group next to it, so it stays visible
     -    and isn't capped to a handful of lines.
     +    sanitizer report in a log group next to it, so it stays visible.
      
          Once a script has one leak, it keeps running: the sanitizer log
          directory is never cleared between tests, so every later test in the
 2:  750c360512 ! 2:  8ec2b53d82 ci: point test failures and fixed known breakages at their file and line
     @@ Commit message
          ci: point test failures and fixed known breakages at their file and line
      
          A test failure or a fixed known breakage gets an annotation that names
     -    the test but carries no file or line, so there is nothing to click
     -    through to from the GitHub UI.
     +    the test but says nothing about where it's defined, so a reviewer has
     +    to search the script by hand to find it.
      
          Find the line a test is defined on by searching the script for its
     -    description as a fixed string, using the first match. A description
     -    can contain characters like `[` or `*` that a regex search would
     -    misread, so match it literally. Fall back to line 1 when the
     -    description is not found verbatim, which happens when a test builds
     -    its description at runtime instead of writing it out literally.
     +    description as a fixed string, using the first match. Fall back to
     +    line 1 when the description is not found verbatim, which happens when
     +    a test builds its description at runtime instead of writing it out
     +    literally.
      
     -    A GitHub annotation is a single line, and a test description is always
     -    one line too, so only a `%` or a stray carriage return in it needs
     -    percent-encoding to keep the annotation intact. Escape `%` first, or a
     -    carriage return's own encoding would be mangled by a `%` substitution
     -    that ran after it.
     +    A GitHub annotation is a single line, so a `%` in a test description
     +    has to be percent-encoded as `%25`, or GitHub misreads it as its own
     +    escape sequence. for-each-ref's format atoms use plenty of them, e.g.
     +    `%(raw)`.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
       }
       
      +github_escape_message_ () {
     -+	# A test description is always one line, so only % and CR need
     -+	# escaping here. Escape % first, or CR's own %-encoding gets mangled.
     -+	# \r is not a portable sed escape, so splice in the actual byte.
     -+	sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"
     ++	# % has to be escaped or GitHub misreads it as the start of its own
     ++	# percent-encoding (e.g. a literal %(raw) in a for-each-ref test
     ++	# description).
     ++	sed -e 's/%/%25/g'
      +}
      +
      +find_test_case_line_ () {
      +	# A description can contain characters like [ or * that would
      +	# corrupt a regex search, so match it literally and take the first
     -+	# hit; -- keeps a description starting with "-" from being read as
     -+	# an option.
     ++	# hit. The -- keeps a description starting with "-" from being read
     ++	# as an option.
      +	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
      +	head -n 1 | cut -d: -f1
      +}

-- 
gitgitgadget

[PATCH v4 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A leak is only discovered once, at the end of a whole script, well
after every test has already reported ok, and it gets no annotation at
all, so a leak-sanitizer job's only visible failure is:

    Process completed with exit code 1.

Give a leak its own annotation. Point it at the test script, the exact
line isn't known, only which script the leak turned up in, and put the
sanitizer report in a log group next to it, so it stays visible.

Once a script has one leak, it keeps running: the sanitizer log
directory is never cleared between tests, so every later test in the
same script sees the same leftover log entries and also reports "not
ok", burying the one real failure in copies of itself. Stop a
leak-sanitizer script at its first failure with --immediate instead.

A failing test already gets its own annotation once its script
finishes, but --immediate exits as soon as that test fails, before
reaching the code that writes it. Write the annotation first, so
turning on --immediate here does not silently drop it.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++
 t/test-lib.sh                        |  6 +++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index c6ccbf8c17..a89f480a78 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)
 	export NO_CVS_TESTS=LetsSaveSomeTime
 	export NO_SVN_TESTS=LetsSaveSomeTime
 	export NO_P4_TESTS=LetsSaveSomeTime
+	GIT_TEST_OPTS="$GIT_TEST_OPTS --immediate"
 	;;
 linux-asan-ubsan)
 	export SANITIZE=address,undefined
diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index fa29a62aa3..0d54496358 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -28,6 +28,11 @@ start_test_output () {
 	github_markup_output="${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup"
 	>$github_markup_output
 	GIT_TEST_TEE_OFFSET=0
+	github_markup_script_name=${0##*/}
+}
+
+github_annotation_ () {
+	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
 }
 
 # No need to override start_test_case_output
@@ -53,4 +58,15 @@ finalize_test_case_output () {
 	echo >>$github_markup_output "::endgroup::"
 }
 
+finalize_test_leak_output () {
+	# The exact line the leak turned up on isn't known, only the script,
+	# so point at line 1.
+	github_annotation_ error "t/$github_markup_script_name" 1 \
+		"memory leak logged in $this_test"
+
+	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
+	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
+	echo >>$github_markup_output "::endgroup::"
+}
+
 # No need to override finalize_test_output
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1f0505e412..e74a12f1dd 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -199,6 +199,7 @@ mark_option_requires_arg () {
 start_test_output () { :; }
 start_test_case_output () { :; }
 finalize_test_case_output () { :; }
+finalize_test_leak_output () { :; }
 finalize_test_output () { :; }
 
 parse_option () {
@@ -822,6 +823,9 @@ test_failure_ () {
 	say_color error "not ok $test_count - ${pfx:+$pfx }$1"
 	shift
 	printf '%s\n' "$*" | sed -e 's/^/#	/'
+	# Write the annotation before either --immediate exit path below,
+	# both of which call exit and would otherwise skip it.
+	finalize_test_case_output failure "$failure_label" "$@"
 	if test -n "$immediate"
 	then
 		say_color error "1..$test_count"
@@ -835,7 +839,6 @@ test_failure_ () {
 		check_test_results_san_file_ "$test_failure"
 		_error_exit
 	fi
-	finalize_test_case_output failure "$failure_label" "$@"
 }
 
 test_known_broken_ok_ () {
@@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {
 		return
 	fi &&
 	say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
+	finalize_test_leak_output &&
 
 	if test "$test_failure" = 0
 	then
-- 
gitgitgadget

[PATCH v4 2/2] ci: point test failures and fixed known breakages at their file and line

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A test failure or a fixed known breakage gets an annotation that names
the test but says nothing about where it's defined, so a reviewer has
to search the script by hand to find it.

Find the line a test is defined on by searching the script for its
description as a fixed string, using the first match. Fall back to
line 1 when the description is not found verbatim, which happens when
a test builds its description at runtime instead of writing it out
literally.

A GitHub annotation is a single line, so a `%` in a test description
has to be percent-encoded as `%25`, or GitHub misreads it as its own
escape sequence. for-each-ref's format atoms use plenty of them, e.g.
`%(raw)`.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 t/test-lib-github-workflow-markup.sh | 38 +++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index 0d54496358..6fee4dfb22 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -31,6 +31,22 @@ start_test_output () {
 	github_markup_script_name=${0##*/}
 }
 
+github_escape_message_ () {
+	# % has to be escaped or GitHub misreads it as the start of its own
+	# percent-encoding (e.g. a literal %(raw) in a for-each-ref test
+	# description).
+	sed -e 's/%/%25/g'
+}
+
+find_test_case_line_ () {
+	# A description can contain characters like [ or * that would
+	# corrupt a regex search, so match it literally and take the first
+	# hit. The -- keeps a description starting with "-" from being read
+	# as an option.
+	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
+	head -n 1 | cut -d: -f1
+}
+
 github_annotation_ () {
 	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
 }
@@ -40,18 +56,28 @@ github_annotation_ () {
 finalize_test_case_output () {
 	test_case_result=$1
 	shift
+
+	case "$test_case_result" in
+	ok|broken)
+		# Exit without printing the "ok" or "broken" tests
+		return
+		;;
+	esac
+
+	test_case_line=$(find_test_case_line_ "$1")
+	test_case_description=$(printf '%s' "$1" | github_escape_message_)
+
 	case "$test_case_result" in
 	failure)
-		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
+		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"failed: $this_test.$test_count $test_case_description"
 		;;
 	fixed)
-		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
-		;;
-	ok|broken)
-		# Exit without printing the "ok" or ""broken" tests
-		return
+		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"fixed: $this_test.$test_count $test_case_description"
 		;;
 	esac
+
 	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
 	test-tool >>$github_markup_output path-utils skip-n-bytes \
 		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
-- 
gitgitgadget

Re: [PATCH v4 2/2] ci: point test failures and fixed known breakages at their file and line

[Phillip Wood]

Hi Harald

On 01/10/2026 19:44, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> A test failure or a fixed known breakage gets an annotation that names
> the test but says nothing about where it's defined, so a reviewer has
> to search the script by hand to find it.

I'm afraid I'm still not clear what this does in practical terms. What 
appears in the test output that the user sees that didn't before?

> Find the line a test is defined on by searching the script for its
> description as a fixed string, using the first match. Fall back to
> line 1 when the description is not found verbatim, which happens when
> a test builds its description at runtime instead of writing it out
> literally.
> 
> A GitHub annotation is a single line, so a `%` in a test description
> has to be percent-encoded as `%25`, or GitHub misreads it as its own
> escape sequence. for-each-ref's format atoms use plenty of them, e.g.
> `%(raw)`.

That's a useful example of why we want to escape the output which makes 
it all the more puzzling that we don't escape the existing annotations 
that I mentioned last time.

Thanks

Phillip

> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>   t/test-lib-github-workflow-markup.sh | 38 +++++++++++++++++++++++-----
>   1 file changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
> index 0d54496358..6fee4dfb22 100644
> --- a/t/test-lib-github-workflow-markup.sh
> +++ b/t/test-lib-github-workflow-markup.sh
> @@ -31,6 +31,22 @@ start_test_output () {
>   	github_markup_script_name=${0##*/}
>   }
>   
> +github_escape_message_ () {
> +	# % has to be escaped or GitHub misreads it as the start of its own
> +	# percent-encoding (e.g. a literal %(raw) in a for-each-ref test
> +	# description).
> +	sed -e 's/%/%25/g'
> +}
> +
> +find_test_case_line_ () {
> +	# A description can contain characters like [ or * that would
> +	# corrupt a regex search, so match it literally and take the first
> +	# hit. The -- keeps a description starting with "-" from being read
> +	# as an option.
> +	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
> +	head -n 1 | cut -d: -f1
> +}
> +
>   github_annotation_ () {
>   	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
>   }
> @@ -40,18 +56,28 @@ github_annotation_ () {
>   finalize_test_case_output () {
>   	test_case_result=$1
>   	shift
> +
> +	case "$test_case_result" in
> +	ok|broken)
> +		# Exit without printing the "ok" or "broken" tests
> +		return
> +		;;
> +	esac
> +
> +	test_case_line=$(find_test_case_line_ "$1")
> +	test_case_description=$(printf '%s' "$1" | github_escape_message_)
> +
>   	case "$test_case_result" in
>   	failure)
> -		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
> +		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
> +			"failed: $this_test.$test_count $test_case_description"
>   		;;
>   	fixed)
> -		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
> -		;;
> -	ok|broken)
> -		# Exit without printing the "ok" or ""broken" tests
> -		return
> +		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
> +			"fixed: $this_test.$test_count $test_case_description"
>   		;;
>   	esac
> +
>   	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
>   	test-tool >>$github_markup_output path-utils skip-n-bytes \
>   		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET

Re: [PATCH v4 2/2] ci: point test failures and fixed known breakages at their file and line

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Harald
>
> On 01/10/2026 19:44, Harald Nordgren via GitGitGadget wrote:
>> From: Harald Nordgren <haraldnordgren@gmail.com>
>> 
>> A test failure or a fixed known breakage gets an annotation that names
>> the test but says nothing about where it's defined, so a reviewer has
>> to search the script by hand to find it.
>
> I'm afraid I'm still not clear what this does in practical terms. What 
> appears in the test output that the user sees that didn't before?
>
>> Find the line a test is defined on by searching the script for its
>> description as a fixed string, using the first match. Fall back to
>> line 1 when the description is not found verbatim, which happens when
>> a test builds its description at runtime instead of writing it out
>> literally.

I agree this is still hard to read.  My interpretation of the above
is

    We only say "the t1234 script failed" (in the first paragraph
    that makes an observation of the status quo), and we try to find
    the test_expect_success block and show it as the finer-grained
    clue (the second paragraph).

but that may be way off the mark.

>> A GitHub annotation is a single line, so a `%` in a test description
>> has to be percent-encoded as `%25`, or GitHub misreads it as its own
>> escape sequence. for-each-ref's format atoms use plenty of them, e.g.
>> `%(raw)`.
>
> That's a useful example of why we want to escape the output which makes 
> it all the more puzzling that we don't escape the existing annotations 
> that I mentioned last time.
>
> Thanks
>
> Phillip

Thanks.

Re: [PATCH v4 2/2] ci: point test failures and fixed known breakages at their file and line

[Harald Nordgren]

> > A GitHub annotation is a single line, so a `%` in a test description
> > has to be percent-encoded as `%25`, or GitHub misreads it as its own
> > escape sequence. for-each-ref's format atoms use plenty of them, e.g.
> > `%(raw)`.
>
> That's a useful example of why we want to escape the output which makes
> it all the more puzzling that we don't escape the existing annotations
> that I mentioned last time.

This feels like a rabbit hole and probably better to just drop the
escaping altogether. It seems that the only thing that would need
escaping is the literal '%25', '%' in ASCII, but it doesn't even
appear in any of our tests. See:

- https://github.com/git/git/actions/runs/36976989079/job/110742888375?pr=2435
- https://github.com/git/git/actions/runs/36977041388/job/110743045462?pr=2436

I'll just drop this now. If needed, better to pick it up in a
different topic. Thanks for pursuing this!


Harald

[PATCH v5 0/2] ci: link failure and leak annotations to the test script

[Harald Nordgren via GitGitGadget]

Link failure and leak annotations in CI to the test script, so both can be
found from the job summary.

V5 CI Job where failures and leaks are reported:
https://github.com/git/git/actions/runs/36979818141/job/110752027863?pr=2426

Changes in v5:

 * Removed % escaping entirely, verified on CI that it isn't needed. Every
   existing test description that uses % renders correctly unescaped.
 * Rewrote the file/line commit message with a concrete example (failed:
   t1060.17 partial clone of corrupted repository).

Changes in v4:

 * Clarify commit messages and simplify escaping logic.

Changes in v3:

 * Fixed bug in the --immediate exit ordering: the --immediate &&
   --invert-exit-code path called exit 0 before the test's annotation was
   written, now a single unconditional call covers both exit paths.
 * github_escape_message_ no longer relies on \r being a portable sed escape
   sequence (not POSIX-guaranteed and BSD sed implementations can differ),
   it splices in the literal carriage-return byte via printf instead.
 * Reverted unrelated test-tool line back to its original form.

Changes in v2:

 * Split into two commits, each explaining its own reasoning.
 * Leak output is no longer capped or embedded in the message, it's now an
   uncapped fold, so multiple leaks in the same test both show in full. A
   second leak in a different test still won't show in the same run,
   --immediate stops the script at the first failure, but it no longer gets
   buried under every later test falsely reporting "not ok" either.
 * Drops the giant unfolded message that annotations used to carry, which is
   what probably caused the scrolling behavior.

Harald Nordgren (2):
  ci: annotate leaks and stop a leak-sanitizer script at its first
    failure
  ci: point test failures and fixed known breakages at their file and
    line

 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 46 ++++++++++++++++++++++++----
 t/test-lib.sh                        |  6 +++-
 3 files changed, 46 insertions(+), 7 deletions(-)


base-commit: c46c1e37724f0478939de636ab8ea5a89086d532
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v5
Pull-Request: https://github.com/git/git/pull/2419

Range-diff vs v4:

 1:  138394b48b = 1:  851efeec8b ci: annotate leaks and stop a leak-sanitizer script at its first failure
 2:  8ec2b53d82 ! 2:  46f93a9e16 ci: point test failures and fixed known breakages at their file and line
     @@ Metadata
       ## Commit message ##
          ci: point test failures and fixed known breakages at their file and line
      
     -    A test failure or a fixed known breakage gets an annotation that names
     -    the test but says nothing about where it's defined, so a reviewer has
     -    to search the script by hand to find it.
     +    When a test fails, GitHub shows an annotation naming it, for example:
      
     -    Find the line a test is defined on by searching the script for its
     -    description as a fixed string, using the first match. Fall back to
     +        failed: t1060.17 partial clone of corrupted repository
     +
     +    but the location GitHub attaches to that annotation is the CI
     +    workflow file itself, not the test script, so there is nothing
     +    pointing at where the test actually lives.
     +
     +    Find the line a test is defined on by searching its script for the
     +    test's own description as a fixed string, using the first match, and
     +    attach that file and line to the annotation instead. Fall back to
          line 1 when the description is not found verbatim, which happens when
          a test builds its description at runtime instead of writing it out
          literally.
      
     -    A GitHub annotation is a single line, so a `%` in a test description
     -    has to be percent-encoded as `%25`, or GitHub misreads it as its own
     -    escape sequence. for-each-ref's format atoms use plenty of them, e.g.
     -    `%(raw)`.
     -
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## t/test-lib-github-workflow-markup.sh ##
     @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
       	github_markup_script_name=${0##*/}
       }
       
     -+github_escape_message_ () {
     -+	# % has to be escaped or GitHub misreads it as the start of its own
     -+	# percent-encoding (e.g. a literal %(raw) in a for-each-ref test
     -+	# description).
     -+	sed -e 's/%/%25/g'
     -+}
     -+
      +find_test_case_line_ () {
      +	# A description can contain characters like [ or * that would
      +	# corrupt a regex search, so match it literally and take the first
     @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
      +	esac
      +
      +	test_case_line=$(find_test_case_line_ "$1")
     -+	test_case_description=$(printf '%s' "$1" | github_escape_message_)
      +
       	case "$test_case_result" in
       	failure)
      -		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
      +		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
     -+			"failed: $this_test.$test_count $test_case_description"
     ++			"failed: $this_test.$test_count $1"
       		;;
       	fixed)
      -		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
     @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
      -		# Exit without printing the "ok" or ""broken" tests
      -		return
      +		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
     -+			"fixed: $this_test.$test_count $test_case_description"
     ++			"fixed: $this_test.$test_count $1"
       		;;
       	esac
      +

-- 
gitgitgadget

[PATCH v5 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A leak is only discovered once, at the end of a whole script, well
after every test has already reported ok, and it gets no annotation at
all, so a leak-sanitizer job's only visible failure is:

    Process completed with exit code 1.

Give a leak its own annotation. Point it at the test script, the exact
line isn't known, only which script the leak turned up in, and put the
sanitizer report in a log group next to it, so it stays visible.

Once a script has one leak, it keeps running: the sanitizer log
directory is never cleared between tests, so every later test in the
same script sees the same leftover log entries and also reports "not
ok", burying the one real failure in copies of itself. Stop a
leak-sanitizer script at its first failure with --immediate instead.

A failing test already gets its own annotation once its script
finishes, but --immediate exits as soon as that test fails, before
reaching the code that writes it. Write the annotation first, so
turning on --immediate here does not silently drop it.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 16 ++++++++++++++++
 t/test-lib.sh                        |  6 +++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index c6ccbf8c17..a89f480a78 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)
 	export NO_CVS_TESTS=LetsSaveSomeTime
 	export NO_SVN_TESTS=LetsSaveSomeTime
 	export NO_P4_TESTS=LetsSaveSomeTime
+	GIT_TEST_OPTS="$GIT_TEST_OPTS --immediate"
 	;;
 linux-asan-ubsan)
 	export SANITIZE=address,undefined
diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index fa29a62aa3..0d54496358 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -28,6 +28,11 @@ start_test_output () {
 	github_markup_output="${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup"
 	>$github_markup_output
 	GIT_TEST_TEE_OFFSET=0
+	github_markup_script_name=${0##*/}
+}
+
+github_annotation_ () {
+	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
 }
 
 # No need to override start_test_case_output
@@ -53,4 +58,15 @@ finalize_test_case_output () {
 	echo >>$github_markup_output "::endgroup::"
 }
 
+finalize_test_leak_output () {
+	# The exact line the leak turned up on isn't known, only the script,
+	# so point at line 1.
+	github_annotation_ error "t/$github_markup_script_name" 1 \
+		"memory leak logged in $this_test"
+
+	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
+	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
+	echo >>$github_markup_output "::endgroup::"
+}
+
 # No need to override finalize_test_output
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 321c2ba339..a893963f86 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -199,6 +199,7 @@ mark_option_requires_arg () {
 start_test_output () { :; }
 start_test_case_output () { :; }
 finalize_test_case_output () { :; }
+finalize_test_leak_output () { :; }
 finalize_test_output () { :; }
 
 parse_option () {
@@ -822,6 +823,9 @@ test_failure_ () {
 	say_color error "not ok $test_count - ${pfx:+$pfx }$1"
 	shift
 	printf '%s\n' "$*" | sed -e 's/^/#	/'
+	# Write the annotation before either --immediate exit path below,
+	# both of which call exit and would otherwise skip it.
+	finalize_test_case_output failure "$failure_label" "$@"
 	if test -n "$immediate"
 	then
 		say_color error "1..$test_count"
@@ -835,7 +839,6 @@ test_failure_ () {
 		check_test_results_san_file_ "$test_failure"
 		_error_exit
 	fi
-	finalize_test_case_output failure "$failure_label" "$@"
 }
 
 test_known_broken_ok_ () {
@@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {
 		return
 	fi &&
 	say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
+	finalize_test_leak_output &&
 
 	if test "$test_failure" = 0
 	then
-- 
gitgitgadget

[PATCH v5 2/2] ci: point test failures and fixed known breakages at their file and line

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

When a test fails, GitHub shows an annotation naming it, for example:

    failed: t1060.17 partial clone of corrupted repository

but the location GitHub attaches to that annotation is the CI
workflow file itself, not the test script, so there is nothing
pointing at where the test actually lives.

Find the line a test is defined on by searching its script for the
test's own description as a fixed string, using the first match, and
attach that file and line to the annotation instead. Fall back to
line 1 when the description is not found verbatim, which happens when
a test builds its description at runtime instead of writing it out
literally.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 t/test-lib-github-workflow-markup.sh | 30 ++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index 0d54496358..ac8c536231 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -31,6 +31,15 @@ start_test_output () {
 	github_markup_script_name=${0##*/}
 }
 
+find_test_case_line_ () {
+	# A description can contain characters like [ or * that would
+	# corrupt a regex search, so match it literally and take the first
+	# hit. The -- keeps a description starting with "-" from being read
+	# as an option.
+	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
+	head -n 1 | cut -d: -f1
+}
+
 github_annotation_ () {
 	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
 }
@@ -40,18 +49,27 @@ github_annotation_ () {
 finalize_test_case_output () {
 	test_case_result=$1
 	shift
+
+	case "$test_case_result" in
+	ok|broken)
+		# Exit without printing the "ok" or "broken" tests
+		return
+		;;
+	esac
+
+	test_case_line=$(find_test_case_line_ "$1")
+
 	case "$test_case_result" in
 	failure)
-		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
+		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"failed: $this_test.$test_count $1"
 		;;
 	fixed)
-		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
-		;;
-	ok|broken)
-		# Exit without printing the "ok" or ""broken" tests
-		return
+		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
+			"fixed: $this_test.$test_count $1"
 		;;
 	esac
+
 	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
 	test-tool >>$github_markup_output path-utils skip-n-bytes \
 		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
-- 
gitgitgadget

Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script

[Phillip Wood]

Hi Harald

On 03/10/2026 09:11, Harald Nordgren via GitGitGadget wrote:
> Link failure and leak annotations in CI to the test script, so both can be
> found from the job summary.
> 
> V5 CI Job where failures and leaks are reported:
> https://github.com/git/git/actions/runs/36979818141/job/110752027863?pr=2426

There does not appear to be any output relating to leaks in that job. 
The first patch hasn't changed so I'm not sure why that is.

> Changes in v5:
> 
>   * Removed % escaping entirely, verified on CI that it isn't needed. Every
>     existing test description that uses % renders correctly unescaped.
>   * Rewrote the file/line commit message with a concrete example (failed:
>     t1060.17 partial clone of corrupted repository).

You have added


     When a test fails, GitHub shows an annotation naming it, for
     example:

         failed: t1060.17 partial clone of corrupted repository

which shows an example of the current output without the filename or 
line annotations. There is no example of what that output changes to, so 
there is no way for someone reading that message to see what has 
actually changed. After spending some time clicking around in Github I 
think what that patch changes is not the test output of individual jobs 
which you linked to above, but what is displayed on the summary page at

https://github.com/git/git/actions/runs/36979818141?pr=2426

That page shows a list of annotations with links to the changes in the 
failed test file. That is a useful improvement but how you expected 
someone reading the commit message to understand what had changed when 
you did not give an example of the new output, and the changes are on a 
different page to the one you linked to in the cover letter is beyond 
me. I'm pretty exasperated that I've had to spend time messing about on 
Github trying to see what has changed because you could not provide a 
link and write a couple of sentences explaining it. After asking what 
this change did in v3 you replied that the commit message wasn't clear 
without explaining what the change actually did. When I asked what the 
change did in practical terms in response to v4 I got no reply. As you 
already know reviewer time is short on this list, so please, when 
someone asks a question answer it rather than replying with an obtuse 
comment or simply ignoring it and sending another patch.

Both these patches are useful improvements, but trying to get an 
explanation of what they did has been like trying getting blood out of a 
stone.

Thanks

Phillip


> Changes in v4:
> 
>   * Clarify commit messages and simplify escaping logic.
> 
> Changes in v3:
> 
>   * Fixed bug in the --immediate exit ordering: the --immediate &&
>     --invert-exit-code path called exit 0 before the test's annotation was
>     written, now a single unconditional call covers both exit paths.
>   * github_escape_message_ no longer relies on \r being a portable sed escape
>     sequence (not POSIX-guaranteed and BSD sed implementations can differ),
>     it splices in the literal carriage-return byte via printf instead.
>   * Reverted unrelated test-tool line back to its original form.
> 
> Changes in v2:
> 
>   * Split into two commits, each explaining its own reasoning.
>   * Leak output is no longer capped or embedded in the message, it's now an
>     uncapped fold, so multiple leaks in the same test both show in full. A
>     second leak in a different test still won't show in the same run,
>     --immediate stops the script at the first failure, but it no longer gets
>     buried under every later test falsely reporting "not ok" either.
>   * Drops the giant unfolded message that annotations used to carry, which is
>     what probably caused the scrolling behavior.
> 
> Harald Nordgren (2):
>    ci: annotate leaks and stop a leak-sanitizer script at its first
>      failure
>    ci: point test failures and fixed known breakages at their file and
>      line
> 
>   ci/lib.sh                            |  1 +
>   t/test-lib-github-workflow-markup.sh | 46 ++++++++++++++++++++++++----
>   t/test-lib.sh                        |  6 +++-
>   3 files changed, 46 insertions(+), 7 deletions(-)
> 
> 
> base-commit: c46c1e37724f0478939de636ab8ea5a89086d532
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v5
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v5
> Pull-Request: https://github.com/git/git/pull/2419
> 
> Range-diff vs v4:
> 
>   1:  138394b48b = 1:  851efeec8b ci: annotate leaks and stop a leak-sanitizer script at its first failure
>   2:  8ec2b53d82 ! 2:  46f93a9e16 ci: point test failures and fixed known breakages at their file and line
>       @@ Metadata
>         ## Commit message ##
>            ci: point test failures and fixed known breakages at their file and line
>        
>       -    A test failure or a fixed known breakage gets an annotation that names
>       -    the test but says nothing about where it's defined, so a reviewer has
>       -    to search the script by hand to find it.
>       +    When a test fails, GitHub shows an annotation naming it, for example:
>        
>       -    Find the line a test is defined on by searching the script for its
>       -    description as a fixed string, using the first match. Fall back to
>       +        failed: t1060.17 partial clone of corrupted repository
>       +
>       +    but the location GitHub attaches to that annotation is the CI
>       +    workflow file itself, not the test script, so there is nothing
>       +    pointing at where the test actually lives.
>       +
>       +    Find the line a test is defined on by searching its script for the
>       +    test's own description as a fixed string, using the first match, and
>       +    attach that file and line to the annotation instead. Fall back to
>            line 1 when the description is not found verbatim, which happens when
>            a test builds its description at runtime instead of writing it out
>            literally.
>        
>       -    A GitHub annotation is a single line, so a `%` in a test description
>       -    has to be percent-encoded as `%25`, or GitHub misreads it as its own
>       -    escape sequence. for-each-ref's format atoms use plenty of them, e.g.
>       -    `%(raw)`.
>       -
>            Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>        
>         ## t/test-lib-github-workflow-markup.sh ##
>       @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
>         	github_markup_script_name=${0##*/}
>         }
>         
>       -+github_escape_message_ () {
>       -+	# % has to be escaped or GitHub misreads it as the start of its own
>       -+	# percent-encoding (e.g. a literal %(raw) in a for-each-ref test
>       -+	# description).
>       -+	sed -e 's/%/%25/g'
>       -+}
>       -+
>        +find_test_case_line_ () {
>        +	# A description can contain characters like [ or * that would
>        +	# corrupt a regex search, so match it literally and take the first
>       @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
>        +	esac
>        +
>        +	test_case_line=$(find_test_case_line_ "$1")
>       -+	test_case_description=$(printf '%s' "$1" | github_escape_message_)
>        +
>         	case "$test_case_result" in
>         	failure)
>        -		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
>        +		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
>       -+			"failed: $this_test.$test_count $test_case_description"
>       ++			"failed: $this_test.$test_count $1"
>         		;;
>         	fixed)
>        -		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
>       @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
>        -		# Exit without printing the "ok" or ""broken" tests
>        -		return
>        +		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
>       -+			"fixed: $this_test.$test_count $test_case_description"
>       ++			"fixed: $this_test.$test_count $1"
>         		;;
>         	esac
>        +
> 

Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script

[Phillip Wood]

On 03/10/2026 14:03, Phillip Wood wrote:
> After spending some time clicking around in Github I 
> think what that patch changes is not the test output of individual jobs 
> which you linked to above, but what is displayed on the summary page at
> 
> https://github.com/git/git/actions/runs/36979818141?pr=2426
> 
> That page shows a list of annotations with links to the changes in the 
> failed test file. That is a useful improvement

But it seems it is only useful if the test changed is in that example. 
If I look at the summary for the CI run from v3 of this series [1] then 
I can see a test failure in t1022

     linux-leaks(ubuntu-rolling): t/t1022-read-tree-partial-clone.sh#L8
     failed: t1022.1 read-tree in partial clone prefetches in one batch

If I click on the link [2] it does not take me to that test file though, 
because it was not changed. That makes this somewhat less useful than I 
initially thought. The current behavior is that when you click on those 
links in the summary page it takes you to the test output for the job 
that failed which seems more useful. For example [3] is recent test run 
that had a leak and clicking on

     linux-leaks:(ubuntu-rolling):
     failed: t1092.58 submodule handling

Takes me to [4] which where I can click to expand the output of the 
failing test.

Thanks

Phillip

[1] https://github.com/git/git/actions/runs/36537917146?pr=2426
[2] https://github.com/git/git/pull/2426/files#annotation_82189987516
[3] https://github.com/benknoble/git/actions/runs/36033463504
[4] 
https://github.com/benknoble/git/actions/runs/36033463504/job/107747745741#step:9:5333

Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script

[Harald Nordgren]

On Sat, Oct 3, 2026 at 9:02 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 03/10/2026 14:03, Phillip Wood wrote:
> > After spending some time clicking around in Github I
> > think what that patch changes is not the test output of individual jobs
> > which you linked to above, but what is displayed on the summary page at
> >
> > https://github.com/git/git/actions/runs/36979818141?pr=2426
> >
> > That page shows a list of annotations with links to the changes in the
> > failed test file. That is a useful improvement
>
> But it seems it is only useful if the test changed is in that example.
> If I look at the summary for the CI run from v3 of this series [1] then
> I can see a test failure in t1022
>
>      linux-leaks(ubuntu-rolling): t/t1022-read-tree-partial-clone.sh#L8
>      failed: t1022.1 read-tree in partial clone prefetches in one batch
>
> If I click on the link [2] it does not take me to that test file though,
> because it was not changed.

Yes, unfortunately GitHub won't let us link to a line that was not
changed in that PR.

> That makes this somewhat less useful than I
> initially thought. The current behavior is that when you click on those
> links in the summary page it takes you to the test output for the job
> that failed which seems more useful. For example [3] is recent test run
> that had a leak and clicking on
>
>      linux-leaks:(ubuntu-rolling):
>      failed: t1092.58 submodule handling
>
> Takes me to [4] which where I can click to expand the output of the
> failing test.
> ...
> [1] https://github.com/git/git/actions/runs/36537917146?pr=2426
> [2] https://github.com/git/git/pull/2426/files#annotation_82189987516
> [3] https://github.com/benknoble/git/actions/runs/36033463504
> [4]
> https://github.com/benknoble/git/actions/runs/36033463504/job/107747745741#step:9:5333

Is this enough to call this a regression? Then maybe it's not worth
doing this part at all.


Harald

Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script

[Phillip Wood]

Hi Harald

On 04/10/2026 12:51, Harald Nordgren wrote:
> On Sat, Oct 3, 2026 at 9:02 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> If I click on the link [2] it does not take me to that test file though,
>> because it was not changed.
> 
> Yes, unfortunately GitHub won't let us link to a line that was not
> changed in that PR.
> 
>> That makes this somewhat less useful than I
>> initially thought. The current behavior is that when you click on those
>> links in the summary page it takes you to the test output for the job
>> that failed which seems more useful. For example [3] is recent test run
>> that had a leak and clicking on
>>
>>       linux-leaks:(ubuntu-rolling):
>>       failed: t1092.58 submodule handling
>>
>> Takes me to [4] which where I can click to expand the output of the
>> failing test.
>> ...
>> [1] https://github.com/git/git/actions/runs/36537917146?pr=2426
>> [2] https://github.com/git/git/pull/2426/files#annotation_82189987516
>> [3] https://github.com/benknoble/git/actions/runs/36033463504
>> [4]
>> https://github.com/benknoble/git/actions/runs/36033463504/job/107747745741#step:9:5333
> 
> Is this enough to call this a regression? Then maybe it's not worth
> doing this part at all.

Yes, I think we should drop this patch. The first step to debugging a 
test failure is to look at the test output, so the current behavior 
where clicking on the links on the summary page takes you to the test 
output is more useful than taking you to a diff that may not even show 
the test that failed. The first patch is definitely worth keeping as it 
makes it much easier to see the LSAN output.

Thanks

Phillip

Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script

[Harald Nordgren]

> > Is this enough to call this a regression? Then maybe it's not worth
> > doing this part at all.
>
> Yes, I think we should drop this patch. The first step to debugging a
> test failure is to look at the test output, so the current behavior
> where clicking on the links on the summary page takes you to the test
> output is more useful than taking you to a diff that may not even show
> the test that failed. The first patch is definitely worth keeping as it
> makes it much easier to see the LSAN output.

I played with instead showing the file name (and line when available)
as part of the annotation text, and leaving the linking as it is. I
think it could gives us the best of both worlds:

    memory leak logged in t1060 (t1060-object-corruption.sh)

and

    failed: t1060.17 partial clone of corrupted repository
(t1060-object-corruption.sh:141)

What do you think?


Harald

Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script

[Phillip Wood]

On 05/10/2026 14:59, Harald Nordgren wrote:
>>> Is this enough to call this a regression? Then maybe it's not worth
>>> doing this part at all.
>>
>> Yes, I think we should drop this patch. The first step to debugging a
>> test failure is to look at the test output, so the current behavior
>> where clicking on the links on the summary page takes you to the test
>> output is more useful than taking you to a diff that may not even show
>> the test that failed. The first patch is definitely worth keeping as it
>> makes it much easier to see the LSAN output.
> 
> I played with instead showing the file name (and line when available)
> as part of the annotation text, and leaving the linking as it is. I
> think it could gives us the best of both worlds:
> 
>      memory leak logged in t1060 (t1060-object-corruption.sh)
> 
> and
> 
>      failed: t1060.17 partial clone of corrupted repository
> (t1060-object-corruption.sh:141)
> 
> What do you think?

I guess having a bit more detail could be useful, I certainly don't object.

Thanks

Phillip

Re: [PATCH v5 0/2] ci: link failure and leak annotations to the test script

[Harald Nordgren]

On Sat, Oct 3, 2026 at 3:03 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Harald
>
> On 03/10/2026 09:11, Harald Nordgren via GitGitGadget wrote:
> > Link failure and leak annotations in CI to the test script, so both can be
> > found from the job summary.
> >
> > V5 CI Job where failures and leaks are reported:
> > https://github.com/git/git/actions/runs/36979818141/job/110752027863?pr=2426
>
> There does not appear to be any output relating to leaks in that job.
> The first patch hasn't changed so I'm not sure why that is.
>
> > Changes in v5:
> >
> >   * Removed % escaping entirely, verified on CI that it isn't needed. Every
> >     existing test description that uses % renders correctly unescaped.
> >   * Rewrote the file/line commit message with a concrete example (failed:
> >     t1060.17 partial clone of corrupted repository).
>
> You have added
>
>
>      When a test fails, GitHub shows an annotation naming it, for
>      example:
>
>          failed: t1060.17 partial clone of corrupted repository
>
> which shows an example of the current output without the filename or
> line annotations. There is no example of what that output changes to, so
> there is no way for someone reading that message to see what has
> actually changed. After spending some time clicking around in Github I
> think what that patch changes is not the test output of individual jobs
> which you linked to above, but what is displayed on the summary page at
>
> https://github.com/git/git/actions/runs/36979818141?pr=2426
>
> That page shows a list of annotations with links to the changes in the
> failed test file. That is a useful improvement but how you expected
> someone reading the commit message to understand what had changed when
> you did not give an example of the new output, and the changes are on a
> different page to the one you linked to in the cover letter is beyond
> me. I'm pretty exasperated that I've had to spend time messing about on
> Github trying to see what has changed because you could not provide a
> link and write a couple of sentences explaining it. After asking what
> this change did in v3 you replied that the commit message wasn't clear
> without explaining what the change actually did. When I asked what the
> change did in practical terms in response to v4 I got no reply. As you
> already know reviewer time is short on this list, so please, when
> someone asks a question answer it rather than replying with an obtuse
> comment or simply ignoring it and sending another patch.
>
> Both these patches are useful improvements, but trying to get an
> explanation of what they did has been like trying getting blood out of a
> stone.

I don't understand why this tone is necessary at all.

Yes, I should clarify that it affects the summary.

But I posted a comment regarding the regression you brought up in your
following message, so we should decide what we want before
progressing. The point of this whole topic was to make the leak
reporting less bad, everything else was a bonus.


Harald

[PATCH v6 0/2] ci: link failure and leak annotations to the test script

[Harald Nordgren via GitGitGadget]

Link failure and leak annotations in CI to the test script, so both can be
found from the job summary.

V7 CI job where failures and leaks are reported:

 * https://github.com/git/git/actions/runs/37205086805?pr=2426

Changes in v7:

 * Revert the linking in annotation message, direct direct line fails when
   error pointed to an unchanged file in that PR, which regresses the
   experiences in many cases. Instead now show the file and line as plain
   text in the annotation.
 * Don't name the the line number as 1 for leaks, where it's never
   available, just omit the line number.

Changes in v6:

 * Update commit message.

Changes in v5:

 * Removed % escaping entirely, verified on CI that it isn't needed. Every
   existing test description that uses % renders correctly unescaped.
 * Rewrote the file/line commit message with a concrete example (failed:
   t1060.17 partial clone of corrupted repository).

Changes in v4:

 * Clarify commit messages and simplify escaping logic.

Changes in v3:

 * Fixed bug in the --immediate exit ordering: the --immediate &&
   --invert-exit-code path called exit 0 before the test's annotation was
   written, now a single unconditional call covers both exit paths.
 * github_escape_message_ no longer relies on \r being a portable sed escape
   sequence (not POSIX-guaranteed and BSD sed implementations can differ),
   it splices in the literal carriage-return byte via printf instead.
 * Reverted unrelated test-tool line back to its original form.

Changes in v2:

 * Split into two commits, each explaining its own reasoning.
 * Leak output is no longer capped or embedded in the message, it's now an
   uncapped fold, so multiple leaks in the same test both show in full. A
   second leak in a different test still won't show in the same run,
   --immediate stops the script at the first failure, but it no longer gets
   buried under every later test falsely reporting "not ok" either.
 * Drops the giant unfolded message that annotations used to carry, which is
   what probably caused the scrolling behavior.

Harald Nordgren (2):
  ci: annotate leaks and stop a leak-sanitizer script at its first
    failure
  ci: point test failures and fixed known breakages at their file and
    line

 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 39 +++++++++++++++++++++++-----
 t/test-lib.sh                        |  6 ++++-
 3 files changed, 39 insertions(+), 7 deletions(-)


base-commit: 8103b446517e0c44e67561b9d0ccce56efa60a71
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v6
Pull-Request: https://github.com/git/git/pull/2419

Range-diff vs v5:

 1:  851efeec8b ! 1:  917f373f91 ci: annotate leaks and stop a leak-sanitizer script at its first failure
     @@ Commit message
      
              Process completed with exit code 1.
      
     -    Give a leak its own annotation. Point it at the test script, the exact
     -    line isn't known, only which script the leak turned up in, and put the
     -    sanitizer report in a log group next to it, so it stays visible.
     +    Give a leak its own annotation, naming the script it turned up in, the
     +    exact line isn't known, only which script:
     +
     +        memory leak logged in t1060 (t1060-object-corruption.sh)
     +
     +    Put the sanitizer report in a log group next to it, so it stays
     +    visible.
      
          Once a script has one leak, it keeps running: the sanitizer log
          directory is never cleared between tests, so every later test in the
     @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
       	>$github_markup_output
       	GIT_TEST_TEE_OFFSET=0
      +	github_markup_script_name=${0##*/}
     -+}
     -+
     -+github_annotation_ () {
     -+	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
       }
       
       # No need to override start_test_case_output
     @@ t/test-lib-github-workflow-markup.sh: finalize_test_case_output () {
       }
       
      +finalize_test_leak_output () {
     -+	# The exact line the leak turned up on isn't known, only the script,
     -+	# so point at line 1.
     -+	github_annotation_ error "t/$github_markup_script_name" 1 \
     -+		"memory leak logged in $this_test"
     ++	echo >>$github_markup_output \
     ++		"::error::memory leak logged in $this_test ($github_markup_script_name)"
      +
      +	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
      +	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
 2:  46f93a9e16 ! 2:  acf1fbd250 ci: point test failures and fixed known breakages at their file and line
     @@ Metadata
       ## Commit message ##
          ci: point test failures and fixed known breakages at their file and line
      
     -    When a test fails, GitHub shows an annotation naming it, for example:
     +    A failing test gets an annotation in the Annotations list on its job's
     +    summary page, naming it, for example:
      
              failed: t1060.17 partial clone of corrupted repository
      
     -    but the location GitHub attaches to that annotation is the CI
     -    workflow file itself, not the test script, so there is nothing
     -    pointing at where the test actually lives.
     +    with no indication of where that test lives.
      
          Find the line a test is defined on by searching its script for the
          test's own description as a fixed string, using the first match, and
     -    attach that file and line to the annotation instead. Fall back to
     -    line 1 when the description is not found verbatim, which happens when
     -    a test builds its description at runtime instead of writing it out
     -    literally.
     +    add the file and line to the annotation's own message text:
     +
     +        failed: t1060.17 partial clone of corrupted repository (t1060-object-corruption.sh:141)
     +
     +    Fall back to naming just the script, with no line, when the
     +    description is not found verbatim, which happens when a test builds
     +    its description at runtime instead of writing it out literally.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
      +	head -n 1 | cut -d: -f1
      +}
      +
     - github_annotation_ () {
     - 	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
     - }
     -@@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
     + # No need to override start_test_case_output
     + 
       finalize_test_case_output () {
       	test_case_result=$1
       	shift
     @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
      +	esac
      +
      +	test_case_line=$(find_test_case_line_ "$1")
     ++	test_case_where="$github_markup_script_name${test_case_line:+:$test_case_line}"
      +
       	case "$test_case_result" in
       	failure)
      -		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
     -+		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
     -+			"failed: $this_test.$test_count $1"
     ++		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1 ($test_case_where)"
       		;;
       	fixed)
      -		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
     @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
      -	ok|broken)
      -		# Exit without printing the "ok" or ""broken" tests
      -		return
     -+		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
     -+			"fixed: $this_test.$test_count $1"
     ++		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1 ($test_case_where)"
       		;;
       	esac
      +

-- 
gitgitgadget

[PATCH v6 1/2] ci: annotate leaks and stop a leak-sanitizer script at its first failure

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A leak is only discovered once, at the end of a whole script, well
after every test has already reported ok, and it gets no annotation at
all, so a leak-sanitizer job's only visible failure is:

    Process completed with exit code 1.

Give a leak its own annotation, naming the script it turned up in, the
exact line isn't known, only which script:

    memory leak logged in t1060 (t1060-object-corruption.sh)

Put the sanitizer report in a log group next to it, so it stays
visible.

Once a script has one leak, it keeps running: the sanitizer log
directory is never cleared between tests, so every later test in the
same script sees the same leftover log entries and also reports "not
ok", burying the one real failure in copies of itself. Stop a
leak-sanitizer script at its first failure with --immediate instead.

A failing test already gets its own annotation once its script
finishes, but --immediate exits as soon as that test fails, before
reaching the code that writes it. Write the annotation first, so
turning on --immediate here does not silently drop it.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 ci/lib.sh                            |  1 +
 t/test-lib-github-workflow-markup.sh | 10 ++++++++++
 t/test-lib.sh                        |  6 +++++-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index c6ccbf8c17..a89f480a78 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -382,6 +382,7 @@ linux-leaks|linux-reftable-leaks)
 	export NO_CVS_TESTS=LetsSaveSomeTime
 	export NO_SVN_TESTS=LetsSaveSomeTime
 	export NO_P4_TESTS=LetsSaveSomeTime
+	GIT_TEST_OPTS="$GIT_TEST_OPTS --immediate"
 	;;
 linux-asan-ubsan)
 	export SANITIZE=address,undefined
diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index fa29a62aa3..3fa7859f0b 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -28,6 +28,7 @@ start_test_output () {
 	github_markup_output="${GIT_TEST_TEE_OUTPUT_FILE%.out}.markup"
 	>$github_markup_output
 	GIT_TEST_TEE_OFFSET=0
+	github_markup_script_name=${0##*/}
 }
 
 # No need to override start_test_case_output
@@ -53,4 +54,13 @@ finalize_test_case_output () {
 	echo >>$github_markup_output "::endgroup::"
 }
 
+finalize_test_leak_output () {
+	echo >>$github_markup_output \
+		"::error::memory leak logged in $this_test ($github_markup_script_name)"
+
+	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
+	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
+	echo >>$github_markup_output "::endgroup::"
+}
+
 # No need to override finalize_test_output
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 321c2ba339..a893963f86 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -199,6 +199,7 @@ mark_option_requires_arg () {
 start_test_output () { :; }
 start_test_case_output () { :; }
 finalize_test_case_output () { :; }
+finalize_test_leak_output () { :; }
 finalize_test_output () { :; }
 
 parse_option () {
@@ -822,6 +823,9 @@ test_failure_ () {
 	say_color error "not ok $test_count - ${pfx:+$pfx }$1"
 	shift
 	printf '%s\n' "$*" | sed -e 's/^/#	/'
+	# Write the annotation before either --immediate exit path below,
+	# both of which call exit and would otherwise skip it.
+	finalize_test_case_output failure "$failure_label" "$@"
 	if test -n "$immediate"
 	then
 		say_color error "1..$test_count"
@@ -835,7 +839,6 @@ test_failure_ () {
 		check_test_results_san_file_ "$test_failure"
 		_error_exit
 	fi
-	finalize_test_case_output failure "$failure_label" "$@"
 }
 
 test_known_broken_ok_ () {
@@ -1218,6 +1221,7 @@ check_test_results_san_file_ () {
 		return
 	fi &&
 	say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
+	finalize_test_leak_output &&
 
 	if test "$test_failure" = 0
 	then
-- 
gitgitgadget

[PATCH v6 2/2] ci: point test failures and fixed known breakages at their file and line

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

A failing test gets an annotation in the Annotations list on its job's
summary page, naming it, for example:

    failed: t1060.17 partial clone of corrupted repository

with no indication of where that test lives.

Find the line a test is defined on by searching its script for the
test's own description as a fixed string, using the first match, and
add the file and line to the annotation's own message text:

    failed: t1060.17 partial clone of corrupted repository (t1060-object-corruption.sh:141)

Fall back to naming just the script, with no line, when the
description is not found verbatim, which happens when a test builds
its description at runtime instead of writing it out literally.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 t/test-lib-github-workflow-markup.sh | 29 ++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index 3fa7859f0b..826c4ac902 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -31,23 +31,40 @@ start_test_output () {
 	github_markup_script_name=${0##*/}
 }
 
+find_test_case_line_ () {
+	# A description can contain characters like [ or * that would
+	# corrupt a regex search, so match it literally and take the first
+	# hit. The -- keeps a description starting with "-" from being read
+	# as an option.
+	grep -n -F -- "$1" "$TEST_DIRECTORY/$github_markup_script_name" |
+	head -n 1 | cut -d: -f1
+}
+
 # No need to override start_test_case_output
 
 finalize_test_case_output () {
 	test_case_result=$1
 	shift
+
+	case "$test_case_result" in
+	ok|broken)
+		# Exit without printing the "ok" or "broken" tests
+		return
+		;;
+	esac
+
+	test_case_line=$(find_test_case_line_ "$1")
+	test_case_where="$github_markup_script_name${test_case_line:+:$test_case_line}"
+
 	case "$test_case_result" in
 	failure)
-		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
+		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1 ($test_case_where)"
 		;;
 	fixed)
-		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
-		;;
-	ok|broken)
-		# Exit without printing the "ok" or ""broken" tests
-		return
+		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1 ($test_case_where)"
 		;;
 	esac
+
 	echo >>$github_markup_output "::group::$test_case_result: $this_test.$test_count $*"
 	test-tool >>$github_markup_output path-utils skip-n-bytes \
 		"$GIT_TEST_TEE_OUTPUT_FILE" $GIT_TEST_TEE_OFFSET
-- 
gitgitgadget

Re: [PATCH v6 0/2] ci: link failure and leak annotations to the test script

[Phillip Wood]

Hi Harald

On 06/10/2026 07:56, Harald Nordgren via GitGitGadget wrote:
> Link failure and leak annotations in CI to the test script, so both can be
> found from the job summary.
> 
> V7 CI job where failures and leaks are reported:
> 
>   * https://github.com/git/git/actions/runs/37205086805?pr=2426
> 
> Changes in v7:
> 
>   * Revert the linking in annotation message, direct direct line fails when
>     error pointed to an unchanged file in that PR, which regresses the
>     experiences in many cases. Instead now show the file and line as plain
>     text in the annotation.
>   * Don't name the the line number as 1 for leaks, where it's never
>     available, just omit the line number.

This has been sent as v6, but the range-diff looks good and matches the 
changes listed here. Lets get this merged!

Thanks

Phillip

> Changes in v6:
> 
>   * Update commit message.
> 
> Changes in v5:
> 
>   * Removed % escaping entirely, verified on CI that it isn't needed. Every
>     existing test description that uses % renders correctly unescaped.
>   * Rewrote the file/line commit message with a concrete example (failed:
>     t1060.17 partial clone of corrupted repository).
> 
> Changes in v4:
> 
>   * Clarify commit messages and simplify escaping logic.
> 
> Changes in v3:
> 
>   * Fixed bug in the --immediate exit ordering: the --immediate &&
>     --invert-exit-code path called exit 0 before the test's annotation was
>     written, now a single unconditional call covers both exit paths.
>   * github_escape_message_ no longer relies on \r being a portable sed escape
>     sequence (not POSIX-guaranteed and BSD sed implementations can differ),
>     it splices in the literal carriage-return byte via printf instead.
>   * Reverted unrelated test-tool line back to its original form.
> 
> Changes in v2:
> 
>   * Split into two commits, each explaining its own reasoning.
>   * Leak output is no longer capped or embedded in the message, it's now an
>     uncapped fold, so multiple leaks in the same test both show in full. A
>     second leak in a different test still won't show in the same run,
>     --immediate stops the script at the first failure, but it no longer gets
>     buried under every later test falsely reporting "not ok" either.
>   * Drops the giant unfolded message that annotations used to carry, which is
>     what probably caused the scrolling behavior.
> 
> Harald Nordgren (2):
>    ci: annotate leaks and stop a leak-sanitizer script at its first
>      failure
>    ci: point test failures and fixed known breakages at their file and
>      line
> 
>   ci/lib.sh                            |  1 +
>   t/test-lib-github-workflow-markup.sh | 39 +++++++++++++++++++++++-----
>   t/test-lib.sh                        |  6 ++++-
>   3 files changed, 39 insertions(+), 7 deletions(-)
> 
> 
> base-commit: 8103b446517e0c44e67561b9d0ccce56efa60a71
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v6
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v6
> Pull-Request: https://github.com/git/git/pull/2419
> 
> Range-diff vs v5:
> 
>   1:  851efeec8b ! 1:  917f373f91 ci: annotate leaks and stop a leak-sanitizer script at its first failure
>       @@ Commit message
>        
>                Process completed with exit code 1.
>        
>       -    Give a leak its own annotation. Point it at the test script, the exact
>       -    line isn't known, only which script the leak turned up in, and put the
>       -    sanitizer report in a log group next to it, so it stays visible.
>       +    Give a leak its own annotation, naming the script it turned up in, the
>       +    exact line isn't known, only which script:
>       +
>       +        memory leak logged in t1060 (t1060-object-corruption.sh)
>       +
>       +    Put the sanitizer report in a log group next to it, so it stays
>       +    visible.
>        
>            Once a script has one leak, it keeps running: the sanitizer log
>            directory is never cleared between tests, so every later test in the
>       @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
>         	>$github_markup_output
>         	GIT_TEST_TEE_OFFSET=0
>        +	github_markup_script_name=${0##*/}
>       -+}
>       -+
>       -+github_annotation_ () {
>       -+	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
>         }
>         
>         # No need to override start_test_case_output
>       @@ t/test-lib-github-workflow-markup.sh: finalize_test_case_output () {
>         }
>         
>        +finalize_test_leak_output () {
>       -+	# The exact line the leak turned up on isn't known, only the script,
>       -+	# so point at line 1.
>       -+	github_annotation_ error "t/$github_markup_script_name" 1 \
>       -+		"memory leak logged in $this_test"
>       ++	echo >>$github_markup_output \
>       ++		"::error::memory leak logged in $this_test ($github_markup_script_name)"
>        +
>        +	echo >>$github_markup_output "::group::leak: $this_test.$test_count"
>        +	cat "$TEST_RESULTS_SAN_FILE".* >>$github_markup_output
>   2:  46f93a9e16 ! 2:  acf1fbd250 ci: point test failures and fixed known breakages at their file and line
>       @@ Metadata
>         ## Commit message ##
>            ci: point test failures and fixed known breakages at their file and line
>        
>       -    When a test fails, GitHub shows an annotation naming it, for example:
>       +    A failing test gets an annotation in the Annotations list on its job's
>       +    summary page, naming it, for example:
>        
>                failed: t1060.17 partial clone of corrupted repository
>        
>       -    but the location GitHub attaches to that annotation is the CI
>       -    workflow file itself, not the test script, so there is nothing
>       -    pointing at where the test actually lives.
>       +    with no indication of where that test lives.
>        
>            Find the line a test is defined on by searching its script for the
>            test's own description as a fixed string, using the first match, and
>       -    attach that file and line to the annotation instead. Fall back to
>       -    line 1 when the description is not found verbatim, which happens when
>       -    a test builds its description at runtime instead of writing it out
>       -    literally.
>       +    add the file and line to the annotation's own message text:
>       +
>       +        failed: t1060.17 partial clone of corrupted repository (t1060-object-corruption.sh:141)
>       +
>       +    Fall back to naming just the script, with no line, when the
>       +    description is not found verbatim, which happens when a test builds
>       +    its description at runtime instead of writing it out literally.
>        
>            Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>        
>       @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
>        +	head -n 1 | cut -d: -f1
>        +}
>        +
>       - github_annotation_ () {
>       - 	echo >>$github_markup_output "::$1 file=$2,line=$3::$4"
>       - }
>       -@@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
>       + # No need to override start_test_case_output
>       +
>         finalize_test_case_output () {
>         	test_case_result=$1
>         	shift
>       @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
>        +	esac
>        +
>        +	test_case_line=$(find_test_case_line_ "$1")
>       ++	test_case_where="$github_markup_script_name${test_case_line:+:$test_case_line}"
>        +
>         	case "$test_case_result" in
>         	failure)
>        -		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1"
>       -+		github_annotation_ error "t/$github_markup_script_name" "${test_case_line:-1}" \
>       -+			"failed: $this_test.$test_count $1"
>       ++		echo >>$github_markup_output "::error::failed: $this_test.$test_count $1 ($test_case_where)"
>         		;;
>         	fixed)
>        -		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
>       @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
>        -	ok|broken)
>        -		# Exit without printing the "ok" or ""broken" tests
>        -		return
>       -+		github_annotation_ notice "t/$github_markup_script_name" "${test_case_line:-1}" \
>       -+			"fixed: $this_test.$test_count $1"
>       ++		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1 ($test_case_where)"
>         		;;
>         	esac
>        +
>