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

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

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