Clean up AbstractQueueVisitor's constructors.

The "concurrent" bit was supposedly around for testing purposes, but who knows if it even works anymore. Making other callsites explicitly state their ErrorClassifier gets us down to two constructors, one of which can delegate to the other.

I think having both these constructors is useful because there's a linkage between creating a new executor service and specifying that the AQV should shut down the service at the end of the visitation. And using a static create() method doesn't work because of AQV's inheritance model.

PiperOrigin-RevId: 155406771

1 files changed, 30 insertions, 11 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java b/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java
index 7be04c4547..5aa63dd94c 100644
--- a/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java
@@ -462,7 +462,6 @@ public class AbstractQueueVisitorTest {
     };
     AbstractQueueVisitor visitor =
         new AbstractQueueVisitor(
-            /*concurrent=*/ true,
             executor,
             /*shutdownOnCompletion=*/ true,
             /*failFastOnException=*/ false,
@@ -557,11 +556,18 @@ public class AbstractQueueVisitorTest {
     private final Object lock = new Object();
 
     public CountingQueueVisitor() {
-      super(/*parallelism=*/ 5, /*keepAlive=*/ 3L, TimeUnit.SECONDS, THREAD_NAME);
+      super(
+          /*parallelism=*/ 5,
+          /*keepAlive=*/ 3L,
+          TimeUnit.SECONDS,
+          /*failFast=*/ false,
+          THREAD_NAME,
+          AbstractQueueVisitor.EXECUTOR_FACTORY,
+          ErrorClassifier.DEFAULT);
     }
 
-    public CountingQueueVisitor(ThreadPoolExecutor executor) {
-      super(executor, false, true);
+    CountingQueueVisitor(ThreadPoolExecutor executor) {
+      super(executor, false, true, ErrorClassifier.DEFAULT);
     }
 
     public void enqueue() {
@@ -588,23 +594,36 @@ public class AbstractQueueVisitorTest {
 
     private final static String THREAD_NAME = "BlazeTest ConcreteQueueVisitor";
 
-    public ConcreteQueueVisitor() {
-      super(5, 3L, TimeUnit.SECONDS, THREAD_NAME);
+    ConcreteQueueVisitor() {
+      super(
+          5,
+          3L,
+          TimeUnit.SECONDS,
+          /*failFast=*/ false,
+          THREAD_NAME,
+          AbstractQueueVisitor.EXECUTOR_FACTORY,
+          ErrorClassifier.DEFAULT);
     }
 
-    public ConcreteQueueVisitor(boolean failFast) {
-      super(true, 5, 3L, TimeUnit.SECONDS, failFast, THREAD_NAME);
+    ConcreteQueueVisitor(boolean failFast) {
+      super(
+          5,
+          3L,
+          TimeUnit.SECONDS,
+          failFast,
+          THREAD_NAME,
+          AbstractQueueVisitor.EXECUTOR_FACTORY,
+          ErrorClassifier.DEFAULT);
     }
 
-    public ConcreteQueueVisitor(ThreadPoolExecutor executor, boolean failFast) {
-      super(executor, /*shutdownOnCompletion=*/ false, failFast);
+    ConcreteQueueVisitor(ThreadPoolExecutor executor, boolean failFast) {
+      super(executor, /*shutdownOnCompletion=*/ false, failFast, ErrorClassifier.DEFAULT);
     }
   }
 
   private static AbstractQueueVisitor createQueueVisitorWithConstantErrorClassification(
       ThreadPoolExecutor executor, final ErrorClassification classification) {
     return new AbstractQueueVisitor(
-        /*concurrent=*/ true,
         executor,
         /*shutdownOnCompletion=*/ true,
         /*failFastOnException=*/ false,