[PATCH v6 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.

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