From 92755d3329163d6ec5b25303b9b665d3f960d839 Mon Sep 17 00:00:00 2001
From: Ivan Kozlovic <ivan@synadia.com>
Date: Mon, 2 May 2022 14:51:19 -0600
Subject: [PATCH] Fixed GithubAction code coverage to handle test panics

We are ok with a flapper or two, because they should not affect
code coverage that much, so it is better to have those and publish
code coverage than to have to recycle the whole test suite until
we get no test failure.

However, if there is a test panic, then all other tests within this
package will NOT run, which then would have possibly a massive
impact in the code coverage percentage.

These changes will ensure that the run fails if one of the code
coverage output is "empty" (it is actually not empty, but the
initial content is "mode: atomic" and then whe code coverage is
complete, it gets filled with actual code coverage data).
On failure, the push to coverall will not happen.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
---
 .github/workflows/cov.yaml |  4 ++--
 scripts/cov.sh             | 28 +++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/cov.yaml b/.github/workflows/cov.yaml
index daf70532..7923bdf3 100644
--- a/.github/workflows/cov.yaml
+++ b/.github/workflows/cov.yaml
@@ -26,10 +26,10 @@ jobs:
 
       - name: Run code coverage
         shell: bash --noprofile --norc -x -eo pipefail {0}
-        # Do not make the build fail even if code coverage reported
-        # a test failure.
         run: |
+          set -e
           ./scripts/cov.sh upload
+          set +e
 
       - name: Convert coverage.out to coverage.lcov
         uses: jandelgado/gcov2lcov-action@v1.0.8
diff --git a/scripts/cov.sh b/scripts/cov.sh
index 558fc17f..70dfa8f9 100755
--- a/scripts/cov.sh
+++ b/scripts/cov.sh
@@ -1,7 +1,18 @@
 #!/bin/bash
 # Run from directory above via ./scripts/cov.sh
 
-# Do not set the -e flag because we don't a flapper to prevent the push to coverall.
+check_file () {
+    # If the content of the file is simply "mode: atomic", then it means that the
+    # code coverage did not complete due to a panic in one of the tests.
+    if [[ $(cat ./cov/$2) == "mode: atomic" ]]; then
+        echo "#############################################"
+        echo "## Code coverage for $1 package failed ##"
+        echo "#############################################"
+        exit 1
+    fi
+}
+
+# Do not globally set the -e flag because we don't a flapper to prevent the push to coverall.
 
 export GO111MODULE="on"
 
@@ -9,14 +20,28 @@ go install github.com/wadey/gocovmerge@latest
 
 rm -rf ./cov
 mkdir cov
+#
 # Since it is difficult to get a full run without a flapper, do not use `-failfast`.
 # It is better to have one flapper or two and still get the report than have
 # to re-run the whole code coverage. One or two failed tests should not affect
 # so much the code coverage.
+#
+# However, we need to take into account that if there is a panic in one test, all
+# other tests in that package will not run, which then would cause the code coverage
+# to drastically be lowered. In that case, we don't want the code coverage to be
+# uploaded.
+#
 go test -v -covermode=atomic -coverprofile=./cov/conf.out ./conf -timeout=20m -tags=skip_no_race_tests
+check_file "conf" "conf.out"
 go test -v -covermode=atomic -coverprofile=./cov/log.out ./logger -timeout=20m -tags=skip_no_race_tests
+check_file "logger" "log.out"
 go test -v -covermode=atomic -coverprofile=./cov/server.out ./server -timeout=20m -tags=skip_no_race_tests
+check_file "server" "server.out"
 go test -v -covermode=atomic -coverprofile=./cov/test.out -coverpkg=./server ./test -timeout=20m -tags=skip_no_race_tests
+check_file "test" "test.out"
+
+# At this point, if that fails, we want the caller to know about the failure.
+set -e
 gocovmerge ./cov/*.out > acc.out
 rm -rf ./cov
 
@@ -24,3 +49,4 @@ rm -rf ./cov
 if [[ $1 == "" ]]; then
     go tool cover -html=acc.out
 fi
+set +e