Re: [PATCH v3 2/2] ci: point test failures and fixed known breakages at their file and line
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Sep 30, 2026, 14:56 UTC
- Message-ID
- <5529bccf-eeb1-40f9-ae03-8fa19dc26f5a@gmail.com>
- In-Reply-To
- <750c3605128c268f331b1b9477ca0489ced75543.1790748583.git.gitgitgadget@gmail.com>
Hi Harald
On 30/09/2026 07:09, Harald Nordgren via GitGitGadget wrote:
Show 5 quoted lines
> From: Harald Nordgren <haraldnordgren@gmail.com> > > 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.
Show 5 quoted lines
> 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
Show 67 quoted lines
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> 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