core: cleanup registration to be part of teardown (#824)

Author: MykolaGolubyev

webtau-browser/src/main/java/org/testingisdocumenting/webtau/browser/driver/WebDriverCreator.java

@@ -20,6 +20,7 @@
 import org.openqa.selenium.firefox.FirefoxDriver;
 import org.openqa.selenium.firefox.FirefoxOptions;
 import org.testingisdocumenting.webtau.browser.BrowserConfig;
+import org.testingisdocumenting.webtau.cleanup.CleanupRegistration;
 import org.testingisdocumenting.webtau.console.ConsoleOutputs;
 import org.testingisdocumenting.webtau.console.ansi.Color;
 import io.github.bonigarcia.wdm.WebDriverManager;
@@ -38,6 +39,8 @@ public class WebDriverCreator {
 
     private static final List<WebDriver> drivers = Collections.synchronizedList(new ArrayList<>());
 
+    private static final ThreadLocal<Boolean> disableBrowserClose = ThreadLocal.withInitial(() -> false);
+
     static {
         registerCleanup();
     }
@@ -51,6 +54,20 @@ public static WebDriver create() {
         return register(driver);
     }
 
+    public static void quitAll() {
+        drivers.forEach(WebDriverCreator::quitWithoutRemove);
+        drivers.clear();
+    }
+
+    public static void withDisabledBrowserAutoClose(Runnable code) {
+        try {
+            disableBrowserClose.set(true);
+            code.run();
+        } finally {
+            disableBrowserClose.set(false);
+        }
+    }
+
     static void quit(WebDriver driver) {
         quitWithoutRemove(driver);
         drivers.remove(driver);
@@ -147,11 +164,6 @@ private static void setupDriverManagerConfig() {
         System.setProperty("wdm.forceCache", "true");
     }
 
-    public static void quitAll() {
-        drivers.forEach(WebDriverCreator::quitWithoutRemove);
-        drivers.clear();
-    }
-
     private static WebDriver register(WebDriver driver) {
         drivers.add(driver);
         WebDriverCreatorListeners.afterDriverCreation(driver);
@@ -170,6 +182,8 @@ private static void initState(WebDriver driver) {
     }
 
     private static void registerCleanup() {
-        Runtime.getRuntime().addShutdownHook(new Thread(WebDriverCreator::quitAll));
+        CleanupRegistration.registerForCleanup("closing", "closed", "browsers",
+                () -> !disableBrowserClose.get() && !drivers.isEmpty(),
+                WebDriverCreator::quitAll);
     }
 }

webtau-cli/src/main/java/org/testingisdocumenting/webtau/cli/CliBackgroundCommandManager.java

@@ -16,6 +16,7 @@
 
 package org.testingisdocumenting.webtau.cli;
 
+import org.testingisdocumenting.webtau.cleanup.CleanupRegistration;
 import org.testingisdocumenting.webtau.reporter.TestListener;
 import org.testingisdocumenting.webtau.reporter.TestResultPayload;
 import org.testingisdocumenting.webtau.reporter.WebTauTest;
@@ -51,12 +52,6 @@ static void remove(CliBackgroundCommand backgroundCommand) {
         runningCommands.remove(backgroundCommand.getBackgroundProcess().getPid());
     }
 
-    @Override
-    public void afterAllTests() {
-        destroyActiveProcesses();
-        runningCommands.clear();
-    }
-
     @Override
     public void beforeTestRun(WebTauTest test) {
         localRunningCommands.get().clear();
@@ -78,10 +73,11 @@ public void afterTestRun(WebTauTest test) {
         test.addTestResultPayload(new TestResultPayload("cliBackground", backgroundCommands));
     }
 
-    static void destroyActiveProcesses() {
+    static synchronized void destroyActiveProcesses() {
         runningCommands.values().stream()
                 .filter(CliBackgroundCommand::isActive)
                 .forEach(CliBackgroundCommand::stop);
+        runningCommands.clear();
     }
 
     private static void validateProcessActive(CliBackgroundCommand backgroundCommand) {
@@ -94,10 +90,9 @@ private static class LazyShutdownHook {
         private static final LazyShutdownHook INSTANCE = new LazyShutdownHook();
 
         private LazyShutdownHook() {
-            // afterAllTests may not be called if cli module is used outside of a test runner
-            // that's why we register clearing of background processed as hook
-            Runtime.getRuntime().addShutdownHook(
-                    new Thread(CliBackgroundCommandManager::destroyActiveProcesses));
+            CleanupRegistration.registerForCleanup("shutting down", "shut down", "cli background processes",
+                    () -> runningCommands.values().stream().anyMatch(CliBackgroundCommand::isActive),
+                    CliBackgroundCommandManager::destroyActiveProcesses);
         }
 
         // to trigger class loading and shutdown hook registration

webtau-core/src/main/java/org/testingisdocumenting/webtau/cleanup/CleanupRegistration.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright 2021 webtau maintainers
+ *
+ * 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 org.testingisdocumenting.webtau.cleanup;
+
+import org.testingisdocumenting.webtau.reporter.TestListener;
+import org.testingisdocumenting.webtau.reporter.WebTauStep;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.function.Supplier;
+
+import static org.testingisdocumenting.webtau.reporter.IntegrationTestsMessageBuilder.action;
+import static org.testingisdocumenting.webtau.reporter.IntegrationTestsMessageBuilder.id;
+import static org.testingisdocumenting.webtau.reporter.TokenizedMessage.tokenizedMessage;
+
+/**
+ * centralized place to register cleanup actions at the end of all test runs
+ * as a catch-all cleans not cleaned things on exit
+ */
+public class CleanupRegistration implements TestListener {
+    private static final List<CleanupEntry> registered = Collections.synchronizedList(new ArrayList<>());
+
+    @Override
+    public void afterAllTests() {
+        cleanup();
+    }
+
+    public static void registerForCleanup(String action,
+                                          String actionCompleted,
+                                          String id,
+                                          Supplier<Boolean> isValid,
+                                          Runnable cleanupCode) {
+        registered.add(new CleanupEntry(action, actionCompleted, id, isValid, cleanupCode));
+
+        // lazy shutdown hook init to avoid nested shutdowns in case of
+        // JUnit4 runner that calls afterAllTests in shutdown hook
+        ShutdownHook.INSTANCE.noOp();
+    }
+
+    private static void cleanup() {
+        registered.stream()
+                .filter(CleanupEntry::isValid)
+                .forEach(CleanupRegistration::cleanup);
+        registered.removeIf(CleanupEntry::isValid);
+    }
+
+    private static void cleanup(CleanupEntry entry) {
+        WebTauStep.createAndExecuteStep(
+                tokenizedMessage(action(entry.action), id(entry.id)),
+                () -> tokenizedMessage(action(entry.actionCompleted), id(entry.id)),
+                entry.cleanupCode);
+    }
+
+    private static void registerShutdownHook() {
+        Runtime.getRuntime().addShutdownHook(new Thread(CleanupRegistration::cleanup));
+    }
+
+    private static class CleanupEntry {
+        String action;
+        String actionCompleted;
+        String id;
+        Supplier<Boolean> isValid;
+        Runnable cleanupCode;
+
+        public CleanupEntry(String action,
+                            String actionCompleted,
+                            String id,
+                            Supplier<Boolean> isValid,
+                            Runnable cleanupCode) {
+            this.action = action;
+            this.actionCompleted = actionCompleted;
+            this.id = id;
+            this.isValid = isValid;
+            this.cleanupCode = cleanupCode;
+        }
+
+        boolean isValid() {
+            return this.isValid.get();
+        }
+    }
+
+    private static class ShutdownHook {
+        final static ShutdownHook INSTANCE = new ShutdownHook();
+
+        private ShutdownHook() {
+            registerShutdownHook();
+        }
+
+        public void noOp() {
+        }
+    }
+}

webtau-core/src/main/resources/META-INF/services/org.testingisdocumenting.webtau.reporter.TestListener

@@ -1 +1,18 @@
-org.testingisdocumenting.webtau.reporter.TestResultPayloadExtractorTestListener
+#
+# Copyright 2021 webtau maintainers
+#
+# 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.
+#
+
+org.testingisdocumenting.webtau.reporter.TestResultPayloadExtractorTestListener
+org.testingisdocumenting.webtau.cleanup.CleanupRegistration

webtau-feature-testing/examples/listeners/ServerAutoStartListener.groovy

@@ -24,22 +24,13 @@ import org.testingisdocumenting.webtau.version.WebtauVersion
 import static org.testingisdocumenting.webtau.WebTauGroovyDsl.*
 
 class ServerAutoStartListener implements TestListener {
-    CliBackgroundCommand server
-
     @Override
     void beforeFirstTest() {
         def jarName = "webtau-testapp-${WebtauVersion.version}-exec.jar"
-        server = cli.runInBackground("java -jar ../webtau-testapp/target/${jarName} --server.port=0 --spring.profiles.active=qa")
+        CliBackgroundCommand server = cli.runInBackground("java -jar ../webtau-testapp/target/${jarName} --server.port=0 --spring.profiles.active=qa")
         server.output.waitTo(contain("Tomcat started on port(s)"), 40_000)
 
         def port = RegexpUtils.extractByRegexp(server.output.get(), /Tomcat started on port\(s\): (\d+)/)
         cfg.baseUrl = "http://localhost:${port}"
     }
-
-    @Override
-    void afterAllTests() {
-        if (server) {
-            server.stop()
-        }
-    }
 }

webtau-feature-testing/src/main/groovy/org/testingisdocumenting/webtau/featuretesting/WebTauEndToEndTestRunner.groovy

@@ -18,9 +18,9 @@
 package org.testingisdocumenting.webtau.featuretesting
 
 import org.eclipse.jetty.server.Handler
+import org.testingisdocumenting.webtau.browser.driver.WebDriverCreator
 import org.testingisdocumenting.webtau.cfg.WebTauConfig
 import org.testingisdocumenting.webtau.cli.WebTauCliApp
-import org.testingisdocumenting.webtau.documentation.DocumentationArtifactsLocation
 import org.testingisdocumenting.webtau.http.testserver.TestServer
 import org.testingisdocumenting.webtau.reporter.*
 
@@ -80,8 +80,11 @@ class WebTauEndToEndTestRunner  {
 
         capturedStepsSummary = [:].withDefault { 0 }
 
-        cliApp.start(WebTauCliApp.WebDriverBehavior.KeepWebDriversOpen) { exitCode ->
-            testDetails.exitCode = exitCode
+        // this is for optimization so browsers stay open in between feature test runs
+        WebDriverCreator.withDisabledBrowserAutoClose {
+            cliApp.start { exitCode ->
+                testDetails.exitCode = exitCode
+            }
         }
 
         testDetails.scenarioDetails = buildScenarioDetails(cliApp.runner.report)

webtau-feature-testing/test-expectations/scenarios/cli/backgroundCommandAutoKill/run-details.json

@@ -9,7 +9,7 @@
     "scenario" : "after all tests",
     "shortContainerId" : "Teardown",
     "stepsSummary" : {
-      "numberOfSuccessful" : 1
+      "numberOfSuccessful" : 2
     }
   }, {
     "scenario" : "dummy scenario",

webtau-feature-testing/test-expectations/scenarios/cli/backgroundExamples/run-details.json

@@ -3,7 +3,7 @@
     "scenario" : "after all tests",
     "shortContainerId" : "Teardown",
     "stepsSummary" : {
-      "numberOfSuccessful" : 1
+      "numberOfSuccessful" : 2
     }
   }, {
     "scenario" : "run in background",

webtau-feature-testing/test-expectations/scenarios/fs/archive/run-details.json

@@ -1,5 +1,11 @@
 {
   "scenarioDetails" : [ {
+    "scenario" : "after all tests",
+    "shortContainerId" : "Teardown",
+    "stepsSummary" : {
+      "numberOfSuccessful" : 1
+    }
+  }, {
     "scenario" : "unzip",
     "shortContainerId" : "archive.groovy",
     "stepsSummary" : {

webtau-feature-testing/test-expectations/scenarios/fs/copy/run-details.json

@@ -1,5 +1,11 @@
 {
   "scenarioDetails" : [ {
+    "scenario" : "after all tests",
+    "shortContainerId" : "Teardown",
+    "stepsSummary" : {
+      "numberOfSuccessful" : 1
+    }
+  }, {
     "scenario" : "copy file to temp dir",
     "shortContainerId" : "copy.groovy",
     "stepsSummary" : {