Re: [PATCH 4/9] ci/run-build-and-tests: add some structure to the GitHub workflow output
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Feb 23, 2022, 12:13 UTC
- Message-ID
- <f73586a9-e9f1-aa1d-ac22-8102e6583e2b@gmail.com>
- In-Reply-To
- <9333ba781b8240f704e739b00d274f8c3d887e39.1643050574.git.gitgitgadget@gmail.com>
Hi Dscho
On 24/01/2022 18:56, Johannes Schindelin via GitGitGadget wrote:
Show 14 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de> > > The current output of Git's GitHub workflow can be quite confusing, > especially for contributors new to the project. > > To make it more helpful, let's introduce some collapsible grouping. > Initially, readers will see the high-level view of what actually > happened (did the build fail, or the test suite?). To drill down, the > respective group can be expanded. > > Note: sadly, workflow output currently cannot contain any nested groups > (see https://github.com/actions/runner/issues/802 for details), > therefore we take pains to ensure to end any previous group before > starting a new one.
Thanks for working on this, I find it makes it much easier to get to the information I need when a test fails. I'm not familiar with github's grouping but I noticed something below I didn't understand.
Show 57 quoted lines
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> ci/lib.sh | 55 ++++++++++++++++++++++++++++++++++-----
> ci/run-build-and-tests.sh | 4 +--
> ci/run-test-slice.sh | 2 +-
> 3 files changed, 51 insertions(+), 10 deletions(-)
>
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 2b2c0932320..4ed8f40ab02 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -1,5 +1,49 @@
> # Library of functions shared by all CI scripts
>
> +if test true != "$GITHUB_ACTIONS"
> +then
> + begin_group () { :; }
> + end_group () { :; }
> +
> + group () {
> + shift
> + "$@"
> + }
> + set -x
> +else
> + begin_group () {
> + need_to_end_group=t
> + echo "::group::$1" >&2
> + set -x
> + }
> +
> + end_group () {
> + test -n "$need_to_end_group" || return 0
> + set +x
> + need_to_end_group=
> + echo '::endgroup::' >&2
> + }
> + trap end_group EXIT
> +
> + group () {
> + set +x
> + begin_group "$1"
> + shift
> + "$@"
> + res=$?
> + end_group
> + return $res
> + }
> +
> + begin_group "CI setup"
> +fi
> +
> +# Set 'exit on error' for all CI scripts to let the caller know that
> +# something went wrong.
> +# Set tracing executed commands, primarily setting environment variables
> +# and installing dependencies.
> +set -eThe comment is moved unchanged but the set command has lost the "-x". We now have several "set -x" commands in the functions above and one below "end_group" lower down. Does the comment need updating as we are not enabling the tracing of executed commands here anymore?
Best Wishes
Phillip
Show 62 quoted lines
> skip_branch_tip_with_tag () {
> # Sometimes, a branch is pushed at the same time the tag that points
> # at the same commit as the tip of the branch is pushed, and building
> @@ -88,12 +132,6 @@ export TERM=${TERM:-dumb}
> # Clear MAKEFLAGS that may come from the outside world.
> export MAKEFLAGS=
>
> -# Set 'exit on error' for all CI scripts to let the caller know that
> -# something went wrong.
> -# Set tracing executed commands, primarily setting environment variables
> -# and installing dependencies.
> -set -ex
> -
> if test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI"
> then
> CI_TYPE=azure-pipelines
> @@ -138,7 +176,7 @@ then
> test_name="${test_exit%.exit}"
> test_name="${test_name##*/}"
> printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
> - cat "t/test-results/$test_name.out"
> + group "Failed test: $test_name" cat "t/test-results/$test_name.out"
>
> trash_dir="t/trash directory.$test_name"
> cp "t/test-results/$test_name.out" t/failed-test-artifacts/
> @@ -234,3 +272,6 @@ linux-leaks)
> esac
>
> MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
> +
> +end_group
> +set -x
> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
> index e49f9eaa8c0..5516f45f7fe 100755
> --- a/ci/run-build-and-tests.sh
> +++ b/ci/run-build-and-tests.sh
> @@ -48,10 +48,10 @@ esac
> # Any new "test" targets should not go after this "make", but should
> # adjust $MAKE_TARGETS. Otherwise compilation-only targets above will
> # start running tests.
> -make
> +group Build make
> if test -n "$run_tests"
> then
> - make test ||
> + group "Run tests" make test ||
> handle_failed_tests
> fi
> check_unignored_build_artifacts
> diff --git a/ci/run-test-slice.sh b/ci/run-test-slice.sh
> index 63358c23e11..a3c67956a8d 100755
> --- a/ci/run-test-slice.sh
> +++ b/ci/run-test-slice.sh
> @@ -10,7 +10,7 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";;
> *) ln -s "$cache_dir/.prove" t/.prove;;
> esac
>
> -make --quiet -C t T="$(cd t &&
> +group "Run tests" make --quiet -C t T="$(cd t &&
> ./helper/test-tool path-utils slice-tests "$1" "$2" t[0-9]*.sh |
> tr '\n' ' ')" ||
> handle_failed_tests