Re: [PATCH] ci: point leak-sanitizer failures at the actual test and error
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Sep 27, 2026, 15:19 UTC
- Message-ID
- <fc4efe9f-69f4-4f58-9f7c-8f2e75a8e590@gmail.com>
- In-Reply-To
- <pull.2419.git.git.1790362443893.gitgitgadget@gmail.com>
Hi Harald
On 25/09/2026 19:54, Harald Nordgren via GitGitGadget wrote:
Show 9 quoted lines
> 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
Show 119 quoted lines
>
> 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