Delete EventHandlerPreconditions. With https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java#L1097, we log exceptions in the EventBus already. No need to have a crusty auxiliary class.

PiperOrigin-RevId: 161707461

3 files changed, 12 insertions, 154 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
index a9415b2ced..9892e7f2c9 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
@@ -35,7 +35,6 @@ import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.TestFilteringCompleteEvent;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
-import com.google.devtools.build.lib.events.ExceptionListener;
 import com.google.devtools.build.lib.rules.test.TestAttempt;
 import com.google.devtools.build.lib.rules.test.TestProvider;
 import com.google.devtools.build.lib.rules.test.TestResult;
@@ -57,7 +56,6 @@ public class AggregatingTestListener {
 
   private final TestResultAnalyzer analyzer;
   private final EventBus eventBus;
-  private final EventHandlerPreconditions preconditionHelper;
   private volatile boolean blazeHalted = false;
 
   // summaryLock guards concurrent access to these two collections, which should be kept
@@ -66,12 +64,9 @@ public class AggregatingTestListener {
   private final Multimap<LabelAndConfiguration, Artifact> remainingRuns;
   private final Object summaryLock = new Object();
 
-  public AggregatingTestListener(TestResultAnalyzer analyzer,
-                                 EventBus eventBus,
-                                 ExceptionListener listener) {
+  public AggregatingTestListener(TestResultAnalyzer analyzer, EventBus eventBus) {
     this.analyzer = analyzer;
     this.eventBus = eventBus;
-    this.preconditionHelper = new EventHandlerPreconditions(listener);
 
     this.summaries = Maps.newHashMap();
     this.remainingRuns = HashMultimap.create();
@@ -96,14 +91,20 @@ public class AggregatingTestListener {
       for (ConfiguredTarget target : event.getTestTargets()) {
         Iterable<Artifact> statusArtifacts =
             target.getProvider(TestProvider.class).getTestParams().getTestStatusArtifacts();
-        preconditionHelper.checkState(remainingRuns.putAll(asKey(target), statusArtifacts));
+        Preconditions.checkState(
+            remainingRuns.putAll(asKey(target), statusArtifacts),
+            "target: %s, statusArtifacts: %s",
+            target,
+            statusArtifacts);
 
         // And create an empty summary suitable for incremental analysis.
         // Also has the nice side effect of mapping labels to RuleConfiguredTargets.
         TestSummary.Builder summary = TestSummary.newBuilder()
             .setTarget(target)
             .setStatus(BlazeTestStatus.NO_STATUS);
-        preconditionHelper.checkState(summaries.put(asKey(target), summary) == null);
+        TestSummary.Builder oldSummary = summaries.put(asKey(target), summary);
+        Preconditions.checkState(
+            oldSummary == null, "target: %s, summaries: %s %s", target, oldSummary, summary);
       }
     }
   }
@@ -132,7 +133,7 @@ public class AggregatingTestListener {
     TestSummary finalTestSummary = null;
     synchronized (summaryLock) {
       TestSummary.Builder summary = summaries.get(targetLabel);
-      preconditionHelper.checkNotNull(summary);
+      Preconditions.checkNotNull(summary);
       if (!remainingRuns.remove(targetLabel, result.getTestStatusArtifact())) {
         // This can happen if a buildCompleteEvent() was processed before this event reached us.
         // This situation is likely to happen if --notest_keep_going is set with multiple targets.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/EventHandlerPreconditions.java b/src/main/java/com/google/devtools/build/lib/runtime/EventHandlerPreconditions.java
deleted file mode 100644
index 2c786a0fa8..0000000000
--- a/src/main/java/com/google/devtools/build/lib/runtime/EventHandlerPreconditions.java
+++ /dev/null
@@ -1,143 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.runtime;
-
-import com.google.devtools.build.lib.events.ExceptionListener;
-import com.google.devtools.build.lib.util.LoggingUtil;
-import com.google.devtools.build.lib.util.Preconditions;
-
-import java.util.logging.Level;
-
-/**
- * Reports precondition failures from within an event handler.
- * Necessary because the EventBus silently ignores exceptions thrown from within a handler.
- * This class logs the exceptions and creates some noise when a precondition check fails.
- */
-public class EventHandlerPreconditions {
-
-  private final ExceptionListener listener;
-
-  /**
-   * Creates a new precondition helper which outputs errors to the given reporter.
-   */
-  public EventHandlerPreconditions(ExceptionListener listener) {
-    this.listener = listener;
-  }
-
-  /**
-   * Verifies that the given condition (a check on an argument) is true,
-   * throwing an IllegalArgumentException if not.
-   *
-   * @param condition a condition to check for truth.
-   * @throws IllegalArgumentException if the condition is false.
-   */
-  @SuppressWarnings("unused")
-  public void checkArgument(boolean condition) {
-    checkArgument(condition, null);
-  }
-
-  /**
-   * Verifies that the given condition (a check on an argument) is true,
-   * throwing an IllegalArgumentException with the given message if not.
-   *
-   * @param condition a condition to check for truth.
-   * @param message extra information to output if the condition is false.
-   * @throws IllegalArgumentException if the condition is false.
-   */
-  public void checkArgument(boolean condition, String message) {
-    try {
-      Preconditions.checkArgument(condition, message);
-    } catch (IllegalArgumentException iae) {
-      String error = "Event handler argument check failed";
-      LoggingUtil.logToRemote(Level.SEVERE, error, iae);
-      listener.error(null, error, iae);
-      throw iae; // Still terminate the handler.
-    }
-  }
-
-  /**
-   * Verifies that the given condition (a check against the program's current state) is true,
-   * throwing an IllegalStateException if not.
-   *
-   * @param condition a condition to check for truth.
-   * @throws IllegalStateException if the condition is false.
-   */
-  public void checkState(boolean condition) {
-    checkState(condition, null);
-  }
-
-  /**
-   * Verifies that the given condition (a check against the program's current state) is true,
-   * throwing an IllegalStateException with the given message if not.
-   *
-   * @param condition a condition to check for truth.
-   * @param message extra information to output if the condition is false.
-   * @throws IllegalStateException if the condition is false.
-   */
-  public void checkState(boolean condition, String message) {
-    try {
-      Preconditions.checkState(condition, message);
-    } catch (IllegalStateException ise) {
-      String error = "Event handler state check failed";
-      LoggingUtil.logToRemote(Level.SEVERE, error, ise);
-      listener.error(null, error, ise);
-      throw ise; // Still terminate the handler.
-    }
-  }
-
-  /**
-   * Fails with an IllegalStateException when invoked.
-   */
-  public void fail(String message) {
-    String error = "Event handler failed: " + message;
-    IllegalStateException ise = new IllegalStateException(message);
-    LoggingUtil.logToRemote(Level.SEVERE, error, ise);
-    listener.error(null, error, ise);
-    throw ise;
-  }
-
-  /**
-   * Verifies that the given argument is not null, throwing a NullPointerException if it is null.
-   * Returns the original argument or throws.
-   *
-   * @param object an object to test for null.
-   * @return the reference which was checked.
-   * @throws NullPointerException if the object is null.
-   */
-  public <T> T checkNotNull(T object) {
-    return checkNotNull(object, null);
-  }
-
-  /**
-   * Verifies that the given argument is not null, throwing a
-   * NullPointerException with the given message if it is null.
-   * Returns the original argument or throws.
-   *
-   * @param object an object to test for null.
-   * @param message extra information to output if the object is null.
-   * @return the reference which was checked.
-   * @throws NullPointerException if the object is null.
-   */
-  public <T> T checkNotNull(T object, String message) {
-    try {
-      return Preconditions.checkNotNull(object, message);
-    } catch (NullPointerException npe) {
-      String error = "Event handler not-null check failed";
-      LoggingUtil.logToRemote(Level.SEVERE, error, npe);
-      listener.error(null, error, npe);
-      throw npe;
-    }
-  }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
index 02e7a7a9b9..f313ab2b53 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
@@ -97,8 +97,8 @@ public class TestCommand implements BlazeCommand {
         options.getOptions(BlazeCommandEventHandler.Options.class).useColor());
 
     // Initialize test handler.
-    AggregatingTestListener testListener = new AggregatingTestListener(
-        resultAnalyzer, env.getEventBus(), env.getReporter());
+    AggregatingTestListener testListener =
+        new AggregatingTestListener(resultAnalyzer, env.getEventBus());
 
     env.getEventBus().register(testListener);
     return doTest(env, options, testListener);