Subject: Re: [PATCH] ci(github): also skip logs of broken test cases

From: Victoria Dye <[email-masked]>
Date: Mon, 22 Jan 2024 14:41:41 -0800

Philippe Blain via GitGitGadget wrote:
> From: Philippe Blain <[email-masked]>
> 
> When a test fails in the GitHub Actions CI pipeline, we mark it up using
> special GitHub syntax so it stands out when looking at the run log. We
> also mark up "fixed" test cases, and skip passing tests since we want to
> concentrate on the failures.
> 
> The finalize_test_case_output function in
> test-lib-github-workflow-markup.sh which performs this markup is however
> missing a fourth case: "broken" tests, i.e. tests using
> 'test_expect_failure' to document a known bug. This leads to these
> "broken" tests appearing along with any failed tests, potentially
> confusing the reader who might not be aware that "broken" is the status
> for 'test_expect_failure' tests that indeed failed, and wondering what
> their commits "broke".
> 
> Also skip these "broken" tests so that only failures and fixed tests
> stand out.
> 
> Signed-off-by: Philippe Blain <[email-masked]>
> ---
>     ci(github): also skip logs of broken test cases
>     
>      * An example of a run with failed tests appearing along with several
>        "broken" tests:
>        https://github.com/phil-blain/git/actions/runs/7589303055/job/20673657755
>      * An example of a run with the same failures, but with this patch on
>        top (no "broken" tests listed):
>      * https://github.com/phil-blain/git/actions/runs/7598605434/job/20694762480

Thanks for making this change, the more focused logs are much nicer to read
(and ostensibly a bit more performant as well).

> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1649%2Fphil-blain%2Fgithub-ci-skip-broken-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1649/phil-blain/github-ci-skip-broken-v1
> Pull-Request: https://github.com/git/git/pull/1649
> 
>  t/test-lib-github-workflow-markup.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
> index 970c6538cba..33405c90d74 100644
> --- a/t/test-lib-github-workflow-markup.sh
> +++ b/t/test-lib-github-workflow-markup.sh
> @@ -42,8 +42,8 @@ finalize_test_case_output () {
>  	fixed)
>  		echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
>  		;;
> -	ok)
> -		# Exit without printing the "ok" tests
> +	ok|broken)
> +		# Exit without printing the "ok" or ""broken" tests

And the implementation itself is nice and simple. Looks good to me!

>  		return
>  		;;
>  	esac
> 
> base-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c