{"thread":{"id":"60776","subject":"[PATCH] ci(github): also skip logs of broken test cases","startedAt":"2024-01-21T03:38:36Z","lastAt":"2024-01-22T23:16:37Z","messageCount":3,"participants":["Philippe Blain via GitGitGadget","Victoria Dye","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"487163","messageId":"pull.1649.git.git.1705808313306.gitgitgadget@gmail.com","threadId":"60776","inReplyTo":null,"subject":"[PATCH] ci(github): also skip logs of broken test cases","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-21T03:38:33Z","receivedAt":"2024-01-21T03:38:36Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain <levraiphilippeblain@gmail.com>\n\nWhen a test fails in the GitHub Actions CI pipeline, we mark it up using\nspecial GitHub syntax so it stands out when looking at the run log. We\nalso mark up \"fixed\" test cases, and skip passing tests since we want to\nconcentrate on the failures.\n\nThe finalize_test_case_output function in\ntest-lib-github-workflow-markup.sh which performs this markup is however\nmissing a fourth case: \"broken\" tests, i.e. tests using\n'test_expect_failure' to document a known bug. This leads to these\n\"broken\" tests appearing along with any failed tests, potentially\nconfusing the reader who might not be aware that \"broken\" is the status\nfor 'test_expect_failure' tests that indeed failed, and wondering what\ntheir commits \"broke\".\n\nAlso skip these \"broken\" tests so that only failures and fixed tests\nstand out.\n\nSigned-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n---\n    ci(github): also skip logs of broken test cases\n    \n     * An example of a run with failed tests appearing along with several\n       \"broken\" tests:\n       https://github.com/phil-blain/git/actions/runs/7589303055/job/20673657755\n     * An example of a run with the same failures, but with this patch on\n       top (no \"broken\" tests listed):\n     * https://github.com/phil-blain/git/actions/runs/7598605434/job/20694762480\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1649%2Fphil-blain%2Fgithub-ci-skip-broken-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1649/phil-blain/github-ci-skip-broken-v1\nPull-Request: https://github.com/git/git/pull/1649\n\n t/test-lib-github-workflow-markup.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\nindex 970c6538cba..33405c90d74 100644\n--- a/t/test-lib-github-workflow-markup.sh\n+++ b/t/test-lib-github-workflow-markup.sh\n@@ -42,8 +42,8 @@ finalize_test_case_output () {\n \tfixed)\n \t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n \t\t;;\n-\tok)\n-\t\t# Exit without printing the \"ok\" tests\n+\tok|broken)\n+\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n \t\treturn\n \t\t;;\n \tesac\n\nbase-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c\n-- \ngitgitgadget\n"},{"id":"487231","messageId":"dbe25fff-e1d4-41f2-8f8f-c538e8c2a77e@github.com","threadId":"60776","inReplyTo":"pull.1649.git.git.1705808313306.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci(github): also skip logs of broken test cases","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2024-01-22T22:41:41Z","receivedAt":"2024-01-22T22:41:43Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Philippe Blain via GitGitGadget wrote:\n> From: Philippe Blain <levraiphilippeblain@gmail.com>\n> \n> When a test fails in the GitHub Actions CI pipeline, we mark it up using\n> special GitHub syntax so it stands out when looking at the run log. We\n> also mark up \"fixed\" test cases, and skip passing tests since we want to\n> concentrate on the failures.\n> \n> The finalize_test_case_output function in\n> test-lib-github-workflow-markup.sh which performs this markup is however\n> missing a fourth case: \"broken\" tests, i.e. tests using\n> 'test_expect_failure' to document a known bug. This leads to these\n> \"broken\" tests appearing along with any failed tests, potentially\n> confusing the reader who might not be aware that \"broken\" is the status\n> for 'test_expect_failure' tests that indeed failed, and wondering what\n> their commits \"broke\".\n> \n> Also skip these \"broken\" tests so that only failures and fixed tests\n> stand out.\n> \n> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n> ---\n>     ci(github): also skip logs of broken test cases\n>     \n>      * An example of a run with failed tests appearing along with several\n>        \"broken\" tests:\n>        https://github.com/phil-blain/git/actions/runs/7589303055/job/20673657755\n>      * An example of a run with the same failures, but with this patch on\n>        top (no \"broken\" tests listed):\n>      * https://github.com/phil-blain/git/actions/runs/7598605434/job/20694762480\n\nThanks for making this change, the more focused logs are much nicer to read\n(and ostensibly a bit more performant as well).\n\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1649%2Fphil-blain%2Fgithub-ci-skip-broken-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1649/phil-blain/github-ci-skip-broken-v1\n> Pull-Request: https://github.com/git/git/pull/1649\n> \n>  t/test-lib-github-workflow-markup.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh\n> index 970c6538cba..33405c90d74 100644\n> --- a/t/test-lib-github-workflow-markup.sh\n> +++ b/t/test-lib-github-workflow-markup.sh\n> @@ -42,8 +42,8 @@ finalize_test_case_output () {\n>  \tfixed)\n>  \t\techo >>$github_markup_output \"::notice::fixed: $this_test.$test_count $1\"\n>  \t\t;;\n> -\tok)\n> -\t\t# Exit without printing the \"ok\" tests\n> +\tok|broken)\n> +\t\t# Exit without printing the \"ok\" or \"\"broken\" tests\n\nAnd the implementation itself is nice and simple. Looks good to me!\n\n>  \t\treturn\n>  \t\t;;\n>  \tesac\n> \n> base-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c\n\n"},{"id":"487234","messageId":"xmqqv87lq7qr.fsf@gitster.g","threadId":"60776","inReplyTo":"dbe25fff-e1d4-41f2-8f8f-c538e8c2a77e@github.com","subject":"Re: [PATCH] ci(github): also skip logs of broken test cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-22T23:16:28Z","receivedAt":"2024-01-22T23:16:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Victoria Dye <vdye@github.com> writes:\n\n> Thanks for making this change, the more focused logs are much nicer to read\n> (and ostensibly a bit more performant as well).\n> ...\n> And the implementation itself is nice and simple. Looks good to me!\n\nThanks, both.\n\nQueued.\n"}]}