From 8a8eb817b3e3477aa8c65d63d74ee880c422a58f Mon Sep 17 00:00:00 2001 From: David van Leusen Date: Sun, 7 Oct 2018 18:35:36 +0200 Subject: [PATCH 1/9] Add JUnit5 implementation of CompilationRule --- pom.xml | 19 ++ .../testing/compile/CompilationExtension.java | 244 ++++++++++++++++++ .../compile/CompilationExtensionTest.java | 80 ++++++ 3 files changed, 343 insertions(+) create mode 100644 src/main/java/com/google/testing/compile/CompilationExtension.java create mode 100644 src/test/java/com/google/testing/compile/CompilationExtensionTest.java diff --git a/pom.xml b/pom.xml index 979f423f..bbd2fd51 100644 --- a/pom.xml +++ b/pom.xml @@ -16,6 +16,7 @@ 0.42 + 5.3.1 http://github.com/google/compile-testing @@ -52,6 +53,24 @@ junit 4.12 + + org.junit.jupiter + junit-jupiter-api + ${junit5.version} + true + + + org.junit.platform + junit-platform-runner + 1.3.1 + test + + + org.junit.jupiter + junit-jupiter-engine + ${junit5.version} + test + com.google.truth truth diff --git a/src/main/java/com/google/testing/compile/CompilationExtension.java b/src/main/java/com/google/testing/compile/CompilationExtension.java new file mode 100644 index 00000000..231c0312 --- /dev/null +++ b/src/main/java/com/google/testing/compile/CompilationExtension.java @@ -0,0 +1,244 @@ +/* + * Copyright (C) 2018 Google, Inc. + * + * 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.testing.compile; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; +import static com.google.testing.compile.Compilation.Status.SUCCESS; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.Extension; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.ParameterContext; +import org.junit.jupiter.api.extension.ParameterResolutionException; +import org.junit.jupiter.api.extension.ParameterResolver; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Phaser; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; +import java.util.function.Supplier; +import javax.annotation.processing.AbstractProcessor; +import javax.annotation.processing.ProcessingEnvironment; +import javax.annotation.processing.RoundEnvironment; +import javax.lang.model.SourceVersion; +import javax.lang.model.element.TypeElement; +import javax.lang.model.util.Elements; +import javax.lang.model.util.Types; +import javax.tools.JavaFileObject; + +/** + * A Junit 5 {@link Extension} that extends a test suite such that an instance of {@link Elements} + * and {@link Types} are available through parameter injection during execution. + * + *

To use this extension, request it with {@link ExtendWith} and add the required parameters: + * + * 

+ * {@code @ExtendWith}(CompilationExtension.class)
+ * class CompilerTest {
+ *   {@code @Test} void testElements({@link Elements} elements, {@link Types} types) {
+ *     // Any methods of the supplied utility classes can now be accessed.
+ *   }
+ * }
+ *
+ * + * @author David van Leusen + */ +public class CompilationExtension + implements BeforeAllCallback, AfterAllCallback, ParameterResolver { + private static final JavaFileObject DUMMY = + JavaFileObjects.forSourceLines("Dummy", "final class Dummy {}"); + private static final ExtensionContext.Namespace NAMESPACE = + ExtensionContext.Namespace.create(CompilationExtension.class); + private static final Map, Function> SUPPORTED_PARAMETERS; + private static final Function RETURN_NULL = ignored -> null; + + private static final StoreAccessor PHASER_KEY = new StoreAccessor<>(Phaser.class); + private static final StoreAccessor> PROCESSINGENV_KEY = + new StoreAccessor<>(ProcessingEnvironment.class); + private static final StoreAccessor> RESULT_KEY = + new StoreAccessor<>(Compilation.class); + + static { + SUPPORTED_PARAMETERS = ImmutableMap., Function>builder() + .put(Elements.class, ProcessingEnvironment::getElementUtils) + .put(Types.class, ProcessingEnvironment::getTypeUtils) + .build(); + } + + @Override + public void beforeAll(ExtensionContext context) { + final Phaser sharedBarrier = new Phaser(2) { + @Override + protected boolean onAdvance(int phase, int parties) { + // Terminate the phaser once all parties have deregistered + return parties == 0; + } + }; + + final AtomicReference sharedState + = new AtomicReference<>(null); + + final CompletionStage futureResult = doOneShotExecution(() -> + Compiler.javac() + .withProcessors(new EvaluatingProcessor(sharedBarrier, sharedState)) + .compile(DUMMY) + ); + + final ExtensionContext.Store store = context.getStore(NAMESPACE); + PHASER_KEY.put(store, sharedBarrier); + PROCESSINGENV_KEY.put(store, sharedState); + RESULT_KEY.put(store, futureResult); + + // Wait until the processor is ready for testing + sharedBarrier.arriveAndAwaitAdvance(); + + checkState(!sharedBarrier.isTerminated(), "Phaser terminated early"); + } + + @Override + public void afterAll(ExtensionContext context) throws Exception { + final ExtensionContext.Store store = context.getStore(NAMESPACE); + final Phaser sharedPhaser = PHASER_KEY.get(store); + + // Allow the processor to finish + sharedPhaser.arriveAndDeregister(); + + // Perform status checks, since processing is 'over' almost instantly + final Compilation compilation = RESULT_KEY.get(store) + .toCompletableFuture().get(1, TimeUnit.SECONDS); + checkState(compilation.status().equals(SUCCESS), compilation); + + // Check precondition + checkState(sharedPhaser.isTerminated(), "Phaser not terminated"); + } + + @Override + public boolean supportsParameter( + ParameterContext parameterContext, + ExtensionContext extensionContext + ) throws ParameterResolutionException { + final Class parameterType = parameterContext.getParameter().getType(); + return SUPPORTED_PARAMETERS.containsKey(parameterType); + } + + @Override + public Object resolveParameter( + ParameterContext parameterContext, + ExtensionContext extensionContext + ) throws ParameterResolutionException { + final ExtensionContext.Store store = extensionContext.getStore(NAMESPACE); + final AtomicReference processingEnvironment + = PROCESSINGENV_KEY.get(store); + + return SUPPORTED_PARAMETERS.getOrDefault( + parameterContext.getParameter().getType(), + RETURN_NULL + ).apply(checkNotNull( + processingEnvironment.get(), + "ProcessingEnvironment not available: %s", + RESULT_KEY.get(store) + )); + } + + private CompletionStage doOneShotExecution(Supplier run) { + final ExecutorService service = Executors.newSingleThreadExecutor(); + final CompletionStage future = CompletableFuture.supplyAsync(run, service); + // Do not accept more actions, just execute the runnable and clean up + service.shutdown(); + return future; + } + + /** + * Utility class to safely access {@link ExtensionContext.Store} when dealing with + * parameterized types. + */ + static final class StoreAccessor { + private final Object key; + + StoreAccessor(Object key) { + this.key = key; + } + + @SuppressWarnings("unchecked") + R get(ExtensionContext.Store store) { + return (R) store.get(key); + } + + void put(ExtensionContext.Store store, R value) { + store.put(key, value); + } + } + + static final class EvaluatingProcessor extends AbstractProcessor { + private final Phaser barrier; + private final AtomicReference sharedState; + + EvaluatingProcessor( + Phaser barrier, + AtomicReference sharedState + ) { + this.barrier = barrier; + this.sharedState = sharedState; + } + + @Override + public SourceVersion getSupportedSourceVersion() { + return SourceVersion.latest(); + } + + @Override + public Set getSupportedAnnotationTypes() { + return ImmutableSet.of("*"); + } + + @Override + public synchronized void init(ProcessingEnvironment processingEnvironment) { + super.init(processingEnvironment); + + // Share the processing environment + if (!sharedState.compareAndSet(null, processingEnvironment)) { + // Invalid state, init() run twice + barrier.forceTermination(); + throw new IllegalStateException("Processor initialized twice"); + } + } + + @Override + public boolean process(Set annotations, RoundEnvironment roundEnv) { + if (roundEnv.processingOver()) { + // Synchronize on the beginning of the test run + barrier.arriveAndAwaitAdvance(); + + // Now wait until testing is over + barrier.awaitAdvance(barrier.arriveAndDeregister()); + + // Clean up the shared state + sharedState.getAndSet(null); + } + return false; + } + } +} diff --git a/src/test/java/com/google/testing/compile/CompilationExtensionTest.java b/src/test/java/com/google/testing/compile/CompilationExtensionTest.java new file mode 100644 index 00000000..530b48d8 --- /dev/null +++ b/src/test/java/com/google/testing/compile/CompilationExtensionTest.java @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2018 Google, Inc. + * + * 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.testing.compile; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.platform.runner.JUnitPlatform; +import org.junit.runner.RunWith; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import javax.lang.model.element.Element; +import javax.lang.model.element.TypeElement; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeMirror; +import javax.lang.model.util.Elements; +import javax.lang.model.util.Types; + +/** + * Tests the {@link CompilationExtension} by applying it to this test. + */ +@RunWith(JUnitPlatform.class) +@ExtendWith(CompilationExtension.class) +public class CompilationExtensionTest { + private final AtomicInteger executions = new AtomicInteger(); + + @Test public void testMethodsExecuteExactlyOnce() { + assertThat(executions.getAndIncrement()).isEqualTo(0); + } + + @BeforeEach /* we also make sure that getElements works in a @Before method */ + @Test public void getElements(Elements elements) { + assertThat(elements).isNotNull(); + } + + @BeforeEach /* we also make sure that getTypes works in a @Before method */ + @Test public void getTypes(Types types) { + assertThat(types).isNotNull(); + } + + /** + * Do some non-trivial operation with {@link Element} instances because they stop working after + * compilation stops. + */ + @Test public void elementsAreValidAndWorking(Elements elements) { + TypeElement stringElement = elements.getTypeElement(String.class.getName()); + assertThat(stringElement.getEnclosingElement()) + .isEqualTo(elements.getPackageElement("java.lang")); + } + + /** + * Do some non-trivial operation with {@link TypeMirror} instances because they stop working after + * compilation stops. + */ + @Test public void typeMirrorsAreValidAndWorking(Elements elements, Types types) { + DeclaredType arrayListOfString = types.getDeclaredType( + elements.getTypeElement(ArrayList.class.getName()), + elements.getTypeElement(String.class.getName()).asType()); + DeclaredType listOfExtendsObjectType = types.getDeclaredType( + elements.getTypeElement(List.class.getName()), + types.getWildcardType(elements.getTypeElement(Object.class.getName()).asType(), null)); + assertThat(types.isAssignable(arrayListOfString, listOfExtendsObjectType)).isTrue(); + } +} From 8b4b6fb469346f8ddb24dedee09683880697a43a Mon Sep 17 00:00:00 2001 From: David van Leusen Date: Sun, 7 Oct 2018 18:46:58 +0200 Subject: [PATCH 2/9] Use daemon threads to avoid potentially getting the JVM stuck waiting for a compilation that never finishes. --- .../testing/compile/CompilationExtension.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/google/testing/compile/CompilationExtension.java b/src/main/java/com/google/testing/compile/CompilationExtension.java index 231c0312..f3b64997 100644 --- a/src/main/java/com/google/testing/compile/CompilationExtension.java +++ b/src/main/java/com/google/testing/compile/CompilationExtension.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.ExtendWith; @@ -81,6 +82,10 @@ public class CompilationExtension private static final StoreAccessor> RESULT_KEY = new StoreAccessor<>(Compilation.class); + private static final ExecutorService COMPILER_EXECUTOR = Executors.newSingleThreadExecutor( + new ThreadFactoryBuilder().setDaemon(true).setNameFormat("async-compiler-%d").build() + ); + static { SUPPORTED_PARAMETERS = ImmutableMap., Function>builder() .put(Elements.class, ProcessingEnvironment::getElementUtils) @@ -101,11 +106,12 @@ protected boolean onAdvance(int phase, int parties) { final AtomicReference sharedState = new AtomicReference<>(null); - final CompletionStage futureResult = doOneShotExecution(() -> - Compiler.javac() - .withProcessors(new EvaluatingProcessor(sharedBarrier, sharedState)) - .compile(DUMMY) - ); + + final CompletionStage futureResult = CompletableFuture.supplyAsync(() -> + Compiler.javac() + .withProcessors(new EvaluatingProcessor(sharedBarrier, sharedState)) + .compile(DUMMY), + COMPILER_EXECUTOR); final ExtensionContext.Store store = context.getStore(NAMESPACE); PHASER_KEY.put(store, sharedBarrier); @@ -163,14 +169,6 @@ public Object resolveParameter( )); } - private CompletionStage doOneShotExecution(Supplier run) { - final ExecutorService service = Executors.newSingleThreadExecutor(); - final CompletionStage future = CompletableFuture.supplyAsync(run, service); - // Do not accept more actions, just execute the runnable and clean up - service.shutdown(); - return future; - } - /** * Utility class to safely access {@link ExtensionContext.Store} when dealing with * parameterized types. From 7e9693ccb44319a802df7f25953dab223305ff8f Mon Sep 17 00:00:00 2001 From: David van Leusen Date: Sun, 7 Oct 2018 19:27:47 +0200 Subject: [PATCH 3/9] If JUnit5 tries to resolve a parameter we cannot handle, it should throw an exception, not return null. --- .../testing/compile/CompilationExtension.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/testing/compile/CompilationExtension.java b/src/main/java/com/google/testing/compile/CompilationExtension.java index f3b64997..42c4b45a 100644 --- a/src/main/java/com/google/testing/compile/CompilationExtension.java +++ b/src/main/java/com/google/testing/compile/CompilationExtension.java @@ -40,7 +40,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; -import java.util.function.Supplier; import javax.annotation.processing.AbstractProcessor; import javax.annotation.processing.ProcessingEnvironment; import javax.annotation.processing.RoundEnvironment; @@ -73,8 +72,9 @@ public class CompilationExtension JavaFileObjects.forSourceLines("Dummy", "final class Dummy {}"); private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(CompilationExtension.class); - private static final Map, Function> SUPPORTED_PARAMETERS; - private static final Function RETURN_NULL = ignored -> null; + private static final Function UNKNOWN_PARAMETER = ignored -> { + throw new IllegalArgumentException("Unknown parameter type"); + }; private static final StoreAccessor PHASER_KEY = new StoreAccessor<>(Phaser.class); private static final StoreAccessor> PROCESSINGENV_KEY = @@ -86,6 +86,8 @@ public class CompilationExtension new ThreadFactoryBuilder().setDaemon(true).setNameFormat("async-compiler-%d").build() ); + private static final Map, Function> SUPPORTED_PARAMETERS; + static { SUPPORTED_PARAMETERS = ImmutableMap., Function>builder() .put(Elements.class, ProcessingEnvironment::getElementUtils) @@ -106,7 +108,6 @@ protected boolean onAdvance(int phase, int parties) { final AtomicReference sharedState = new AtomicReference<>(null); - final CompletionStage futureResult = CompletableFuture.supplyAsync(() -> Compiler.javac() .withProcessors(new EvaluatingProcessor(sharedBarrier, sharedState)) @@ -137,7 +138,7 @@ public void afterAll(ExtensionContext context) throws Exception { .toCompletableFuture().get(1, TimeUnit.SECONDS); checkState(compilation.status().equals(SUCCESS), compilation); - // Check precondition + // Check postcondition checkState(sharedPhaser.isTerminated(), "Phaser not terminated"); } @@ -161,7 +162,7 @@ public Object resolveParameter( return SUPPORTED_PARAMETERS.getOrDefault( parameterContext.getParameter().getType(), - RETURN_NULL + UNKNOWN_PARAMETER ).apply(checkNotNull( processingEnvironment.get(), "ProcessingEnvironment not available: %s", From 57ec9b50c118f265dddf64f356baf275e24a3f7b Mon Sep 17 00:00:00 2001 From: David van Leusen Date: Sun, 7 Oct 2018 19:38:11 +0200 Subject: [PATCH 4/9] Executors.newSingleThreadExecutor would force parallel test execution to run sequentially. --- .../java/com/google/testing/compile/CompilationExtension.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/testing/compile/CompilationExtension.java b/src/main/java/com/google/testing/compile/CompilationExtension.java index 42c4b45a..30a2ec93 100644 --- a/src/main/java/com/google/testing/compile/CompilationExtension.java +++ b/src/main/java/com/google/testing/compile/CompilationExtension.java @@ -82,7 +82,7 @@ public class CompilationExtension private static final StoreAccessor> RESULT_KEY = new StoreAccessor<>(Compilation.class); - private static final ExecutorService COMPILER_EXECUTOR = Executors.newSingleThreadExecutor( + private static final ExecutorService COMPILER_EXECUTOR = Executors.newCachedThreadPool( new ThreadFactoryBuilder().setDaemon(true).setNameFormat("async-compiler-%d").build() ); From 8684613796069a4f6962d4100a7d96c928924a37 Mon Sep 17 00:00:00 2001 From: Kiskae Date: Sun, 7 Oct 2018 20:26:18 +0200 Subject: [PATCH 5/9] Use JUnit5's specific exception JUnit5 provides a specific exception for parameter resolution failures --- .../java/com/google/testing/compile/CompilationExtension.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/testing/compile/CompilationExtension.java b/src/main/java/com/google/testing/compile/CompilationExtension.java index 30a2ec93..73f95465 100644 --- a/src/main/java/com/google/testing/compile/CompilationExtension.java +++ b/src/main/java/com/google/testing/compile/CompilationExtension.java @@ -73,7 +73,7 @@ public class CompilationExtension private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(CompilationExtension.class); private static final Function UNKNOWN_PARAMETER = ignored -> { - throw new IllegalArgumentException("Unknown parameter type"); + throw new ParameterResolutionException("Unknown parameter type"); }; private static final StoreAccessor PHASER_KEY = new StoreAccessor<>(Phaser.class); From ee51e6e56bf19cf9b1a9981950b5e055025127e4 Mon Sep 17 00:00:00 2001 From: David van Leusen Date: Mon, 8 Oct 2018 01:02:18 +0200 Subject: [PATCH 6/9] Fix exception during failed compilation. If the compilation fails before the EvaluatingProcessor blocks then the extension would be permanently waiting. Forcing termination on completion of the compilation will ensure it can never be stuck waiting. --- .../testing/compile/CompilationExtension.java | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/google/testing/compile/CompilationExtension.java b/src/main/java/com/google/testing/compile/CompilationExtension.java index 73f95465..67b2a6c8 100644 --- a/src/main/java/com/google/testing/compile/CompilationExtension.java +++ b/src/main/java/com/google/testing/compile/CompilationExtension.java @@ -72,9 +72,6 @@ public class CompilationExtension JavaFileObjects.forSourceLines("Dummy", "final class Dummy {}"); private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(CompilationExtension.class); - private static final Function UNKNOWN_PARAMETER = ignored -> { - throw new ParameterResolutionException("Unknown parameter type"); - }; private static final StoreAccessor PHASER_KEY = new StoreAccessor<>(Phaser.class); private static final StoreAccessor> PROCESSINGENV_KEY = @@ -96,7 +93,7 @@ public class CompilationExtension } @Override - public void beforeAll(ExtensionContext context) { + public void beforeAll(ExtensionContext context) throws Exception { final Phaser sharedBarrier = new Phaser(2) { @Override protected boolean onAdvance(int phase, int parties) { @@ -108,21 +105,29 @@ protected boolean onAdvance(int phase, int parties) { final AtomicReference sharedState = new AtomicReference<>(null); - final CompletionStage futureResult = CompletableFuture.supplyAsync(() -> - Compiler.javac() + final CompletionStage futureResult = CompletableFuture.supplyAsync( + () -> { + try { + return Compiler.javac() .withProcessors(new EvaluatingProcessor(sharedBarrier, sharedState)) - .compile(DUMMY), - COMPILER_EXECUTOR); + .compile(DUMMY); + } finally { + sharedBarrier.forceTermination(); + } + }, COMPILER_EXECUTOR); final ExtensionContext.Store store = context.getStore(NAMESPACE); PHASER_KEY.put(store, sharedBarrier); PROCESSINGENV_KEY.put(store, sharedState); RESULT_KEY.put(store, futureResult); - // Wait until the processor is ready for testing - sharedBarrier.arriveAndAwaitAdvance(); - - checkState(!sharedBarrier.isTerminated(), "Phaser terminated early"); + // Wait until the processor is ready for testing, handle termination on error + if (sharedBarrier.arriveAndAwaitAdvance() < 0) { + // Rethrow the exception thrown by the compiler, otherwise throw based on the result. + final Compilation result = futureResult.toCompletableFuture() + .get(5, TimeUnit.SECONDS); + throw new IllegalStateException(result.toString()); + } } @Override @@ -162,7 +167,9 @@ public Object resolveParameter( return SUPPORTED_PARAMETERS.getOrDefault( parameterContext.getParameter().getType(), - UNKNOWN_PARAMETER + ignored -> { + throw new ParameterResolutionException("Unknown parameter type"); + } ).apply(checkNotNull( processingEnvironment.get(), "ProcessingEnvironment not available: %s", From 033de5768c11fba6bafa2f44dd98d393bf77ddb0 Mon Sep 17 00:00:00 2001 From: David van Leusen Date: Mon, 8 Oct 2018 13:23:27 +0200 Subject: [PATCH 7/9] Check that the required state has been initialized before running a test --- .../google/testing/compile/CompilationExtension.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/testing/compile/CompilationExtension.java b/src/main/java/com/google/testing/compile/CompilationExtension.java index 67b2a6c8..0a6df177 100644 --- a/src/main/java/com/google/testing/compile/CompilationExtension.java +++ b/src/main/java/com/google/testing/compile/CompilationExtension.java @@ -24,6 +24,7 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.Extension; import org.junit.jupiter.api.extension.ExtensionContext; @@ -67,7 +68,7 @@ * @author David van Leusen */ public class CompilationExtension - implements BeforeAllCallback, AfterAllCallback, ParameterResolver { + implements BeforeAllCallback, BeforeEachCallback, AfterAllCallback, ParameterResolver { private static final JavaFileObject DUMMY = JavaFileObjects.forSourceLines("Dummy", "final class Dummy {}"); private static final ExtensionContext.Namespace NAMESPACE = @@ -130,6 +131,15 @@ protected boolean onAdvance(int phase, int parties) { } } + @Override + public void beforeEach(ExtensionContext extensionContext) { + checkState( + PHASER_KEY.get(extensionContext.getStore(NAMESPACE)) != null, + "CompilationExtension is only available as a class-level extension. " + + "Using it as an instance-level extension through @RegisterExtension is not supported" + ); + } + @Override public void afterAll(ExtensionContext context) throws Exception { final ExtensionContext.Store store = context.getStore(NAMESPACE); From bcf955af2a04e7435d518aaadf7923054a3a486a Mon Sep 17 00:00:00 2001 From: David van Leusen Date: Tue, 9 Oct 2018 19:35:58 +0200 Subject: [PATCH 8/9] Refactor CompilationExtension to support all usage scenarios. - Refactor so in absence of (before/after)All lifecycle events, it falls back on the (before/after)Each events. This supports @RegisterExtension and per-method @ExtendWith usage scenarios. - Slim down EvaluatingProcessor so it can either fail to the CompletableFuture or block on the Phaser, simplifying the exception path. - Extract all state in a dedicated object which can do checks on the state of the phaser before actions are performed. - Add tests for the compilation state and the various operations that synchronize it with the tests. - Test each of the different extension usage scenarios. - @ExtendWith on class - @ExtendWith on method - @RegisterExtension with Lifecycle.PER_CLASS - @RegisterExtension with Lifecycle.PER_METHOD --- .../testing/compile/CompilationExtension.java | 283 ++++++++++++------ .../compile/CompilationExtensionTest.java | 176 +++++++++-- 2 files changed, 334 insertions(+), 125 deletions(-) diff --git a/src/main/java/com/google/testing/compile/CompilationExtension.java b/src/main/java/com/google/testing/compile/CompilationExtension.java index 0a6df177..71d7dccb 100644 --- a/src/main/java/com/google/testing/compile/CompilationExtension.java +++ b/src/main/java/com/google/testing/compile/CompilationExtension.java @@ -22,7 +22,9 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.ThreadFactoryBuilder; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.ExtendWith; @@ -34,13 +36,15 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; -import java.util.concurrent.ExecutorService; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.Phaser; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; +import java.util.function.Supplier; import javax.annotation.processing.AbstractProcessor; import javax.annotation.processing.ProcessingEnvironment; import javax.annotation.processing.RoundEnvironment; @@ -67,20 +71,14 @@ * * @author David van Leusen */ -public class CompilationExtension - implements BeforeAllCallback, BeforeEachCallback, AfterAllCallback, ParameterResolver { +public class CompilationExtension implements BeforeAllCallback, BeforeEachCallback, + AfterAllCallback, AfterEachCallback, ParameterResolver { private static final JavaFileObject DUMMY = JavaFileObjects.forSourceLines("Dummy", "final class Dummy {}"); private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(CompilationExtension.class); - private static final StoreAccessor PHASER_KEY = new StoreAccessor<>(Phaser.class); - private static final StoreAccessor> PROCESSINGENV_KEY = - new StoreAccessor<>(ProcessingEnvironment.class); - private static final StoreAccessor> RESULT_KEY = - new StoreAccessor<>(Compilation.class); - - private static final ExecutorService COMPILER_EXECUTOR = Executors.newCachedThreadPool( + private static final Executor DEFAULT_COMPILER_EXECUTOR = Executors.newCachedThreadPool( new ThreadFactoryBuilder().setDaemon(true).setNameFormat("async-compiler-%d").build() ); @@ -93,68 +91,63 @@ public class CompilationExtension .build(); } + private final Executor compilerExecutor; + + public CompilationExtension(Executor compilerExecutor) { + this.compilerExecutor = compilerExecutor; + } + + public CompilationExtension() { + this(DEFAULT_COMPILER_EXECUTOR); + } + @Override - public void beforeAll(ExtensionContext context) throws Exception { - final Phaser sharedBarrier = new Phaser(2) { - @Override - protected boolean onAdvance(int phase, int parties) { - // Terminate the phaser once all parties have deregistered - return parties == 0; - } - }; - - final AtomicReference sharedState - = new AtomicReference<>(null); - - final CompletionStage futureResult = CompletableFuture.supplyAsync( - () -> { - try { - return Compiler.javac() - .withProcessors(new EvaluatingProcessor(sharedBarrier, sharedState)) - .compile(DUMMY); - } finally { - sharedBarrier.forceTermination(); - } - }, COMPILER_EXECUTOR); - - final ExtensionContext.Store store = context.getStore(NAMESPACE); - PHASER_KEY.put(store, sharedBarrier); - PROCESSINGENV_KEY.put(store, sharedState); - RESULT_KEY.put(store, futureResult); - - // Wait until the processor is ready for testing, handle termination on error - if (sharedBarrier.arriveAndAwaitAdvance() < 0) { - // Rethrow the exception thrown by the compiler, otherwise throw based on the result. - final Compilation result = futureResult.toCompletableFuture() - .get(5, TimeUnit.SECONDS); - throw new IllegalStateException(result.toString()); - } + public void beforeAll(ExtensionContext context) throws InterruptedException { + final CompilerState state = context.getStore(NAMESPACE).getOrComputeIfAbsent( + CompilerState.class, + ignored -> new CompilerState(this.compilerExecutor, TestInstance.Lifecycle.PER_CLASS), + CompilerState.class + ); + + checkState(state.prepareForTests(), state); } @Override - public void beforeEach(ExtensionContext extensionContext) { - checkState( - PHASER_KEY.get(extensionContext.getStore(NAMESPACE)) != null, - "CompilationExtension is only available as a class-level extension. " + - "Using it as an instance-level extension through @RegisterExtension is not supported" + public void beforeEach(ExtensionContext context) throws InterruptedException { + final CompilerState state = context.getStore(NAMESPACE).getOrComputeIfAbsent( + CompilerState.class, + ignored -> new CompilerState(this.compilerExecutor, TestInstance.Lifecycle.PER_METHOD), + CompilerState.class ); + + checkState(state.prepareForTests(), state); } @Override - public void afterAll(ExtensionContext context) throws Exception { - final ExtensionContext.Store store = context.getStore(NAMESPACE); - final Phaser sharedPhaser = PHASER_KEY.get(store); + public void afterEach(ExtensionContext context) throws Exception { + final CompilerState state = checkNotNull(context.getStore(NAMESPACE).get( + CompilerState.class, + CompilerState.class + )); - // Allow the processor to finish - sharedPhaser.arriveAndDeregister(); + if (state.getLifecycle() == TestInstance.Lifecycle.PER_METHOD) { + // Created on a per-method basis, must clean up as a mirror action + final Compilation compilation = state.allowTermination(); + checkState(compilation.status().equals(SUCCESS), compilation); + } + } - // Perform status checks, since processing is 'over' almost instantly - final Compilation compilation = RESULT_KEY.get(store) - .toCompletableFuture().get(1, TimeUnit.SECONDS); - checkState(compilation.status().equals(SUCCESS), compilation); + @Override + public void afterAll(ExtensionContext context) throws ExecutionException, InterruptedException { + final CompilerState state = checkNotNull(context.getStore(NAMESPACE).get( + CompilerState.class, + CompilerState.class + )); + + checkState(state.getLifecycle() == TestInstance.Lifecycle.PER_CLASS); - // Check postcondition - checkState(sharedPhaser.isTerminated(), "Phaser not terminated"); + final Compilation compilation = state.allowTermination(); + checkState(compilation.status().equals(SUCCESS), compilation); } @Override @@ -171,52 +164,144 @@ public Object resolveParameter( ParameterContext parameterContext, ExtensionContext extensionContext ) throws ParameterResolutionException { - final ExtensionContext.Store store = extensionContext.getStore(NAMESPACE); - final AtomicReference processingEnvironment - = PROCESSINGENV_KEY.get(store); + final CompilerState state = extensionContext.getStore(NAMESPACE).get( + CompilerState.class, + CompilerState.class + ); + + checkState(state != null, "CompilerState not initialized"); return SUPPORTED_PARAMETERS.getOrDefault( parameterContext.getParameter().getType(), ignored -> { throw new ParameterResolutionException("Unknown parameter type"); } - ).apply(checkNotNull( - processingEnvironment.get(), - "ProcessingEnvironment not available: %s", - RESULT_KEY.get(store) - )); + ).apply(state.getProcessingEnvironment()); } - /** - * Utility class to safely access {@link ExtensionContext.Store} when dealing with - * parameterized types. - */ - static final class StoreAccessor { - private final Object key; + static final class CompilerState implements ExtensionContext.Store.CloseableResource { + private final AtomicReference sharedState; + private final Phaser syncBarrier; + private final CompletableFuture result; + private final TestInstance.Lifecycle lifecycle; + + CompilerState(Executor compilerExecutor, TestInstance.Lifecycle lifecycle) { + this.lifecycle = lifecycle; + this.sharedState = new AtomicReference<>(null); + this.syncBarrier = new Phaser(2) { + @Override + protected boolean onAdvance(int phase, int parties) { + // Terminate the phaser once all parties have deregistered + return parties == 0; + } + }; + this.result = CompletableFuture.supplyAsync( + new EvaluatingProcessor(syncBarrier, sharedState), + compilerExecutor + ); + } + + ProcessingEnvironment getProcessingEnvironment() throws ParameterResolutionException { + // Only while the phaser is in phase 1 should the ProcessingEnvironment be valid. + if (this.syncBarrier.getPhase() != 1) { + throw new ParameterResolutionException(this.toString()); + } - StoreAccessor(Object key) { - this.key = key; + final ProcessingEnvironment processingEnvironment = this.sharedState.get(); + if (processingEnvironment != null) { + return processingEnvironment; + } else { + throw new ParameterResolutionException( + String.format("ProcessingEnvironment was not initialized: %s", this) + ); + } } - @SuppressWarnings("unchecked") - R get(ExtensionContext.Store store) { - return (R) store.get(key); + TestInstance.Lifecycle getLifecycle() { + return this.lifecycle; } - void put(ExtensionContext.Store store, R value) { - store.put(key, value); + boolean prepareForTests() throws InterruptedException { + switch (this.syncBarrier.getPhase()) { + case 0: // Compiler has been started, but might not yet be initialized + return checkNotTerminated(this.syncBarrier.arriveAndAwaitAdvance()); + case 1: // Compiler has been initialized, ready for tests + return true; + default: + throw new IllegalStateException(this.toString()); + } + } + + Compilation allowTermination() throws InterruptedException, ExecutionException { + if (this.syncBarrier.getPhase() == 1) { + checkState(this.syncBarrier.arriveAndDeregister() == 1, this); + } else if (!this.syncBarrier.isTerminated()) { + throw new IllegalStateException(this.toString()); + } + + try { + final Compilation result = this.result.get(1, TimeUnit.SECONDS); + checkState(this.syncBarrier.isTerminated(), this); + return result; + } catch (TimeoutException e) { + // This really should never happen, since the 'syncBarrier' is the only thing the + // processor blocks on, deregistering at this point should allow the processor + // to run until it finishes. + throw new AssertionError("Timed out waiting for the compiler to finish"); + } + } + + private boolean checkNotTerminated(int phaseNumber) throws InterruptedException { + if (phaseNumber < 0) { + // Phaser has terminated unexpectedly, throw exception based on result. + + try { + // 'Successful' result + final Compilation result = this.result.get(5, TimeUnit.SECONDS); + throw new IllegalStateException( + String.format("Anomalous compilation result: %s", result) + ); + } catch (ExecutionException e) { + // Exception in the compiler + throw new IllegalStateException("Exception during annotation processing", e.getCause()); + } catch (TimeoutException e) { + // This really should never happen, since the 'syncBarrier' is the only thing the + // processor blocks on, termination should mean it runs until it finished, + // resolving 'result' + throw new AssertionError("Timed out waiting for the cause of termination"); + } + } + + return true; + } + + @Override + public void close() { + // If the owning ExtensionContext.Store is closed, ensure the compilation terminates as well + this.syncBarrier.forceTermination(); + } + + @Override + public String toString() { + return "CompilerState{" + + "sharedState=" + sharedState + + ", syncBarrier=" + syncBarrier + + ", result=" + result + + ", lifecycle=" + lifecycle + + '}'; } } - static final class EvaluatingProcessor extends AbstractProcessor { - private final Phaser barrier; + static final class EvaluatingProcessor extends AbstractProcessor + implements Supplier { + private final Phaser syncBarrier; private final AtomicReference sharedState; EvaluatingProcessor( - Phaser barrier, + Phaser syncBarrier, AtomicReference sharedState ) { - this.barrier = barrier; + this.syncBarrier = syncBarrier; this.sharedState = sharedState; } @@ -235,26 +320,34 @@ public synchronized void init(ProcessingEnvironment processingEnvironment) { super.init(processingEnvironment); // Share the processing environment - if (!sharedState.compareAndSet(null, processingEnvironment)) { - // Invalid state, init() run twice - barrier.forceTermination(); - throw new IllegalStateException("Processor initialized twice"); - } + checkState( + sharedState.compareAndSet(null, processingEnvironment), + "Shared ProcessingEnvironment was already initialized" + ); } @Override public boolean process(Set annotations, RoundEnvironment roundEnv) { if (roundEnv.processingOver()) { // Synchronize on the beginning of the test run - barrier.arriveAndAwaitAdvance(); + syncBarrier.arriveAndAwaitAdvance(); // Now wait until testing is over - barrier.awaitAdvance(barrier.arriveAndDeregister()); + syncBarrier.awaitAdvance(syncBarrier.arriveAndDeregister()); // Clean up the shared state - sharedState.getAndSet(null); + sharedState.lazySet(null); } return false; } + + @Override + public Compilation get() { + try { + return Compiler.javac().withProcessors(this).compile(DUMMY); + } finally { + syncBarrier.forceTermination(); + } + } } } diff --git a/src/test/java/com/google/testing/compile/CompilationExtensionTest.java b/src/test/java/com/google/testing/compile/CompilationExtensionTest.java index 530b48d8..b8117b1b 100644 --- a/src/test/java/com/google/testing/compile/CompilationExtensionTest.java +++ b/src/test/java/com/google/testing/compile/CompilationExtensionTest.java @@ -16,14 +16,24 @@ package com.google.testing.compile; import static com.google.common.truth.Truth.assertThat; +import static com.google.testing.compile.CompilationSubject.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.platform.runner.JUnitPlatform; import org.junit.runner.RunWith; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicInteger; import javax.lang.model.element.Element; import javax.lang.model.element.TypeElement; @@ -36,45 +46,151 @@ * Tests the {@link CompilationExtension} by applying it to this test. */ @RunWith(JUnitPlatform.class) -@ExtendWith(CompilationExtension.class) public class CompilationExtensionTest { - private final AtomicInteger executions = new AtomicInteger(); - @Test public void testMethodsExecuteExactlyOnce() { - assertThat(executions.getAndIncrement()).isEqualTo(0); + @Test + void testAsyncCompiler() { + final ExecutorService executor = Executors.newSingleThreadExecutor(); + final CompilationExtension.CompilerState state = new CompilationExtension.CompilerState( + executor, + TestInstance.Lifecycle.PER_CLASS + ); + executor.shutdown(); // Allow executor to finish with the compiler + + // Calling .allowTermination before .prepareForTests should not result in invalid state + assertThrows(IllegalStateException.class, state::allowTermination); + + // prepareForTests is idempotent + assertDoesNotThrow(() -> { + assertThat(state.prepareForTests()).isTrue(); + assertThat(state.prepareForTests()).isTrue(); + }); + + // it should only need to finish the compilation once. + assertDoesNotThrow(() -> { + CompilationSubject subject = assertThat(state.allowTermination()); + + subject.succeeded(); + + // Repeat calls should just return the same results + subject.isEqualTo(state.allowTermination()); + }); + + // Prepare after termination should fail. + assertThrows(IllegalStateException.class, state::prepareForTests); + + // Since compilation has finished, the executor should also be idle + assertThat(executor.isTerminated()).isTrue(); } - @BeforeEach /* we also make sure that getElements works in a @Before method */ - @Test public void getElements(Elements elements) { - assertThat(elements).isNotNull(); + @Nested + @ExtendWith(CompilationExtension.class) + @DisplayName("@ExtendWith Class") + @TestInstance(TestInstance.Lifecycle.PER_CLASS) + class ClassExtendWith extends ExtensionTests { + @BeforeAll + void testBeforeAll(Elements elements, Types types) { + elementsAreValidAndWorking(elements); + typeMirrorsAreValidAndWorking(elements, types); + } + } + + @Nested + @DisplayName("@ExtendWith Method") + class MethodExtendWith extends ExtensionTests { + @Override + @ExtendWith(CompilationExtension.class) + @Test public void testMethodsExecuteExactlyOnce() { + // By definition a method-based extension would not support @BeforeEach + super.testMethodsExecuteExactlyOnce(); + } + + @Override + @ExtendWith(CompilationExtension.class) + @Test public void getElements(Elements elements) { + super.getElements(elements); + } + + @Override + @ExtendWith(CompilationExtension.class) + @Test public void getTypes(Types types) { + super.getTypes(types); + } + + @Override + @ExtendWith(CompilationExtension.class) + @Test public void elementsAreValidAndWorking(Elements elements) { + super.elementsAreValidAndWorking(elements); + } + + @Override + @ExtendWith(CompilationExtension.class) + @Test public void typeMirrorsAreValidAndWorking(Elements elements, Types types) { + super.typeMirrorsAreValidAndWorking(elements, types); + } } - @BeforeEach /* we also make sure that getTypes works in a @Before method */ - @Test public void getTypes(Types types) { - assertThat(types).isNotNull(); + @Nested + @DisplayName("@RegisterWith - Lifecycle.PER_METHOD") + class RegisterWithPerMethod extends ExtensionTests { + @RegisterExtension + CompilationExtension ext = new CompilationExtension(); } - /** - * Do some non-trivial operation with {@link Element} instances because they stop working after - * compilation stops. - */ - @Test public void elementsAreValidAndWorking(Elements elements) { - TypeElement stringElement = elements.getTypeElement(String.class.getName()); - assertThat(stringElement.getEnclosingElement()) - .isEqualTo(elements.getPackageElement("java.lang")); + @Nested + @DisplayName("@RegisterWith - Lifecycle.PER_CLASS") + @TestInstance(TestInstance.Lifecycle.PER_CLASS) + class RegisterWithPerClass extends ExtensionTests { + @RegisterExtension + CompilationExtension ext = new CompilationExtension(); + + @BeforeAll + void testBeforeAll(Elements elements, Types types) { + elementsAreValidAndWorking(elements); + typeMirrorsAreValidAndWorking(elements, types); + } } - /** - * Do some non-trivial operation with {@link TypeMirror} instances because they stop working after - * compilation stops. - */ - @Test public void typeMirrorsAreValidAndWorking(Elements elements, Types types) { - DeclaredType arrayListOfString = types.getDeclaredType( - elements.getTypeElement(ArrayList.class.getName()), - elements.getTypeElement(String.class.getName()).asType()); - DeclaredType listOfExtendsObjectType = types.getDeclaredType( - elements.getTypeElement(List.class.getName()), - types.getWildcardType(elements.getTypeElement(Object.class.getName()).asType(), null)); - assertThat(types.isAssignable(arrayListOfString, listOfExtendsObjectType)).isTrue(); + abstract class ExtensionTests { + private final AtomicInteger executions = new AtomicInteger(); + + @Test public void testMethodsExecuteExactlyOnce() { + assertThat(executions.getAndIncrement()).isEqualTo(0); + } + + @BeforeEach + @Test public void getElements(Elements elements) { + assertThat(elements).isNotNull(); + } + + + @BeforeEach + @Test public void getTypes(Types types) { + assertThat(types).isNotNull(); + } + + /** + * Do some non-trivial operation with {@link Element} instances because they stop working after + * compilation stops. + */ + @Test public void elementsAreValidAndWorking(Elements elements) { + TypeElement stringElement = elements.getTypeElement(String.class.getName()); + assertThat(stringElement.getEnclosingElement()) + .isEqualTo(elements.getPackageElement("java.lang")); + } + + /** + * Do some non-trivial operation with {@link TypeMirror} instances because they stop working after + * compilation stops. + */ + @Test public void typeMirrorsAreValidAndWorking(Elements elements, Types types) { + DeclaredType arrayListOfString = types.getDeclaredType( + elements.getTypeElement(ArrayList.class.getName()), + elements.getTypeElement(String.class.getName()).asType()); + DeclaredType listOfExtendsObjectType = types.getDeclaredType( + elements.getTypeElement(List.class.getName()), + types.getWildcardType(elements.getTypeElement(Object.class.getName()).asType(), null)); + assertThat(types.isAssignable(arrayListOfString, listOfExtendsObjectType)).isTrue(); + } } } From bdc7fc9840fa990fdf88b20ddb3c29208f68178a Mon Sep 17 00:00:00 2001 From: David van Leusen Date: Tue, 9 Oct 2018 19:41:24 +0200 Subject: [PATCH 9/9] Travis apparently changes default scheduler behaviour. --- .../com/google/testing/compile/CompilationExtensionTest.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/test/java/com/google/testing/compile/CompilationExtensionTest.java b/src/test/java/com/google/testing/compile/CompilationExtensionTest.java index b8117b1b..a7f1a007 100644 --- a/src/test/java/com/google/testing/compile/CompilationExtensionTest.java +++ b/src/test/java/com/google/testing/compile/CompilationExtensionTest.java @@ -78,9 +78,6 @@ void testAsyncCompiler() { // Prepare after termination should fail. assertThrows(IllegalStateException.class, state::prepareForTests); - - // Since compilation has finished, the executor should also be idle - assertThat(executor.isTerminated()).isTrue(); } @Nested