ci: make it easier to find failed tests' logs in the GitHub workflow

When investigating a test failure, the time that matters most is the
time it takes from getting aware of the failure to displaying the output
of the failing test case.

You currently have to know a lot of implementation details when
investigating test failures in the CI runs. The first step is easy: the
failed job is marked quite clearly, but when opening it, the failed step
is expanded, which in our case is the one running
`ci/run-build-and-tests.sh`. This step, most notably, only offers a
high-level view of what went wrong: it prints the output of `prove`
which merely tells the reader which test script failed.

The actually interesting part is in the detailed log of said failed
test script. But that log is shown in the CI run's step that runs
`ci/print-test-failures.sh`. And that step is _not_ expanded in the web
UI by default. It is even marked as "successful", which makes it very
easy to miss that there is useful information hidden in there.

Let's help the reader by showing the failed tests' detailed logs in the
step that is expanded automatically, i.e. directly after the test suite
failed.

This also helps the situation where the _build_ failed and the
`print-test-failures` step was executed under the assumption that the
_test suite_ failed, and consequently failed to find any failed tests.

An alternative way to implement this patch would be to source
`ci/print-test-failures.sh` in the `handle_test_failures` function to
show these logs. However, over the course of the next few commits, we
want to introduce some grouping which would be harder to achieve that
way (for example, we do want a leaner, and colored, preamble for each
failed test script, and it would be trickier to accommodate the lack of
nested groupings in GitHub workflows' output).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index c35200defb9357b6438ba3391bb4e17fed67acdc..3fa88b78b6db04b145f6653ba74e9b4457f40a58 100644 (file)
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -119,10 +119,6 @@ jobs:
     - name: test
       shell: bash
       run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10
-    - name: ci/print-test-failures.sh
-      if: failure()
-      shell: bash
-      run: ci/print-test-failures.sh
     - name: Upload failed tests' directories
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
       uses: actions/upload-artifact@v2
@@ -204,10 +200,6 @@ jobs:
       env:
         NO_SVN_TESTS: 1
       run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10
-    - name: ci/print-test-failures.sh
-      if: failure()
-      shell: bash
-      run: ci/print-test-failures.sh
     - name: Upload failed tests' directories
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
       uses: actions/upload-artifact@v2
@@ -261,8 +253,6 @@ jobs:
     - uses: actions/checkout@v2
     - run: ci/install-dependencies.sh
     - run: ci/run-build-and-tests.sh
-    - run: ci/print-test-failures.sh
-      if: failure()
     - name: Upload failed tests' directories
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
       uses: actions/upload-artifact@v2
@@ -292,8 +282,6 @@ jobs:
     - uses: actions/checkout@v1
     - run: ci/install-docker-dependencies.sh
     - run: ci/run-build-and-tests.sh
-    - run: ci/print-test-failures.sh
-      if: failure()
     - name: Upload failed tests' directories
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
       uses: actions/upload-artifact@v1

diff --git a/ci/lib.sh b/ci/lib.sh
index d718f4e386dbe945d993b080722030d5bbe01764..65f5188a55034721b75b1c019ba329115863e64f 100755 (executable)
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -78,6 +78,10 @@ check_unignored_build_artifacts () {
        }
 }
 
+handle_failed_tests () {
+       return 1
+}
+
 # GitHub Action doesn't set TERM, which is required by tput
 export TERM=${TERM:-dumb}
 
@@ -123,6 +127,25 @@ then
        CI_JOB_ID="$GITHUB_RUN_ID"
        CC="${CC_PACKAGE:-${CC:-gcc}}"
        DONT_SKIP_TAGS=t
+       handle_failed_tests () {
+               mkdir -p t/failed-test-artifacts
+               echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
+
+               for test_exit in t/test-results/*.exit
+               do
+                       test 0 != "$(cat "$test_exit")" || continue
+
+                       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"
+
+                       trash_dir="t/trash directory.$test_name"
+                       cp "t/test-results/$test_name.out" t/failed-test-artifacts/
+                       tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
+               done
+               return 1
+       }
 
        cache_dir="$HOME/none"
 

diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index 2818b3046aef4a062d83027c3aae8ea85c79ee53..1ede75e5556a797936e669371207ffc5b40f3485 100755 (executable)
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -48,7 +48,8 @@ esac
 make
 if test -n "$run_tests"
 then
-       make test
+       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 f8c2c3106a2ef43af525ec0a770d6697160ec91f..63358c23e118e9381aeeb188f101f04c8489da84 100755 (executable)
--- a/ci/run-test-slice.sh
+++ b/ci/run-test-slice.sh
@@ -12,6 +12,7 @@ esac
 
 make --quiet -C t T="$(cd t &&
        ./helper/test-tool path-utils slice-tests "$1" "$2" t[0-9]*.sh |
-       tr '\n' ' ')"
+       tr '\n' ' ')" ||
+handle_failed_tests
 
 check_unignored_build_artifacts