Merge pull request #118404 from pohly/verify-failures

better JUnit failure messages for golangci-lint and verify in general

hack/make-rules/verify.sh

@@ -127,10 +127,10 @@ function run-cmd {
   local tr
 
   if ${SILENT}; then
-    juLog -output="${output}" -class="verify" -name="${testname}" "$@" &> /dev/null
+    juLog -output="${output}" -class="verify" -name="${testname}" -fail="^ERROR: " "$@" &> /dev/null
     tr=$?
   else
-    juLog -output="${output}" -class="verify" -name="${testname}" "$@"
+    juLog -output="${output}" -class="verify" -name="${testname}" -fail="^ERROR: " "$@"
     tr=$?
   fi
   return ${tr}

hack/verify-golangci-lint-pr.sh

@@ -18,7 +18,6 @@
 # golangci-lint. It does nothing when invoked as part of a normal "make
 # verify".
 
-set -o errexit
 set -o nounset
 set -o pipefail
 
@@ -29,9 +28,12 @@ fi
 
 KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
 
+# include shell2junit library
+source "${KUBE_ROOT}/third_party/forked/shell2junit/sh2ju.sh"
+
 # TODO (https://github.com/kubernetes/test-infra/issues/17056):
 # take this additional artifact and convert it to GitHub annotations
 # to make it easier to see these problems during a PR review.
 #
 # -g "${ARTIFACTS}/golangci-lint-githubactions.log"
-"${KUBE_ROOT}/hack/verify-golangci-lint.sh" -r "${PULL_BASE_SHA}" -s
+juLog -output="${ARTIFACTS:-/tmp/results}" -class="golangci" -name="golangci-strict-pr" -fail="^ERROR: " "${KUBE_ROOT}/hack/verify-golangci-lint.sh" -r "${PULL_BASE_SHA}" -s

hack/verify-golangci-lint.sh

@@ -97,6 +97,19 @@ if [ "${golangci_config}" ]; then
     golangci+=(--config="${golangci_config}")
 fi
 
+# Below the output of golangci-lint is going to be piped into sed to add
+# a prefix to each output line. This helps make the output more visible
+# in the Prow log viewer ("error" is a key word there) and ensures that
+# only those lines get included as failure message in a JUnit file
+# by "make verify".
+#
+# The downside is that the automatic detection whether to colorize output
+# doesn't work anymore, so here we force it ourselves when connected to
+# a tty.
+if tty -s; then
+    golangci+=(--color=always)
+fi
+
 if [ "$base" ]; then
     # Must be a something that git can resolve to a commit.
     # "git rev-parse --verify" checks that and prints a detailed
@@ -139,15 +152,15 @@ res=0
 run () {
   if [[ "${#targets[@]}" -gt 0 ]]; then
     echo "running ${golangci[*]} ${targets[*]}" >&2
-    "${golangci[@]}" "${targets[@]}" >&2 || res=$?
+    "${golangci[@]}" "${targets[@]}" 2>&1 | sed -e 's;^;ERROR: ;' >&2 || res=$?
   else
     echo "running ${golangci[*]} ./..." >&2
-    "${golangci[@]}" ./... >&2 || res=$?
+    "${golangci[@]}" ./... 2>&1 | sed -e 's;^;ERROR: ;' >&2 || res=$?
     for d in staging/src/k8s.io/*; do
       MODPATH="staging/src/k8s.io/$(basename "${d}")"
       echo "running ( cd ${KUBE_ROOT}/${MODPATH}; ${golangci[*]} --path-prefix ${MODPATH} ./... )"
       pushd "${KUBE_ROOT}/${MODPATH}" >/dev/null
-        "${golangci[@]}" --path-prefix "${MODPATH}" ./... >&2 || res=$?
+        "${golangci[@]}" --path-prefix "${MODPATH}" ./... 2>&1 | sed -e 's;^;ERROR: ;' >&2 || res=$?
       popd >/dev/null
     done
   fi

third_party/forked/shell2junit/sh2ju.sh

@@ -18,6 +18,10 @@
 ###             -name="TestName" : the test name which will be shown in the junit report
 ###             -error="RegExp"  : a regexp which sets the test as failure when the output matches it
 ###             -ierror="RegExp" : same as -error but case insensitive
+###             -fail="RegExp"   : Any line from stderr which contains this pattern becomes part of
+###                                the failure messsage, without the text matching that pattern.
+###                                Example: -failure="^ERROR: "
+###                                Default is to use the entire stderr as failure message.
 ###             -output="Path"   : path to output directory, defaults to "./results"
 ###     - Junit reports are left in the folder 'result' under the directory where the script is executed.
 ###     - Configure Jenkins to parse junit files from the generated folder
@@ -61,6 +65,7 @@ function juLog() {
   errfile=/tmp/evErr.$$.log
   date="$(which gdate 2>/dev/null || which date)"
   asserts=00; errors=0; total=0; content=""
+  local failureRe=""
 
   # parse arguments
   ya=""; icase=""
@@ -70,6 +75,7 @@ function juLog() {
       -class=*)  class="$(echo "$1" | ${SED} -e 's/-class=//')";   shift;;
       -ierror=*) ereg="$(echo "$1" | ${SED} -e 's/-ierror=//')"; icase="-i"; shift;;
       -error=*)  ereg="$(echo "$1" | ${SED} -e 's/-error=//')";  shift;;
+      -fail=*)  failureRe="$(echo "$1" | ${SED} -e 's/-fail=//')";  shift;;
       -output=*) juDIR="$(echo "$1" | ${SED} -e 's/-output=//')";  shift;;
       *)         ya=1;;
     esac
@@ -135,9 +141,23 @@ function juLog() {
 
   # write the junit xml report
   ## failure tag
-  [[ ${err} = 0 ]] && failure="" || failure="
+  local failure=""
-      <failure type=\"ScriptError\" message=\"Script Error\"><![CDATA[${errMsg}]]></failure>
+  if [[ ${err} != 0 ]]; then
+      local failureMsg
+      if [ -n "${failureRe}" ]; then
+          failureMsg="$(echo "${errMsg}" | grep -e "${failureRe}" | ${SED} -e "s;${failureRe};;")"
+          if [ -z "${failureMsg}" ]; then
+              failureMsg="see stderr for details"
+          fi
+      else
+          failureMsg="${errMsg}"
+      fi
+      failure="
+      <failure type=\"ScriptError\"><![CDATA[
+${failureMsg}
+]]></failure>
   "
+  fi
   ## testcase tag
   content="${content}
     <testcase assertions=\"1\" name=\"${name}\" time=\"${time}\" classname=\"${class}\">