Check logs for errors at smoke tests cleanup

Author: smola

dd-smoke-tests/armeria-grpc/src/test/groovy/datadog/smoketest/ArmeriaSmokeTest.groovy

@@ -57,8 +57,7 @@ class ArmeriaSmokeTest extends AbstractServerSmokeTest {
     })
     waitForTraceCount(totalInvocations) >= totalInvocations
     validateLogInjection() == totalInvocations
-    checkLogPostExit()
-    !logHasErrors
+    assertNoErrorLogs()
   }
 
   void doAndValidateRequest(int id) {

dd-smoke-tests/custom-systemloader/src/test/groovy/datadog/smoketest/CustomSystemLoaderSmokeTest.groovy

@@ -30,20 +30,21 @@ class CustomSystemLoaderSmokeTest extends AbstractSmokeTest {
   def "resource types loaded by custom system class-loader are transformed"() {
     when:
     testedProcess.waitFor(TIMEOUT_SECS, SECONDS)
+
+    then:
+    testedProcess.exitValue() == 0
     int loadedResources = 0
     int transformedResources = 0
-    checkLogPostExit {
+    assertNoErrorLogs { String it ->
       if (it =~ /Loading sample.app.Resource[$]Test[1-3] from TestLoader/) {
         loadedResources++
       }
       if (it =~ /Transformed.*class=sample.app.Resource[$]Test[1-3].*classloader=datadog.smoketest.systemloader.TestLoader/) {
         transformedResources++
       }
+      false
     }
-    then:
-    testedProcess.exitValue() == 0
     loadedResources == 3
     transformedResources == 3
-    !logHasErrors
   }
 }

dd-smoke-tests/src/main/groovy/datadog/smoketest/AbstractSmokeTest.groovy

@@ -243,6 +243,7 @@ abstract class AbstractSmokeTest extends ProcessManager {
 
   def cleanupSpec() {
     stopServer()
+    assertNoErrorLogs()
   }
 
   def startServer() {

dd-smoke-tests/src/main/groovy/datadog/smoketest/ProcessManager.groovy

@@ -252,6 +252,49 @@ abstract class ProcessManager extends Specification {
     return "http://localhost:${profilingPort}/"
   }
 
+  /**
+   * Asserts that there are no errors printed by the application to the log.
+   */
+  void assertNoErrorLogs() {
+    assertNoErrorLogs({ false })
+  }
+
+  /**
+   * Checks if a log line is an error. This method may be overridden by test suites to consider additional messages.
+   * These will be checked on suite shutdown, or explicitly by calling {@link #assertNoErrorLogs()}.
+   */
+  boolean isErrorLog(String line) {
+    return line.contains("ERROR") || line.contains("ASSERTION FAILED")
+    || line.contains("Failed to handle exception in instrumentation")
+  }
+
+  /**
+   * Asserts that there are no errors printed by the application to the log.
+   * This should only be called after the process exits, otherwise it's not guaranteed that reading the log file will
+   * yield its final contents. Most tests should not need this, since it will be called at the end of every smoke test
+   * suite.
+   *
+   * @param isErrorLog Returns true if certain log line must be considered an error (in addition to defaults).
+   */
+  void assertNoErrorLogs(final Closure<Boolean> extraIsErrorLog) {
+    final Closure<Boolean> effectiveIsErrorLog = { String it -> isErrorLog(it) || extraIsErrorLog(it) }
+    final List<String> errorLogs = new ArrayList<>()
+    for (String lfp : logFilePaths) {
+      ProcessManager.eachLine(new File(lfp)) {
+        if (effectiveIsErrorLog(it)) {
+          errorLogs << it
+        }
+      }
+    }
+    if (!errorLogs.isEmpty()) {
+      final StringBuilder sb = new StringBuilder("Test application log contains ${errorLogs.size()} errors:\n")
+      errorLogs.eachWithIndex { String entry, int i ->
+        sb.append("${i + 1}: ${entry}\n")
+      }
+      assert errorLogs.isEmpty(), sb.toString()
+    }
+  }
+
   /**
    * Check the test application log and set {@linkplain AbstractSmokeTest#logHasErrors} variable
    *