Re: [PATCH v4 2/2] ci: point test failures and fixed known breakages at their file and line
flat view
From: Phillip Wood <hidden>
Date: 2026-10-01 19:49:18
Hi Harald On 01/10/2026 19:44, Harald Nordgren via GitGitGadget wrote:
From: Harald Nordgren <redacted> 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.
I'm afraid I'm still not clear what this does in practical terms. What appears in the test output that the user sees that didn't before?
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 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)`.
That's a useful example of why we want to escape the output which makes it all the more puzzling that we don't escape the existing annotations that I mentioned last time. Thanks Phillip
quoted hunk ↗ jump to hunk
Signed-off-by: Harald Nordgren <redacted> --- t/test-lib-github-workflow-markup.sh | 38 +++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-)diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh index 0d54496358..6fee4dfb22 100644 --- a/t/test-lib-github-workflow-markup.sh +++ b/t/test-lib-github-workflow-markup.sh@@ -31,6 +31,22 @@ 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 + # hit. The -- keeps a description starting with "-" from being read + # as an option. + 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" }@@ -40,18 +56,28 @@ github_annotation_ () { 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_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" ;; 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 $test_case_description" ;; 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