From 6d26fecba3f8b4d57b583d30b7176c7f4b5ac1b1 Mon Sep 17 00:00:00 2001
From: Rex Fleischer <rfleischer@instructure.com>
Date: Thu, 16 Apr 2020 09:36:55 -0700
Subject: [PATCH] only send rspec/selenium failures when stages are considered
 failed

fixes: CCI-328
flag = none

Test-Plan:
force fail selenium/rspec tests to make sure the reports still send.
then make sure the reports dont send when build passes.

Change-Id: I912dbb85dbfd9354e4f6dbdab1329bde8a718415
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/234328
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: James Butters <jbutters@instructure.com>
Reviewed-by: Jacob Powell <spowell@instructure.com>
QA-Review: James Butters <jbutters@instructure.com>
Product-Review: Rex Fleischer <rfleischer@instructure.com>
---
 build/new-jenkins/groovy/reports.groovy   | 33 ++++++++++++-----------
 build/new-jenkins/groovy/rspec.groovy     |  7 +++--
 build/new-jenkins/groovy/successes.groovy |  7 +++++
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/build/new-jenkins/groovy/reports.groovy b/build/new-jenkins/groovy/reports.groovy
index 8f5aae4479c..3d14e2dc8b5 100644
--- a/build/new-jenkins/groovy/reports.groovy
+++ b/build/new-jenkins/groovy/reports.groovy
@@ -45,30 +45,35 @@ def publishSpecCoverageToS3(prefix, ci_node_total, coverage_type) {
 }
 
 def appendFailMessageReport(message, link) {
+  if (!env.GERRIT_CHANGE_NUMBER || !env.GERRIT_PATCHSET_NUMBER) {
+    echo "build not associated with a PS... not sending message"
+  }
   dir ("_buildmeta") {
-    if (!fileExists("failure_messages.txt")) {
-      sh "echo 'failure links:' >> failure_messages.txt"
+    def message_file = "failure-messages-${BUILD_NUMBER}.txt"
+    if (!fileExists(message_file)) {
+      sh "echo 'failure links:' >> $message_file"
     }
-    sh "echo '$message' >> failure_messages.txt"
-    sh "echo '$link' >> failure_messages.txt"
+    sh "echo '$message' >> $message_file"
+    sh "echo '$link' >> $message_file"
   }
   archiveArtifacts(artifacts: '_buildmeta/*')
 }
 
 def sendFailureMessageIfPresent() {
-  if (fileExists("_buildmeta/failure_messages.txt")) {
+  def message_file = "_buildmeta/failure-messages-${BUILD_NUMBER}.txt"
+  if (fileExists(message_file)) {
     echo "sending failure message"
-    sh "cat _buildmeta/failure_messages.txt"
+    sh "cat $message_file"
     if (!env.GERRIT_CHANGE_NUMBER || !env.GERRIT_PATCHSET_NUMBER) {
       echo "build not associated with a PS... not sending message"
     }
     else {
       load('build/new-jenkins/groovy/credentials.groovy').withGerritCredentials({
-        sh '''
-          gerrit_message=`cat _buildmeta/failure_messages.txt`
-          ssh -i "$SSH_KEY_PATH" -l "$SSH_USERNAME" -p $GERRIT_PORT \
-            $GERRIT_HOST gerrit review -m "'$gerrit_message'" $GERRIT_CHANGE_NUMBER,$GERRIT_PATCHSET_NUMBER
-        '''
+        sh """
+          gerrit_message=`cat $message_file`
+          ssh -i "\$SSH_KEY_PATH" -l "\$SSH_USERNAME" -p \$GERRIT_PORT \
+            \$GERRIT_HOST gerrit review -m "'\$gerrit_message'" \$GERRIT_CHANGE_NUMBER,\$GERRIT_PATCHSET_NUMBER
+        """
       })
     }
   }
@@ -105,10 +110,7 @@ def publishSpecFailuresAsHTML(prefix, ci_node_total, report_title) {
   }
 
   def report_name = "spec-failure-$prefix"
-  if (htmlFiles.size() > 1) {
-    def url_name = java.net.URLEncoder.encode(report_name, "UTF-8")
-    appendFailMessageReport("$report_title:", "${BUILD_URL}${url_name}")
-  }
+  def report_url = "${BUILD_URL}${report_name}"
   archiveArtifacts(artifacts: "$working_dir/**")
   publishHTML target: [
     allowMissing: false,
@@ -120,6 +122,7 @@ def publishSpecFailuresAsHTML(prefix, ci_node_total, report_title) {
     reportTitles: report_title
   ]
   sh "rm -vrf ./$working_dir"
+  return report_url
 }
 
 def buildIndexPage() {
diff --git a/build/new-jenkins/groovy/rspec.groovy b/build/new-jenkins/groovy/rspec.groovy
index 072289f55fd..d3f36999e58 100644
--- a/build/new-jenkins/groovy/rspec.groovy
+++ b/build/new-jenkins/groovy/rspec.groovy
@@ -127,8 +127,11 @@ def uploadRSpecFailures() {
 }
 
 def _uploadSpecFailures(prefix, total, test_name) {
-  def reports = load 'build/new-jenkins/groovy/reports.groovy'
-  reports.publishSpecFailuresAsHTML(prefix, total, test_name)
+  def reports = load('build/new-jenkins/groovy/reports.groovy')
+  def report_url = reports.publishSpecFailuresAsHTML(prefix, total, test_name)
+  if (!load('build/new-jenkins/groovy/successes.groovy').hasSuccessOrBuildIsSuccessful(prefix, total)) {
+    reports.appendFailMessageReport("Spec Failure For $prefix", report_url)
+  }
 }
 
 return this
diff --git a/build/new-jenkins/groovy/successes.groovy b/build/new-jenkins/groovy/successes.groovy
index 3a3e88da626..91aef4d419c 100644
--- a/build/new-jenkins/groovy/successes.groovy
+++ b/build/new-jenkins/groovy/successes.groovy
@@ -24,6 +24,13 @@ def successFile() {
   return "_buildmeta/${env.GERRIT_CHANGE_NUMBER}-${env.GERRIT_PATCHSET_NUMBER}-successes"
 }
 
+// we have a stage that marks a build as successful. this removes
+// all success marks and replaces it with the 'build' mark. this
+// causes the post hooks to be unable to read successes.
+def hasSuccessOrBuildIsSuccessful(name, required_count = 1) {
+  return hasSuccess('build') || hasSuccess(name, required_count)
+}
+
 def hasSuccess(name, required_count = 1) {
   if (!fileExists(successFile())) {
     copyArtifacts(