Update Selenium 4.18.0 (#1815)

* Update Selenium to 4.18.0
* Create `ApiBootstrapHelper` create initial user entirely via API
* Update `ArtifactCollector` to not require a test object
* Move heap dump method to `BaseWebDriverTest`

.gitignore

@@ -4,6 +4,11 @@ out/
 # Transform jars built by Gradle
 data/qc/*.jar
 
+# Logs and temp files created by tests
+data/**/*.log
+data/**/assaydata/
+data/**/drt_temp/
+
 # Generated by test runner
 remainingTests.txt
 

build.gradle

@@ -33,8 +33,6 @@ project.dependencies {
     implementation("org.seleniumhq.selenium:selenium-remote-driver:${seleniumVersion}")
     implementation("org.seleniumhq.selenium:selenium-chrome-driver:${seleniumVersion}")
     implementation "org.seleniumhq.selenium:selenium-ie-driver:${seleniumVersion}"
-    // https://github.com/SeleniumHQ/selenium/issues/11750#issuecomment-1470357124
-    implementation "org.seleniumhq.selenium:selenium-http-jdk-client:${seleniumVersion}"
     implementation("org.eclipse.jetty:jetty-util:${jettyVersion}")
 
     implementation("com.google.guava:guava:${guavaVersion}")

gradle.properties

@@ -4,7 +4,7 @@ awaitilityVersion=4.2.0
 
 lookfirstSardineVersion=5.7
 jettyVersion=12.0.4
-seleniumVersion=4.11.0
+seleniumVersion=4.18.0
 mockserverNettyVersion=5.15.0
 
 labkeySchemasTestVersion=23.11-SNAPSHOT

src/org/labkey/test/BaseWebDriverTest.java

@@ -143,6 +143,7 @@
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.labkey.test.TestProperties.isHeapDumpCollectionEnabled;
 import static org.labkey.test.TestProperties.isInjectionCheckEnabled;
 import static org.labkey.test.TestProperties.isLeakCheckSkipped;
 import static org.labkey.test.TestProperties.isLinkCheckEnabled;
@@ -155,9 +156,11 @@
 import static org.labkey.test.WebTestHelper.GC_ATTEMPT_LIMIT;
 import static org.labkey.test.WebTestHelper.MAX_LEAK_LIMIT;
 import static org.labkey.test.WebTestHelper.buildURL;
+import static org.labkey.test.WebTestHelper.isLocalServer;
 import static org.labkey.test.WebTestHelper.logToServer;
 import static org.labkey.test.components.ext4.Window.Window;
 import static org.labkey.test.components.html.RadioButton.RadioButton;
+import static org.labkey.test.teamcity.TeamCityUtils.publishArtifact;
 
 /**
  * This class should be used as the base for all functional test classes
@@ -190,6 +193,7 @@ public abstract class BaseWebDriverTest extends LabKeySiteWrapper implements Cle
     private String _lastPageText = null;
     protected static boolean _testFailed = false;
     protected static boolean _anyTestFailed = false;
+    private static boolean _dumpedHeap = false;
     private final ArtifactCollector _artifactCollector;
     private final DeferredErrorCollector _errorCollector;
 
@@ -1068,7 +1072,7 @@ private void handleFailure(Throwable error, @LoggedParam String testName)
                             .filter(s -> !s.isEmpty()).toList();
                         TestLogger.error("Remaining files after attempting to delete: " + path + "\n\t" + String.join("\t\n", subdirs), notEmpty);
                     }
-                    getArtifactCollector().dumpHeap();
+                    dumpHeap();
                 }
                 catch (IOException e)
                 {
@@ -1115,6 +1119,47 @@ private Throwable unwrapRuntimeException(Throwable throwable)
         return null;
     }
 
+    public void dumpHeap()
+    {
+        if (_dumpedHeap || // Only one heap dump per suite (don't want to overload TeamCity)
+                !isLocalServer() ||
+                isGuestModeTest() ||
+                !isHeapDumpCollectionEnabled()
+        )
+        {
+            return;
+        }
+
+        pushLocation();
+
+        // Use dumpHeapAction rather that touching file so that we can get file name and publish artifact.
+        beginAt(WebTestHelper.buildURL("admin", "dumpHeap"));
+        String dumpMsg = Locators.bodyPanel().childTag("div").findElement(getDriver()).getText();
+        String filePrefix = "Heap dumped to ";
+        int prefixIndex = dumpMsg.indexOf(filePrefix);
+        if (prefixIndex < 0)
+        {
+            checker().error("Unable to extract heap dump filename from page body.\n" + dumpMsg);
+            return;
+        }
+        String filename = dumpMsg.substring(prefixIndex + filePrefix.length());
+        File heapDump = new File(filename);
+        File destFile = new File(getArtifactCollector().ensureDumpDir(), heapDump.getName());
+        try
+        {
+            Files.move(heapDump.toPath(), destFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
+            publishArtifact(destFile, null);
+            _dumpedHeap = true;
+        }
+        catch (IOException e)
+        {
+            TestLogger.error("Failed to move HeapDump file to test logs directory.");
+            e.printStackTrace();
+        }
+
+        popLocation(); // go back to get screenshot if needed.
+    }
+
     private void dumpBrowserConsole()
     {
         List<?> logEntries = executeScript("return console.everything;", List.class);
@@ -1353,7 +1398,7 @@ protected void checkLeaks()
 
             if (newLeak != null)
             {
-                getArtifactCollector().dumpHeap();
+                dumpHeap();
                 ArtifactCollector.dumpThreads();
                 fail(String.format("Found memory leak: %s [1 of %d, MAX:%d]\nSee test artifacts for more information.", newLeak, leakCount, MAX_LEAK_LIMIT));
             }
@@ -2693,7 +2738,7 @@ private File getDownloadDir()
         private void setUp(BaseWebDriverTest test)
         {
             WebDriver oldWebDriver = getWebDriver();
-            File newDownloadDir = new File(test.getArtifactCollector().ensureDumpDir(test.getClass().getSimpleName()), "downloads");
+            File newDownloadDir = new File(ArtifactCollector.ensureDumpDir(test.getClass().getSimpleName()), "downloads");
             _driverAndService = test.createNewWebDriver(_driverAndService, test.BROWSER_TYPE, newDownloadDir);
             if (getWebDriver() != oldWebDriver) // downloadDir only changes when a new WebDriver is started.
                 _downloadDir = newDownloadDir;

src/org/labkey/test/ExtraSiteWrapper.java

@@ -15,6 +15,7 @@
  */
 package org.labkey.test;
 
+import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.remote.service.DriverService;
@@ -31,6 +32,12 @@ public ExtraSiteWrapper(BrowserType browserType, File downloadDir)
         this.extraDriver = createNewWebDriver(browserType, downloadDir);
     }
 
+    public ExtraSiteWrapper(WebDriver driver)
+    {
+        super();
+        this.extraDriver = new ImmutablePair<>(driver, null);
+    }
+
     @Override
     public void close()
     {

src/org/labkey/test/Locator.java

@@ -1364,17 +1364,18 @@ public XPathLocator withPredicate(XPathLocator descendant)
 
         public XPathLocator withPredicate(@Language("XPath") String predicate)
         {
-            return this.append("[" + getRelativeXPath(predicate) + "]");
+            // Make compatible with Chrome "/../self::*[predicate]" instead of "/..[predicate]"
+            return this.append((getLoc().endsWith("/..") ? "/self::*" : "") + "[" + getRelativeXPath(predicate) + "]");
         }
 
         public XPathLocator withoutPredicate(@Language("XPath") String predicate)
         {
-            return this.append("[not(" + getRelativeXPath(predicate) + ")]");
+            return this.withPredicate("not(" + getRelativeXPath(predicate) + ")");
         }
 
         public XPathLocator withoutPredicate(XPathLocator predicate)
         {
-            return this.append("[not(" + getRelativeXPath(predicate.toXpath()) + ")]");
+            return this.withoutPredicate(getRelativeXPath(predicate.toXpath()));
         }
 
         public XPathLocator attributeStartsWith(String attribute, String text)

src/org/labkey/test/TestProperties.java

@@ -38,8 +38,6 @@ public abstract class TestProperties
 
     static
     {
-        // https://github.com/SeleniumHQ/selenium/issues/11750#issuecomment-1470357124
-        System.setProperty("webdriver.http.factory", "jdk-http-client");
 
         final File propFile = new File(TestFileUtils.getTestRoot(), "test.properties");
         final File propFileTemplate = new File(TestFileUtils.getTestRoot(), "test.properties.template");

src/org/labkey/test/components/domain/DomainFormPanel.java

@@ -625,6 +625,7 @@ private List<DomainFieldRow> findFieldRows()
 
         private DomainFieldRow findFieldRow(String name)
         {
+            WebDriverWrapper.waitFor(() -> !findFieldRows().isEmpty(), 1_000);
             List<DomainFieldRow> fieldRows = findFieldRows();
             for (int i = 0; i < fieldRows.size(); i++)
             {

src/org/labkey/test/tests/JUnitTest.java

@@ -43,18 +43,20 @@
 import org.labkey.remoteapi.SimplePostCommand;
 import org.labkey.remoteapi.collections.CaseInsensitiveHashMap;
 import org.labkey.test.BaseWebDriverTest;
+import org.labkey.test.ExtraSiteWrapper;
+import org.labkey.test.LabKeySiteWrapper;
 import org.labkey.test.Runner;
 import org.labkey.test.SuiteFactory;
 import org.labkey.test.TestProperties;
-import org.labkey.test.TestTimeoutException;
+import org.labkey.test.WebDriverWrapper;
 import org.labkey.test.WebTestHelper;
 import org.labkey.test.categories.BVT;
 import org.labkey.test.categories.UnitTests;
+import org.labkey.test.util.ApiBootstrapHelper;
 import org.labkey.test.util.ArtifactCollector;
 import org.labkey.test.util.JUnitFooter;
 import org.labkey.test.util.JUnitHeader;
 import org.labkey.test.util.LogMethod;
-import org.labkey.test.util.QuickBootstrapPseudoTest;
 import org.labkey.test.util.TestLogger;
 
 import java.io.IOException;
@@ -108,47 +110,38 @@ public String toString()
         return getClass().getName();
     }
 
-    public static class JUnitSeleniumHelper extends BaseWebDriverTest
-    {
-        @Override
-        protected String getProjectName() {return null;}
-        @Override
-        protected void doCleanup(boolean afterTest) throws TestTimeoutException
-        { }
-        @Override
-        public List<String> getAssociatedModules() { return null; }
-
-        @Override public BrowserType bestBrowser() {return BrowserType.CHROME;}
-    }
-
     // Use WebDriver to ensure we're upgraded
     @LogMethod
     private static void upgradeHelper(boolean skipInitialUserChecks)
     {
         // TODO: remove upgrade helper from JUnitTest and run before suite starts.
-        BaseWebDriverTest helper;
+
         if (skipInitialUserChecks)
         {
-            helper = new QuickBootstrapPseudoTest();
-        }
-        else
-        {
-            helper = new JUnitSeleniumHelper();
+            new ApiBootstrapHelper().signIn();
+            return;
         }
+
+        ExtraSiteWrapper bootstrapBrowser = new ExtraSiteWrapper(WebDriverWrapper.BrowserType.FIREFOX,
+                ArtifactCollector.ensureDumpDir(JUnitTest.class.getSimpleName()));
+
         try
         {
-            helper.setUp();
             // sign in performs upgrade if necessary
-            helper.signIn();
+            bootstrapBrowser.signIn();
         }
         catch (Throwable t)
         {
-            if (helper.getWrappedDriver() != null)
+            if (bootstrapBrowser.getWrappedDriver() != null)
             {
-                helper.getArtifactCollector().dumpPageSnapshot("ServerBootstrap", null);
+                new ArtifactCollector(bootstrapBrowser, JUnitTest.class.getSimpleName()).dumpPageSnapshot("ServerBootstrap", null);
             }
             throw t;
         }
+        finally
+        {
+            bootstrapBrowser.close();
+        }
     }
 
     public static TestSuite dynamicSuite(Collection<String> categories, Collection<String> excludedCategories)

src/org/labkey/test/util/APIUserHelper.java

@@ -37,6 +37,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
@@ -45,9 +46,17 @@
 
 public class APIUserHelper extends AbstractUserHelper
 {
+    private final Supplier<Connection> connectionSupplier;
+    public APIUserHelper(Supplier<Connection> connectionSupplier)
+    {
+        super(null);
+        this.connectionSupplier = connectionSupplier;
+    }
+
     public APIUserHelper(WebDriverWrapper driver)
     {
         super(driver);
+        connectionSupplier = driver::createDefaultConnection;
     }
 
     @Override
@@ -82,7 +91,7 @@ public void _setDisplayName(String email, String newDisplayName)
 
         try
         {
-            new QueryApiHelper(getWrapper().createDefaultConnection(), "/", "core", "siteusers")
+            new QueryApiHelper(connectionSupplier.get(), "/", "core", "siteusers")
                     .updateRows(List.of(Maps.of("userId", userId, "DisplayName", newDisplayName)));
         }
         catch (IOException | CommandException e)
@@ -122,7 +131,7 @@ public JSONObject getJsonObject()
             }
         };
         command.setSendEmail(sendEmail);
-        Connection connection = getWrapper().createDefaultConnection();
+        Connection connection = connectionSupplier.get();
         try
         {
             CreateUserResponse response = command.execute(connection, "");
@@ -157,8 +166,8 @@ public GetUsersResponse getUsers(boolean includeInactive)
     {
         GetUsersCommand command = new GetUsersCommand();
         command.setIncludeInactive(includeInactive);
-        Connection connection = getWrapper().createDefaultConnection();
-        if (getWrapper().isImpersonating())
+        Connection connection = connectionSupplier.get();
+        if (getWrapper() != null && getWrapper().isImpersonating())
         {
             // Don't use browser session. Tests often call 'getDisplayNameForEmail' while impersonating non-admins.
             connection = WebTestHelper.getRemoteApiConnection(false);
@@ -205,11 +214,10 @@ protected void _deleteUser(String userEmail)
 
     private void deleteUser(@NotNull Integer userId)
     {
-        Connection connection = getWrapper().createDefaultConnection();
         DeleteUserCommand command = new DeleteUserCommand(userId);
         try
         {
-            command.execute(connection, "/");
+            command.execute(connectionSupplier.get(), "/");
         }
         catch (IOException|CommandException e)
         {