From dbbb21469f46758d912fea115f62a795d84c685f Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Fri, 2 Jun 2023 11:41:05 +0200 Subject: [PATCH] verify: pick relevant lines from verify-golangci-lint.sh as failure message When sh2ju.sh was called to generate the junit_verify.xml, it used to include the entire output of a failed script twice: once as failure message, once as log output. This output can be large and often the actual failure isn't near the top, but rather at the end or (in the case of the different golangci-lint invocations) embedded in the log. This makes them hard to see at a glance when looking at the Prow result page for a job. Now a verify script can prefix relevant lines with "ERROR: " and then only those lines are used as failure message in JUnit, without that prefix. That string was chosen because Prow itself also then picks up those lines when viewing the entire build log and it is unlikely that some script prints such lines when they are not meant to be part of the failure. If some script outputs no such lines, "see stderr for details" is used as failure message. This is better than before because it avoids the redundancy. --- hack/make-rules/verify.sh | 4 ++-- hack/verify-golangci-lint.sh | 19 ++++++++++++++++--- third_party/forked/shell2junit/sh2ju.sh | 22 ++++++++++++++++++++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/hack/make-rules/verify.sh b/hack/make-rules/verify.sh index dbcdf577a79..30a9a5bf7db 100755 --- a/hack/make-rules/verify.sh +++ b/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} diff --git a/hack/verify-golangci-lint.sh b/hack/verify-golangci-lint.sh index 5d4925ea899..7dd154ea05f 100755 --- a/hack/verify-golangci-lint.sh +++ b/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 diff --git a/third_party/forked/shell2junit/sh2ju.sh b/third_party/forked/shell2junit/sh2ju.sh index 5a136a99603..29a820ac9d1 100755 --- a/third_party/forked/shell2junit/sh2ju.sh +++ b/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,21 @@ function juLog() { # write the junit xml report ## failure tag - [[ ${err} = 0 ]] && failure="" || failure=" - + local 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=" + " + fi ## testcase tag content="${content}