Re: [PATCH v3 2/2] ci: point test failures and fixed known breakages at their file and line
flat view
From: Phillip Wood <hidden>
Date: 2026-09-30 14:56:48
Hi Harald On 30/09/2026 07:09, 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 carries no file or line, so there is nothing to click through to from the GitHub UI.
Have you got an example of this? As I said in my last mail, I can't see any links in the output from the linux-leaks job.
Find the line a test is defined on by searching the script for its description as a fixed string, using the first match. A description can contain characters like `[` or `*` that a regex search would misread, so match it literally.
This second sentence doesn't really add anything - you've already said we're searching for a fixed string.
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.
Ironically, it is the dynamically generated tests where a line number would be most useful, but there is no easy way to determine what line we should be using.
A GitHub annotation is a single line, and a test description is always one line too, so only a `%` or a stray carriage return in it needs percent-encoding to keep the annotation intact. Escape `%` first, or a carriage return's own encoding would be mangled by a `%` substitution that ran after it.
Why do we need to escape the test descriptions when we haven't been doing so up to now? Also if the test description is a single line why are we worring about '\r'? If it is so important to escape the output why does this patch not convert the existing annotations like the "group::" on in the trailing context lines? 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..66d2ccca18 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_ () { + # A test description is always one line, so only % and CR need + # escaping here. Escape % first, or CR's own %-encoding gets mangled. + # \r is not a portable sed escape, so splice in the actual byte. + sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/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; -- 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