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

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

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