Thread (40 messages) 40 messages, 5 authors, 7d ago

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

flat view
COOLING7d SUPERSEDED

From: Harald Nordgren via GitGitGadget <hidden>
Date: 2026-10-03 08:12:00

Revision v5 of 5 in this series.

Revisions (5)
  1. v2 [diff vs current]
  2. v3 [diff vs current]
  3. v4 [diff vs current]
  4. v5 current
  5. v6 [diff vs current]
Link failure and leak annotations in CI to the test script, so both can be
found from the job summary.

V5 CI Job where failures and leaks are reported:
https://github.com/git/git/actions/runs/36979818141/job/110752027863?pr=2426

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 | 46 ++++++++++++++++++++++++----
 t/test-lib.sh                        |  6 +++-
 3 files changed, 46 insertions(+), 7 deletions(-)


base-commit: c46c1e37724f0478939de636ab8ea5a89086d532
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2419%2FHaraldNordgren%2Fci-annotation-file-line-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2419/HaraldNordgren/ci-annotation-file-line-v5
Pull-Request: https://github.com/git/git/pull/2419

Range-diff vs v4:

 1:  138394b48b = 1:  851efeec8b ci: annotate leaks and stop a leak-sanitizer script at its first failure
 2:  8ec2b53d82 ! 2:  46f93a9e16 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
      
     -    A test failure or a fixed known breakage gets an annotation that names
     -    the test but says nothing about where it's defined, so a reviewer has
     -    to search the script by hand to find it.
     +    When a test fails, GitHub shows an annotation naming it, for example:
      
     -    Find the line a test is defined on by searching the script for its
     -    description as a fixed string, using the first match. Fall back to
     +        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.
     +
     +    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.
      
     -    A GitHub annotation is a single line, so a `%` in a test description
     -    has to be percent-encoded as `%25`, or GitHub misreads it as its own
     -    escape sequence. for-each-ref's format atoms use plenty of them, e.g.
     -    `%(raw)`.
     -
          Signed-off-by: Harald Nordgren [off-list ref]
      
       ## t/test-lib-github-workflow-markup.sh ##
     @@ t/test-lib-github-workflow-markup.sh: start_test_output () {
       	github_markup_script_name=${0##*/}
       }
       
     -+github_escape_message_ () {
     -+	# % has to be escaped or GitHub misreads it as the start of its own
     -+	# percent-encoding (e.g. a literal %(raw) in a for-each-ref test
     -+	# description).
     -+	sed -e 's/%/%25/g'
     -+}
     -+
      +find_test_case_line_ () {
      +	# A description can contain characters like [ or * that would
      +	# corrupt a regex search, so match it literally and take the first
     @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
      +	esac
      +
      +	test_case_line=$(find_test_case_line_ "$1")
     -+	test_case_description=$(printf '%s' "$1" | github_escape_message_)
      +
       	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 $test_case_description"
     ++			"failed: $this_test.$test_count $1"
       		;;
       	fixed)
      -		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
     @@ t/test-lib-github-workflow-markup.sh: github_annotation_ () {
      -		# 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 $test_case_description"
     ++			"fixed: $this_test.$test_count $1"
       		;;
       	esac
      +

-- 
gitgitgadget
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help