From 1bbbe605692530a7769403c83b01a49b13254c58 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Wed, 23 Jun 2021 11:15:32 +1000 Subject: [PATCH] Fix dev mode compile issues If a file has a compile problem, and then another file is modified that compiles correctly dev mode will restart and ignore the original compile error. This changes the compilation so that the timestamps are only updated on a sucessful compile, so files that failed to compile will always be recompiled. (cherry picked from commit 0fcde158b7139cf3b037dbb1bee1994bcd91d585) --- .../dev/RuntimeUpdatesProcessor.java | 26 ++++++++----- .../devmode/CompileCorrectlyEndpoint.java | 14 +++++++ .../http/devmode/CompileErrorEndpoint.java | 14 +++++++ .../http/devmode/CompileProblemTest.java | 39 +++++++++++++++++++ 4 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileCorrectlyEndpoint.java create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileErrorEndpoint.java create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileProblemTest.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesProcessor.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesProcessor.java index 62edd7ce0f8da..76907bbf02f66 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesProcessor.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesProcessor.java @@ -283,7 +283,9 @@ private ClassScanResult compileTestClasses() { testRunner.testCompileFailed(compileProblem); compileProblem = null; //we don't want to block the app over a test problem } else { - testRunner.testCompileSucceeded(); + if (changedTestClassResult.isChanged()) { + testRunner.testCompileSucceeded(); + } } } catch (Throwable e) { testRunner.testCompileFailed(e); @@ -558,7 +560,7 @@ ClassScanResult checkForChangedTestClasses(boolean firstScan) { * This is useful in two ways. * - To make sure that source time stamps have been recorded at least once * - To avoid re-compiling on first run by ignoring all first time changes detected by - * {@link RuntimeUpdatesProcessor#checkIfFileModified(Path, Map, boolean)} during the first scan. + * {@link RuntimeUpdatesProcessor#checkIfFileModified(Path, Map, boolean, boolean)} during the first scan. */ ClassScanResult checkForChangedClasses(QuarkusCompiler compiler, Function cuf, boolean firstScan, @@ -579,7 +581,7 @@ ClassScanResult checkForChangedClasses(QuarkusCompiler compiler, changedSourceFiles = sourcesStream .parallel() .filter(p -> matchingHandledExtension(p).isPresent() - && sourceFileWasRecentModified(p, ignoreFirstScanChanges)) + && sourceFileWasRecentModified(p, ignoreFirstScanChanges, firstScan)) .map(Path::toFile) //Needing a concurrent Set, not many standard options: .collect(Collectors.toCollection(ConcurrentSkipListSet::new)); @@ -643,6 +645,7 @@ && sourceFileWasRecentModified(p, ignoreFirstScanChanges)) //now we re-update the underlying timestamps, to the values we just compiled //if the file has changed in the meantime it will be picked up in the next //scan + //note that if compile failed these are not updated, so failing files will always be re-compiled for (Map.Entry entry : compileTimestamps.entrySet()) { sourceFileTimestamps.put(entry.getKey().toPath(), entry.getValue()); } @@ -881,13 +884,13 @@ Set checkForFileChange(Function pathModificationTimes, boolean ignoreFirstScanChanges) { + private boolean checkIfFileModified(Path path, Map pathModificationTimes, boolean ignoreFirstScanChanges, + boolean updateTimestamp) { try { final long lastModificationTime = Files.getLastModifiedTime(path).toMillis(); final Long lastRecordedChange = pathModificationTimes.get(path); if (lastRecordedChange == null) { - pathModificationTimes.put(path, lastModificationTime); + if (updateTimestamp) { + pathModificationTimes.put(path, lastModificationTime); + } return !ignoreFirstScanChanges; } if (lastRecordedChange != lastModificationTime) { - pathModificationTimes.put(path, lastModificationTime); + if (updateTimestamp) { + pathModificationTimes.put(path, lastModificationTime); + } return true; } diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileCorrectlyEndpoint.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileCorrectlyEndpoint.java new file mode 100644 index 0000000000000..b514f915f2c12 --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileCorrectlyEndpoint.java @@ -0,0 +1,14 @@ +package io.quarkus.vertx.http.devmode; + +import javax.enterprise.event.Observes; + +import io.vertx.ext.web.Router; + +public class CompileCorrectlyEndpoint { + + void addConfigRoute(@Observes Router router) { + router.route("/correct") + .produces("text/plain") + .handler(rc -> rc.response().end("correct")); + } +} diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileErrorEndpoint.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileErrorEndpoint.java new file mode 100644 index 0000000000000..7149f2e7a61d6 --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileErrorEndpoint.java @@ -0,0 +1,14 @@ +package io.quarkus.vertx.http.devmode; + +import javax.enterprise.event.Observes; + +import io.vertx.ext.web.Router; + +public class CompileErrorEndpoint { + + void addConfigRoute(@Observes Router router) { + router.route("/error") + .produces("text/plain") + .handler(rc -> rc.response().end("error")); + } +} diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileProblemTest.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileProblemTest.java new file mode 100644 index 0000000000000..60e53bd5f7ab2 --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/devmode/CompileProblemTest.java @@ -0,0 +1,39 @@ +package io.quarkus.vertx.http.devmode; + +import static org.hamcrest.Matchers.equalTo; + +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusDevModeTest; +import io.restassured.RestAssured; + +/** + * Tests that once a file has a compile error restart will not happen until it is fixed, even if + * other files are subsequently modified that do compile. + */ +public class CompileProblemTest { + + @RegisterExtension + static final QuarkusDevModeTest test = new QuarkusDevModeTest() + .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) + .addClasses(CompileErrorEndpoint.class, CompileCorrectlyEndpoint.class)); + + @Test + public void test() { + RestAssured.get("/error").then().body(equalTo("error")); + RestAssured.get("/correct").then().body(equalTo("correct")); + test.modifySourceFile(CompileErrorEndpoint.class, s -> s.replace("\"error\"", "\"compile error")); + RestAssured.get("/error").then().statusCode(500); + RestAssured.get("/correct").then().statusCode(500); + test.modifySourceFile(CompileCorrectlyEndpoint.class, s -> s.replace("\"correct\"", "\"compiled correctly\"")); + //make sure that we are still in an error state, as CompileErrorEndpoint is broken + RestAssured.get("/error").then().statusCode(500); + RestAssured.get("/correct").then().statusCode(500); + test.modifySourceFile(CompileErrorEndpoint.class, s -> s.replace("compile error", "compile error fixed\"")); + RestAssured.get("/error").then().body(equalTo("compile error fixed")); + RestAssured.get("/correct").then().body(equalTo("compiled correctly")); + } +}