Skip to content

Commit

Permalink
Migrate *TestHelpers to in-memory file objects and the cached filem…
Browse files Browse the repository at this point in the history 
…anager

to improve test performance.

PiperOrigin-RevId: 348537738
  • Loading branch information
@cushon
cushon authored and Error Prone Team committed Dec 21, 2020
1 parent d15d48a commit 7f4d76b
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 45 deletions.
43 changes: 16 additions & 27 deletions test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import static com.google.common.truth.Truth.assertAbout;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.errorprone.FileObjects.forResource;
import static com.google.errorprone.FileObjects.forSourceLines;
import static com.google.testing.compile.JavaSourceSubjectFactory.javaSource;
import static org.junit.Assert.fail;

Expand Down Expand Up @@ -56,7 +58,7 @@
import java.io.PrintWriter;
import java.io.StringWriter;
import java.io.UncheckedIOException;
import java.nio.file.FileAlreadyExistsException;
import java.net.URI;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
Expand Down Expand Up @@ -142,8 +144,8 @@ public Fix choose(List<Fix> fixes) {
}

private final Map<JavaFileObject, JavaFileObject> sources = new HashMap<>();
private final Class<?> clazz;
private final ErrorProneScanner scanner;
private final ErrorProneInMemoryFileManager fileManager;

private FixChooser fixChooser = FixChoosers.FIRST;
private List<String> options = ImmutableList.of();
Expand All @@ -153,7 +155,7 @@ public Fix choose(List<Fix> fixes) {
private boolean run = false;

private BugCheckerRefactoringTestHelper(Class<?> clazz, ErrorProneScanner scanner) {
this.fileManager = new ErrorProneInMemoryFileManager(clazz);
this.clazz = clazz;
this.scanner = scanner;
}

Expand All @@ -179,13 +181,11 @@ public static BugCheckerRefactoringTestHelper newInstance(
}

public BugCheckerRefactoringTestHelper.ExpectOutput addInput(String inputFilename) {
return new ExpectOutput(fileManager.forResource(inputFilename));
return new ExpectOutput(forResource(clazz, inputFilename));
}

public BugCheckerRefactoringTestHelper.ExpectOutput addInputLines(String path, String... input) {
String inputPath = getPath("in/", path);
assertThat(fileManager.exists(inputPath)).isFalse();
return new ExpectOutput(fileManager.forSourceLines(inputPath, input));
return new ExpectOutput(forSourceLines(path, input));
}

public BugCheckerRefactoringTestHelper setFixChooser(FixChooser chooser) {
Expand Down Expand Up @@ -274,26 +274,26 @@ private JCCompilationUnit doCompile(
throw new IllegalArgumentException("Exception during argument processing: " + e);
}
context.put(ErrorProneOptions.class, errorProneOptions);
fileManager.createAndInstallTempFolderForOutput();
StringWriter out = new StringWriter();
JavacTaskImpl task =
(JavacTaskImpl)
tool.getTask(
new PrintWriter(out, true),
fileManager,
FileManagers.testFileManager(),
diagnosticsCollector,
ImmutableList.copyOf(errorProneOptions.getRemainingArgs()),
/*classes=*/ null,
files,
context);
Iterable<? extends CompilationUnitTree> trees = task.parse();
task.analyze();
ImmutableMap<JavaFileObject, ? extends CompilationUnitTree> bySource =
stream(trees).collect(toImmutableMap(t -> t.getSourceFile(), t -> t));
ImmutableMap<URI, ? extends CompilationUnitTree> byURI =
stream(trees).collect(toImmutableMap(t -> t.getSourceFile().toUri(), t -> t));
URI inputURI = input.toUri();
assertWithMessage(out + Joiner.on('\n').join(diagnosticsCollector.getDiagnostics()))
.that(bySource)
.containsKey(input);
JCCompilationUnit tree = (JCCompilationUnit) bySource.get(input);
.that(byURI)
.containsKey(inputURI);
JCCompilationUnit tree = (JCCompilationUnit) byURI.get(inputURI);
Iterable<Diagnostic<? extends JavaFileObject>> errorDiagnostics =
Iterables.filter(
diagnosticsCollector.getDiagnostics(), d -> d.getKind() == Diagnostic.Kind.ERROR);
Expand Down Expand Up @@ -348,29 +348,18 @@ private ExpectOutput(JavaFileObject input) {
}

public BugCheckerRefactoringTestHelper addOutputLines(String path, String... output) {
String outputPath = getPath("out/", path);
if (fileManager.exists(outputPath)) {
throw new UncheckedIOException(new FileAlreadyExistsException(outputPath));
}
return addInputAndOutput(input, fileManager.forSourceLines(outputPath, output));
return addInputAndOutput(input, forSourceLines(path, output));
}

public BugCheckerRefactoringTestHelper addOutput(String outputFilename) {
return addInputAndOutput(input, fileManager.forResource(outputFilename));
return addInputAndOutput(input, forResource(clazz, outputFilename));
}

public BugCheckerRefactoringTestHelper expectUnchanged() {
return addInputAndOutput(input, input);
}
}

private String getPath(String prefix, String path) {
// return prefix + path;
int insertAt = path.lastIndexOf('/');
insertAt = insertAt == -1 ? 0 : insertAt + 1;
return new StringBuilder(path).insert(insertAt, prefix + "/").toString();
}

private static void closeCompiler(Context context) {
JavaCompiler compiler = context.get(JavaCompiler.compilerKey);
if (compiler != null) {
Expand Down
27 changes: 11 additions & 16 deletions test_helpers/src/main/java/com/google/errorprone/CompilationTestHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.errorprone.FileObjects.forResource;
import static com.google.errorprone.FileObjects.forSourceLines;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Arrays.stream;
import static org.junit.Assert.fail;

import com.google.common.base.Joiner;
Expand All @@ -45,7 +48,6 @@
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
Expand All @@ -56,7 +58,6 @@
import javax.tools.JavaCompiler;
import javax.tools.JavaCompiler.CompilationTask;
import javax.tools.JavaFileObject;
import javax.tools.StandardLocation;

/** Helps test Error Prone bug checkers and compilations. */
@CheckReturnValue
Expand All @@ -78,7 +79,7 @@ public class CompilationTestHelper {
private final DiagnosticTestHelper diagnosticHelper;
private final BaseErrorProneJavaCompiler compiler;
private final ByteArrayOutputStream outputStream;
private final ErrorProneInMemoryFileManager fileManager;
private final Class<?> clazz;
private final List<JavaFileObject> sources = new ArrayList<>();
private ImmutableList<String> extraArgs = ImmutableList.of();
@Nullable private ImmutableList<Class<?>> overrideClasspath;
Expand All @@ -91,12 +92,7 @@ public class CompilationTestHelper {
private boolean run = false;

private CompilationTestHelper(ScannerSupplier scannerSupplier, String checkName, Class<?> clazz) {
this.fileManager = new ErrorProneInMemoryFileManager(clazz);
try {
fileManager.setLocation(StandardLocation.SOURCE_PATH, Collections.emptyList());
} catch (IOException e) {
throw new UncheckedIOException(e);
}
this.clazz = clazz;
this.diagnosticHelper = new DiagnosticTestHelper(checkName);
this.outputStream = new ByteArrayOutputStream();
this.compiler = new BaseErrorProneJavaCompiler(JavacTool.create(), scannerSupplier);
Expand Down Expand Up @@ -188,7 +184,7 @@ private static Optional<Path> getOverrideClasspath(@Nullable List<Class<?>> over
// TODO(eaftan): We could eliminate this path parameter and just infer the path from the
// package and class name
public CompilationTestHelper addSourceLines(String path, String... lines) {
this.sources.add(fileManager.forSourceLines(path, lines));
this.sources.add(forSourceLines(path, lines));
return this;
}

Expand All @@ -200,7 +196,7 @@ public CompilationTestHelper addSourceLines(String path, String... lines) {
* @param path the path to the source file
*/
public CompilationTestHelper addSourceFile(String path) {
this.sources.add(fileManager.forResource(path));
this.sources.add(forResource(clazz, path));
return this;
}

Expand All @@ -221,7 +217,7 @@ public CompilationTestHelper addModules(String... modules) {
return this;
}
return setArgs(
Arrays.stream(modules)
stream(modules)
.map(m -> String.format("--add-exports=%s=ALL-UNNAMED", m))
.collect(toImmutableList()));
}
Expand Down Expand Up @@ -359,13 +355,12 @@ private Result compile() {
if (checkWellFormed) {
checkWellFormed(sources, processedArgs);
}
fileManager.createAndInstallTempFolderForOutput();
return compiler
.getTask(
new PrintWriter(
new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8)),
/*autoFlush=*/ true),
fileManager,
FileManagers.testFileManager(),
diagnosticHelper.collector,
/* options= */ ImmutableList.copyOf(processedArgs),
/* classes= */ ImmutableList.of(),
Expand All @@ -375,8 +370,8 @@ private Result compile() {
: Result.ERROR;
}

// TODO(b/176108531): check for non-Error Prone diagnostics without compiling everything twice
private void checkWellFormed(Iterable<JavaFileObject> sources, List<String> args) {
fileManager.createAndInstallTempFolderForOutput();
JavaCompiler compiler = JavacTool.create();
OutputStream outputStream = new ByteArrayOutputStream();
List<String> remainingArgs = null;
Expand All @@ -390,7 +385,7 @@ private void checkWellFormed(Iterable<JavaFileObject> sources, List<String> args
new PrintWriter(
new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8)),
/*autoFlush=*/ true),
fileManager,
FileManagers.testFileManager(),
null,
remainingArgs,
null,
Expand Down
5 changes: 3 additions & 2 deletions test_helpers/src/main/java/com/google/errorprone/FileManagers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static com.google.common.base.StandardSystemProperty.JAVA_CLASS_PATH;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.Streams.stream;
import static java.lang.ThreadLocal.withInitial;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Arrays.stream;
Expand Down Expand Up @@ -90,8 +91,8 @@ public static JavacFileManager testFileManager() {

/** Returns the current runtime's classpath. */
private static ImmutableList<Path> systemClassPath() {
return Splitter.on(File.pathSeparatorChar)
.splitToStream(JAVA_CLASS_PATH.value())
// splitToStream isn't available if Android guava is on the classpath
return stream(Splitter.on(File.pathSeparatorChar).split(JAVA_CLASS_PATH.value()))
.map(Paths::get)
.collect(toImmutableList());
}
Expand Down

0 comments on commit 7f4d76b

Please sign in to comment.