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

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 [off-list ref]
      
       ## 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 [off-list ref]
      
     @@ 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