From 4f6de33f4147f745eb808b717666eb4d35a12716 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Sun, 28 Aug 2022 21:05:30 +0100
Subject: [PATCH] Print complement failure results last (#13639)

Since github always scrolls to the bottom of any test output, let's put the
failed tests last and hide any successful packages.
---
 .ci/complement_package.gotpl        | 14 ++++++--------
 .ci/scripts/gotestfmt               | 21 +++++++++++++++++++++
 .github/workflows/latest_deps.yml   |  2 +-
 .github/workflows/tests.yml         |  2 +-
 .github/workflows/twisted_trunk.yml |  2 +-
 changelog.d/13639.misc              |  1 +
 6 files changed, 31 insertions(+), 11 deletions(-)
 create mode 100755 .ci/scripts/gotestfmt
 create mode 100644 changelog.d/13639.misc

diff --git a/.ci/complement_package.gotpl b/.ci/complement_package.gotpl
index e1625fd31..2dd5e5e0e 100644
--- a/.ci/complement_package.gotpl
+++ b/.ci/complement_package.gotpl
@@ -27,10 +27,10 @@ which is under the Unlicense licence.
         {{- . -}}{{- "\n" -}}
     {{- end -}}
     {{- with .TestCases -}}
-        {{- /* Failing tests are first */ -}}
+        {{- /* Passing tests are first */ -}}
         {{- range . -}}
-            {{- if and (ne .Result "PASS") (ne .Result "SKIP") -}}
-                ::group::{{ "\033" }}[0;31m❌{{ " " }}{{- .Name -}}
+            {{- if eq .Result "PASS" -}}
+                ::group::{{ "\033" }}[0;32m✅{{ " " }}{{- .Name -}}
                 {{- "\033" -}}[0;37m ({{if $settings.ShowTestStatus}}{{.Result}}; {{end}}{{ .Duration -}}
                 {{- with .Coverage -}}
                     , coverage: {{ . }}%
@@ -47,7 +47,6 @@ which is under the Unlicense licence.
             {{- end -}}
         {{- end -}}
 
-
         {{- /* Then skipped tests are second */ -}}
         {{- range . -}}
             {{- if eq .Result "SKIP" -}}
@@ -68,11 +67,10 @@ which is under the Unlicense licence.
             {{- end -}}
         {{- end -}}
 
-
-        {{- /* Then passing tests are last */ -}}
+        {{- /* and failing tests are last */ -}}
         {{- range . -}}
-            {{- if eq .Result "PASS" -}}
-                ::group::{{ "\033" }}[0;32m✅{{ " " }}{{- .Name -}}
+            {{- if and (ne .Result "PASS") (ne .Result "SKIP") -}}
+                ::group::{{ "\033" }}[0;31m❌{{ " " }}{{- .Name -}}
                 {{- "\033" -}}[0;37m ({{if $settings.ShowTestStatus}}{{.Result}}; {{end}}{{ .Duration -}}
                 {{- with .Coverage -}}
                     , coverage: {{ . }}%
diff --git a/.ci/scripts/gotestfmt b/.ci/scripts/gotestfmt
new file mode 100755
index 000000000..83e0ec636
--- /dev/null
+++ b/.ci/scripts/gotestfmt
@@ -0,0 +1,21 @@
+#!/bin/bash
+#
+# wraps `gotestfmt`, hiding output from successful packages unless
+# all tests passed.
+
+set -o pipefail
+set -e
+
+# tee the test results to a log, whilst also piping them into gotestfmt,
+# telling it to hide successful results, so that we can clearly see
+# unsuccessful results.
+tee complement.log | gotestfmt -hide successful-packages
+
+# gotestfmt will exit non-zero if there were any failures, so if we got to this
+# point, we must have had a successful result.
+echo "All tests successful; showing all test results"
+
+# Pipe the test results back through gotestfmt, showing all results.
+# The log file consists of JSON lines giving the test results, interspersed
+# with regular stdout lines (including reports of downloaded packages).
+grep '^{"Time":' complement.log | gotestfmt
diff --git a/.github/workflows/latest_deps.yml b/.github/workflows/latest_deps.yml
index f263cf612..7dac617c4 100644
--- a/.github/workflows/latest_deps.yml
+++ b/.github/workflows/latest_deps.yml
@@ -163,7 +163,7 @@ jobs:
 
       - run: |
           set -o pipefail
-          TEST_ONLY_IGNORE_POETRY_LOCKFILE=1 POSTGRES=${{ (matrix.database == 'Postgres') && 1 || '' }} WORKERS=${{ (matrix.arrangement == 'workers') && 1 || '' }} COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh -json 2>&1 | gotestfmt
+          TEST_ONLY_IGNORE_POETRY_LOCKFILE=1 POSTGRES=${{ (matrix.database == 'Postgres') && 1 || '' }} WORKERS=${{ (matrix.arrangement == 'workers') && 1 || '' }} COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh -json 2>&1 | synapse/.ci/scripts/gotestfmt
         shell: bash
         name: Run Complement Tests
 
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index 144cb9ffa..3e6d718a7 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -354,7 +354,7 @@ jobs:
 
       - run: |
           set -o pipefail
-          POSTGRES=${{ (matrix.database == 'Postgres') && 1 || '' }} WORKERS=${{ (matrix.arrangement == 'workers') && 1 || '' }} COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh -json 2>&1 | gotestfmt
+          POSTGRES=${{ (matrix.database == 'Postgres') && 1 || '' }} WORKERS=${{ (matrix.arrangement == 'workers') && 1 || '' }} COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh -json 2>&1 | synapse/.ci/scripts/gotestfmt
         shell: bash
         name: Run Complement Tests
 
diff --git a/.github/workflows/twisted_trunk.yml b/.github/workflows/twisted_trunk.yml
index dd8e6fbb1..0906101cc 100644
--- a/.github/workflows/twisted_trunk.yml
+++ b/.github/workflows/twisted_trunk.yml
@@ -137,7 +137,7 @@ jobs:
 
       - run: |
           set -o pipefail
-          TEST_ONLY_SKIP_DEP_HASH_VERIFICATION=1 POSTGRES=${{ (matrix.database == 'Postgres') && 1 || '' }} WORKERS=${{ (matrix.arrangement == 'workers') && 1 || '' }} COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh -json 2>&1 | gotestfmt
+          TEST_ONLY_SKIP_DEP_HASH_VERIFICATION=1 POSTGRES=${{ (matrix.database == 'Postgres') && 1 || '' }} WORKERS=${{ (matrix.arrangement == 'workers') && 1 || '' }} COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh -json 2>&1 | synapse/.ci/scripts/gotestfmt
         shell: bash
         name: Run Complement Tests
 
diff --git a/changelog.d/13639.misc b/changelog.d/13639.misc
new file mode 100644
index 000000000..de4e4d120
--- /dev/null
+++ b/changelog.d/13639.misc
@@ -0,0 +1 @@
+Improve readability of Complement CI logs by printing failure results last.