Crash in tests if anybody tries to log remotely. Remote logging usually indicates an error, and tests shouldn't normally have such errors.

--
MOS_MIGRATED_REVID=99789434

2 files changed, 51 insertions, 0 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 8afb03a17c..8225fce736 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -30,6 +30,7 @@ import com.google.common.collect.Sets;
 import com.google.common.eventbus.EventBus;
 import com.google.common.eventbus.SubscriberExceptionContext;
 import com.google.common.eventbus.SubscriberExceptionHandler;
+import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.Uninterruptibles;
 import com.google.devtools.build.lib.actions.PackageRootResolver;
 import com.google.devtools.build.lib.actions.cache.ActionCache;
@@ -138,9 +139,12 @@ import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.UUID;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Handler;
 import java.util.logging.Level;
+import java.util.logging.LogRecord;
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
 
@@ -1489,6 +1493,11 @@ public final class BlazeRuntime {
                 ? new BlazeRuntime.BugReportingExceptionHandler()
                 : new BlazeRuntime.RemoteExceptionHandler());
 
+    if (System.getenv("TEST_TMPDIR") != null
+        && System.getenv("NO_CRASH_ON_LOGGING_IN_TEST") == null) {
+      LoggingUtil.installRemoteLogger(getTestCrashLogger());
+    }
+
     for (BlazeModule blazeModule : blazeModules) {
       runtimeBuilder.addBlazeModule(blazeModule);
     }
@@ -1499,6 +1508,40 @@ public final class BlazeRuntime {
   }
 
   /**
+   * Returns a logger that crashes as soon as it's written to, since tests should not cause events
+   * that would be logged.
+   */
+  @VisibleForTesting
+  public static Future<Logger> getTestCrashLogger() {
+    Logger crashLogger = Logger.getAnonymousLogger();
+    crashLogger.addHandler(
+        new Handler() {
+          @Override
+          public void publish(LogRecord record) {
+            throw new IllegalStateException(
+                record.getSourceClassName()
+                    + "#"
+                    + record.getSourceMethodName()
+                    + ": "
+                    + record.getMessage()
+                    + "\n"
+                    + record.getThrown());
+          }
+
+          @Override
+          public void flush() {
+            throw new IllegalStateException();
+          }
+
+          @Override
+          public void close() {
+            throw new IllegalStateException();
+          }
+        });
+    return Futures.immediateFuture(crashLogger);
+  }
+
+  /**
    * Make sure async threads cannot be orphaned. This method makes sure bugs are reported to
    * telemetry and the proper exit code is reported.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/util/LoggingUtil.java b/src/main/java/com/google/devtools/build/lib/util/LoggingUtil.java
index 5170727c6e..6edcd004e5 100644
--- a/src/main/java/com/google/devtools/build/lib/util/LoggingUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/util/LoggingUtil.java
@@ -46,6 +46,14 @@ public final class LoggingUtil {
     remoteLogger = logger;
   }
 
+  /**
+   * Installs the remote logger. Same as {@link #installRemoteLogger}, but since multiple tests will
+   * run in the same JVM, does not assert that this is the first time the logger is being installed.
+   */
+  public static synchronized void installRemoteLoggerForTesting(Future<Logger> logger) {
+    remoteLogger = logger;
+  }
+
   /** Returns the installed logger, or null if none is installed. */
   public static synchronized Logger getRemoteLogger() {
     try {