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.

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<DevModeContext.ModuleInfo, DevModeContext.CompilationUnit> 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<File, Long> entry : compileTimestamps.entrySet()) {
                         sourceFileTimestamps.put(entry.getKey().toPath(), entry.getValue());
                     }
@@ -881,13 +884,13 @@ Set<String> checkForFileChange(Function<DevModeContext.ModuleInfo, DevModeContex
         return ret;
     }
 
-    private boolean sourceFileWasRecentModified(final Path sourcePath, boolean ignoreFirstScanChanges) {
-        return checkIfFileModified(sourcePath, sourceFileTimestamps, ignoreFirstScanChanges);
+    private boolean sourceFileWasRecentModified(final Path sourcePath, boolean ignoreFirstScanChanges, boolean firstScan) {
+        return checkIfFileModified(sourcePath, sourceFileTimestamps, ignoreFirstScanChanges, firstScan);
     }
 
     private boolean classFileWasRecentModified(final Path classFilePath, boolean ignoreFirstScanChanges,
             TimestampSet timestampSet) {
-        return checkIfFileModified(classFilePath, timestampSet.classFileChangeTimeStamps, ignoreFirstScanChanges);
+        return checkIfFileModified(classFilePath, timestampSet.classFileChangeTimeStamps, ignoreFirstScanChanges, true);
     }
 
     private boolean classFileWasAdded(final Path classFilePath, boolean ignoreFirstScanChanges, TimestampSet timestampSet) {
@@ -902,18 +905,23 @@ private boolean classFileWasAdded(final Path classFilePath, boolean ignoreFirstS
         return lastRecordedChange == null && !ignoreFirstScanChanges;
     }
 
-    private boolean checkIfFileModified(Path path, Map<Path, Long> pathModificationTimes, boolean ignoreFirstScanChanges) {
+    private boolean checkIfFileModified(Path path, Map<Path, Long> 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;
             }
 

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"));
+    }
+}

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"));
+    }
+}

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"));
+    }
+}