GH-356: Write tests for JctCompilationFactoryImpl

- Add test pack for JctCompilationFactoryImpl.
- Add new helper test class for Mockito matchers used within
  collection-like types.
- Add a new method TeeWriter#getContent that gets the lines of content
  within a TeeWriter, as this is easier to stub than #toString due to
  how Mockito works internally.
- Add a new method TeeWriter#wrapOutputStream that wraps a given
  OutputStream and Charset in an OutputStreamWriter and inserts that
  into a new TeeWriter that is returned.
- Fix regression in JctCompilationFactoryImpl where the locale was not
  being set during compilations.
- Fix bug in JctCompilationFactory where the TeeWriter was not being flushed,
  so buffered content may not have been written to System.out properly
  in some edge cases.
- Improve performance of JctCompilationFactoryImpl for explicit class name
  lookup by avoiding creating a distinct collection that is iteratively
  resized with O(n) space complexity.

Author: ascopes

java-compiler-testing/src/main/java/io/github/ascopes/jct/compilers/impl/JctCompilationFactoryImpl.java

@@ -16,7 +16,6 @@
 package io.github.ascopes.jct.compilers.impl;
 
 import static java.util.Objects.requireNonNull;
-import static java.util.function.Function.identity;
 import static java.util.stream.Collectors.toList;
 
 import io.github.ascopes.jct.compilers.JctCompilation;
@@ -30,7 +29,6 @@
 import io.github.ascopes.jct.filemanagers.PathFileObject;
 import io.github.ascopes.jct.utils.IterableUtils;
 import java.io.IOException;
-import java.io.OutputStreamWriter;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
@@ -73,9 +71,11 @@ public JctCompilation createCompilation(
   ) {
     try {
       return createCheckedCompilation(flags, fileManager, jsr199Compiler, classNames);
+
     } catch (JctCompilerException ex) {
-      // Fall through, do not rewrap these.
+      // Rethrow JctCompilerExceptions -- we don't want to wrap these again.
       throw ex;
+
     } catch (Exception ex) {
       throw new JctCompilerException(
           "Failed to perform compilation, an unexpected exception was raised", ex
@@ -92,13 +92,11 @@ private JctCompilation createCheckedCompilation(
     var compilationUnits = findFilteredCompilationUnits(fileManager, classNames);
 
     if (compilationUnits.isEmpty()) {
-      throw classNames == null
-          ? new JctCompilerException("No compilation units were found in the given workspace")
-          : new JctCompilerException("No compilation units were found for the given class names");
+      throw new JctCompilerException("No compilation units were found in the given workspace");
     }
 
     // Do not close stdout, it breaks test engines, especially IntellIJ.
-    var writer = new TeeWriter(new OutputStreamWriter(System.out, compiler.getLogCharset()));
+    var writer = TeeWriter.wrapOutputStream(System.out, compiler.getLogCharset());
 
     var diagnosticListener = new TracingDiagnosticListener<>(
         compiler.getDiagnosticLoggingMode() != LoggingMode.DISABLED,
@@ -119,6 +117,8 @@ private JctCompilation createCheckedCompilation(
       task.setProcessors(processors);
     }
 
+    task.setLocale(compiler.getLocale());
+
     LOGGER
         .atInfo()
         .setMessage("Starting compilation with {} (found {} compilation units)")
@@ -133,6 +133,9 @@ private JctCompilation createCheckedCompilation(
     );
     var delta = (System.nanoTime() - start) / 1_000_000L;
 
+    // Ensure we commit the writer contents to the wrapped output stream in full.
+    writer.flush();
+
     LOGGER
         .atInfo()
         .setMessage("Compilation with {} {} after approximately {}ms (roughly {} classes/sec)")
@@ -149,16 +152,16 @@ private JctCompilation createCheckedCompilation(
 
     return JctCompilationImpl
         .builder()
-        .compilationUnits(compilationUnits)
+        .compilationUnits(Set.copyOf(compilationUnits))
         .fileManager(fileManager)
-        .outputLines(writer.toString().lines().collect(toList()))
+        .outputLines(writer.getContent().lines().collect(toList()))
         .diagnostics(diagnosticListener.getDiagnostics())
         .success(success)
         .failOnWarnings(compiler.isFailOnWarnings())
         .build();
   }
 
-  private Set<JavaFileObject> findFilteredCompilationUnits(
+  private Collection<JavaFileObject> findFilteredCompilationUnits(
       JctFileManager fileManager,
       @Nullable Collection<String> classNames
   ) throws IOException {
@@ -175,7 +178,9 @@ private Set<JavaFileObject> findFilteredCompilationUnits(
     return filterCompilationUnitsByBinaryNames(compilationUnits, classNames);
   }
 
-  private Set<JavaFileObject> findCompilationUnits(JctFileManager fileManager) throws IOException {
+  private Collection<JavaFileObject> findCompilationUnits(
+      JctFileManager fileManager
+  ) throws IOException {
     var locations = IterableUtils
         .flatten(fileManager.listLocationsForModules(StandardLocation.MODULE_SOURCE_PATH));
 
@@ -199,8 +204,8 @@ private Set<JavaFileObject> findCompilationUnits(JctFileManager fileManager) thr
     return objects;
   }
 
-  private Set<JavaFileObject> filterCompilationUnitsByBinaryNames(
-      Set<JavaFileObject> compilationUnits,
+  private Collection<JavaFileObject> filterCompilationUnitsByBinaryNames(
+      Collection<JavaFileObject> compilationUnits,
       Collection<String> classNames
   ) {
     var binaryNamesToCompilationUnits = compilationUnits
@@ -209,30 +214,27 @@ private Set<JavaFileObject> filterCompilationUnitsByBinaryNames(
         // care too much as the implementation should conform to this anyway. We just cannot enforce
         // it due to covariance rules.
         .map(PathFileObject.class::cast)
+        .filter(fo -> classNames.contains(fo.getBinaryName()))
         .collect(Collectors.toMap(
             PathFileObject::getBinaryName,
-            identity()
+            JavaFileObject.class::cast
         ));
 
-    var filteredCompilationUnits = new HashSet<JavaFileObject>();
-
     for (var className : classNames) {
       var compilationUnit = binaryNamesToCompilationUnits.get(className);
       if (compilationUnit == null) {
         throw new JctCompilerException(
             "No compilation unit matching " + className + " found in the provided sources"
         );
       }
-
-      filteredCompilationUnits.add(compilationUnit);
     }
 
     LOGGER.atDebug()
         .setMessage("Filtered {} candidate compilation units down to {} final compilation units")
         .addArgument(compilationUnits::size)
-        .addArgument(filteredCompilationUnits::size)
+        .addArgument(binaryNamesToCompilationUnits::size)
         .log();
 
-    return filteredCompilationUnits;
+    return binaryNamesToCompilationUnits.values();
   }
 }

java-compiler-testing/src/main/java/io/github/ascopes/jct/diagnostics/TeeWriter.java

@@ -16,9 +16,13 @@
 package io.github.ascopes.jct.diagnostics;
 
 import static java.util.Objects.requireNonNull;
+import static java.util.Objects.requireNonNullElse;
 
 import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
 import java.io.Writer;
+import java.nio.charset.Charset;
 import org.apiguardian.api.API;
 import org.apiguardian.api.API.Status;
 
@@ -80,13 +84,31 @@ public void flush() throws IOException {
     }
   }
 
-  @Override
-  public String toString() {
+  /**
+   * Get the content of the internal buffer.
+   *
+   * @return the content.
+   * @since 0.2.1
+   */
+  @API(since = "0.2.1", status = Status.STABLE)
+  public String getContent() {
     synchronized (lock) {
       return builder.toString();
     }
   }
 
+  /**
+   * Get the content of the internal buffer.
+   *
+   * <p>This calls {@link #getContent()} internally as of 0.2.1.
+   *
+   * @return the content.
+   */
+  @Override
+  public String toString() {
+    return getContent();
+  }
+
   @Override
   public void write(char[] cbuf, int off, int len) throws IOException {
     synchronized (lock) {
@@ -104,4 +126,23 @@ private void ensureOpen() {
       throw new IllegalStateException("TeeWriter is closed");
     }
   }
+
+  /**
+   * Create a tee writer for the given output stream.
+   *
+   * <p>Remember you may need to manually flush the tee writer for all contents to be committed to
+   * the output stream.
+   *
+   * @param outputStream the output stream.
+   * @param charset the charset.
+   * @return the Tee Writer.
+   * @since 0.2.1
+   */
+  @API(since = "0.2.1", status = Status.STABLE)
+  public static TeeWriter wrapOutputStream(OutputStream outputStream, Charset charset) {
+    requireNonNull(outputStream, "outputStream");
+    requireNonNull(charset, "charset");
+    var writer = new OutputStreamWriter(outputStream, charset);
+    return new TeeWriter(writer);
+  }
 }

java-compiler-testing/src/test/java/io/github/ascopes/jct/tests/helpers/ExtraArgumentMatchers.java

@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2022 - 2023, the original author or authors.
+ *
+ * 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 io.github.ascopes.jct.tests.helpers;
+
+import static org.mockito.ArgumentMatchers.argThat;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Set;
+import org.mockito.ArgumentMatcher;
+
+/**
+ * Extra Mockito argument matchers.
+ *
+ * @author Ashley Scopes
+ */
+public final class ExtraArgumentMatchers {
+
+  private ExtraArgumentMatchers() {
+    throw new UnsupportedOperationException("static-only class");
+  }
+
+  @SafeVarargs
+  public static <E, T extends Iterable<E>> T containsExactlyElements(E... expected) {
+    return containsExactlyElements(Set.of(expected));
+  }
+
+  public static <E, T extends Iterable<E>> T containsExactlyElements(Collection<E> expected) {
+    return argThat(new ArgumentMatcher<>() {
+      @Override
+      public String toString() {
+        return "containsExactlyElements(" + expected + ")";
+      }
+
+      @Override
+      public boolean matches(T actualIterable) {
+        if (actualIterable == null) {
+          return false;
+        }
+
+        var actualElements = new ArrayList<E>();
+        actualIterable.forEach(actualElements::add);
+
+        // All expected are in actual
+        for (var expectedElement : expected) {
+          if (!actualElements.contains(expectedElement)) {
+            throw new IllegalArgumentException(
+                "Expected element " + expectedElement + " was not in the actual collection"
+            );
+          }
+        }
+
+        // All actual are in expected.
+        var expectedSet = Set.copyOf(expected);
+        for (var actualElement : actualElements) {
+          if (!expectedSet.contains(actualElement)) {
+            throw new IllegalArgumentException(
+                "Actual element " + actualElement + " was not in the expected elements array"
+            );
+          }
+        }
+
+        return true;
+      }
+    });
+  }
+}