From ca3a3e3643ddd6d838437b4e4e5a82385aa00953 Mon Sep 17 00:00:00 2001 From: Trey Chadick Date: Tue, 6 Jan 2026 09:00:35 -0800 Subject: [PATCH 1/4] Minor test fixes for intermittent failures (#2828) - Wait for file browser checkbox to appear - Stop running server-side JUnit tests after 5 consecutive timeouts - Add missing `conceptURI` to `ColumnType.VisitLabel` - Add retry to `ProjectMenu.open` --- .../test/components/core/ProjectMenu.java | 27 ++++++++++++------- .../labkey/test/params/FieldDefinition.java | 2 +- src/org/labkey/test/tests/JUnitTest.java | 16 ++++++++--- .../labkey/test/util/FileBrowserHelper.java | 2 +- src/org/labkey/test/util/TestLogger.java | 12 ++++----- 5 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/org/labkey/test/components/core/ProjectMenu.java b/src/org/labkey/test/components/core/ProjectMenu.java index ff029ae5c1..ecff1d476b 100644 --- a/src/org/labkey/test/components/core/ProjectMenu.java +++ b/src/org/labkey/test/components/core/ProjectMenu.java @@ -15,6 +15,7 @@ */ package org.labkey.test.components.core; +import org.apache.commons.lang3.StringUtils; import org.labkey.test.Locator; import org.labkey.test.WebDriverWrapper; import org.labkey.test.components.Component; @@ -28,6 +29,7 @@ import java.util.List; +import static org.labkey.test.Locators.loadingSpinner; import static org.labkey.test.WebDriverWrapper.WAIT_FOR_JAVASCRIPT; /** @@ -68,20 +70,27 @@ public String getCurrentProject() private boolean isOpen() { - return elementCache().menuContainer.getAttribute("class").contains("open"); + return StringUtils.trimToEmpty(elementCache().menuContainer.getAttribute("class")).contains("open") && + !loadingSpinner.existsIn(elementCache().menuContainer); } public ProjectMenu open() { if (!isOpen()) { - getWrapper().executeScript("window.scrollTo(0,0);"); - if (getWrapper().isElementPresent(Locator.css("li.dropdown.open > .lk-custom-dropdown-menu"))) - getWrapper().mouseOver(elementCache().menuToggle); // Just need to hover if another menu is already open - else - elementCache().menuToggle.click(); - WebDriverWrapper.waitFor(this::isOpen, "Project menu didn't open", 2000); - getWrapper().waitForElement(Locator.tagWithClass("div", "folder-nav")); + Runnable openMenu = () -> { + getWrapper().executeScript("window.scrollTo(0,0);"); + if (getWrapper().isElementPresent(Locator.css("li.dropdown.open > .lk-custom-dropdown-menu"))) + getWrapper().mouseOver(elementCache().menuToggle); // Just need to hover if another menu is already open + else + elementCache().menuToggle.click(); + WebDriverWrapper.waitFor(this::isOpen, "Project menu didn't open", 2000); + }; + openMenu.run(); + if (!Locator.tagWithClass("div", "folder-nav").existsIn(this)) + { + openMenu.run(); // retry + } } return this; } @@ -222,7 +231,7 @@ protected ElementCache newElementCache() return new ElementCache(); } - protected class ElementCache extends Component.ElementCache + protected class ElementCache extends Component.ElementCache { final WebElement menuContainer = Locators.menuProjectNav.refindWhenNeeded(getComponentElement()); final WebElement menuToggle = Locator.tagWithAttribute("a", "data-toggle", "dropdown").refindWhenNeeded(menuContainer); diff --git a/src/org/labkey/test/params/FieldDefinition.java b/src/org/labkey/test/params/FieldDefinition.java index 568e903161..15f92c02e9 100644 --- a/src/org/labkey/test/params/FieldDefinition.java +++ b/src/org/labkey/test/params/FieldDefinition.java @@ -590,7 +590,7 @@ public boolean isMeasureByDefault() ColumnType OntologyLookup = new ColumnTypeImpl("Ontology Lookup", "string", "http://www.labkey.org/types#conceptCode", null); ColumnType VisitId = new ColumnTypeImpl("Visit ID", "double", "http://cpas.labkey.com/Study#VisitId", null); ColumnType VisitDate = new ColumnTypeImpl("Visit Date", "dateTime", "http://cpas.labkey.com/Study#VisitId", null); - ColumnType VisitLabel = new ColumnTypeImpl("Visit Label", "string"); + ColumnType VisitLabel = new ColumnTypeImpl("Visit Label", "string", "http://cpas.labkey.com/Study#VisitId", null); ColumnType Sample = new ColumnTypeImpl("Sample", "int", "http://www.labkey.org/exp/xml#sample", new IntLookup( "exp", "Materials")); ColumnType Barcode = new ColumnTypeImpl("Unique ID", "string", "http://www.labkey.org/types#storageUniqueId", null); ColumnType TextChoice = new ColumnTypeImpl("Text Choice", "string", "http://www.labkey.org/types#textChoice", null); diff --git a/src/org/labkey/test/tests/JUnitTest.java b/src/org/labkey/test/tests/JUnitTest.java index 5cfa9cdc0e..2b785e1fee 100644 --- a/src/org/labkey/test/tests/JUnitTest.java +++ b/src/org/labkey/test/tests/JUnitTest.java @@ -36,6 +36,7 @@ import org.json.JSONException; import org.json.JSONObject; import org.junit.Assert; +import org.junit.Assume; import org.junit.experimental.categories.Category; import org.labkey.remoteapi.CommandException; import org.labkey.remoteapi.CommandResponse; @@ -69,6 +70,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; @Category({BVT.class, UnitTests.class}) @@ -318,6 +320,7 @@ else if (responseBody.contains("Upgrade Status") || boolean addedHeader = false; for (String key : json.keySet()) { + AtomicInteger ioeCounter = new AtomicInteger(0); TestSuite testsuite = new TestSuite(key); JSONArray testClassArray = json.getJSONArray(key); // Individual tests include both the class name and the requested timeout @@ -331,7 +334,7 @@ else if (responseBody.contains("Upgrade Status") || // Timeout is represented in seconds int timeout = testClass.getInt("timeout"); if (accept.test(testClass.toMap())) - testsuite.addTest(new RemoteTest(className, timeout)); + testsuite.addTest(new RemoteTest(className, timeout, ioeCounter)); } } @@ -370,23 +373,27 @@ else if (responseBody.contains("Upgrade Status") || @SuppressWarnings("JUnitMalformedDeclaration") public static class RemoteTest extends TestCase { - String _remoteClass; + private final String _remoteClass; /** Timeout in seconds to wait for the whole testcase to finish on the server */ private final int _timeout; + // Skip tests after a certain number of IOExceptions + private final AtomicInteger _ioeCounter; /** Stash and reuse so that we can keep using the same session instead of re-authenticating with every request */ private static final Connection connection = WebTestHelper.getRemoteApiConnection(); - public RemoteTest(String remoteClass, int timeout) + public RemoteTest(String remoteClass, int timeout, AtomicInteger ioeCounter) { super(remoteClass); _remoteClass = remoteClass; _timeout = timeout; + _ioeCounter = ioeCounter; } @Override protected void runTest() { + Assume.assumeTrue("Too many consecutive test timeouts", _ioeCounter.get() < 5); long startTime = System.currentTimeMillis(); try { @@ -405,9 +412,11 @@ else if (resultJson.get("wasSuccessful") != Boolean.TRUE) WebTestHelper.logToServer(getLogTestString("successful", startTime) + ", " + dump(resultJson, false), connection); LOG.info(getLogTestString("successful", startTime)); LOG.info(dump(resultJson, true)); + _ioeCounter.set(0); } catch (SocketTimeoutException ste) { + _ioeCounter.incrementAndGet(); String timed_out = getLogTestString("timed out", startTime); LOG.error(timed_out); ArtifactCollector.dumpThreads(); @@ -415,6 +424,7 @@ else if (resultJson.get("wasSuccessful") != Boolean.TRUE) } catch (IOException ioe) { + _ioeCounter.incrementAndGet(); String message = getLogTestString("failed: " + ioe.getMessage(), startTime); LOG.error(message); throw new RuntimeException(message, ioe); diff --git a/src/org/labkey/test/util/FileBrowserHelper.java b/src/org/labkey/test/util/FileBrowserHelper.java index 72a403853c..8562caa4a7 100644 --- a/src/org/labkey/test/util/FileBrowserHelper.java +++ b/src/org/labkey/test/util/FileBrowserHelper.java @@ -248,7 +248,7 @@ private void checkFileBrowserFileCheckbox(String fileName, boolean checkTheBox) final Checkbox checkbox; try { - checkbox = Ext4Checkbox().locatedBy(Locators.gridRowCheckbox(fileName)).find(getDriver()); + checkbox = Ext4Checkbox().locatedBy(Locators.gridRowCheckbox(fileName)).timeout(1_000).find(getDriver()); } catch (NoSuchElementException nse) { diff --git a/src/org/labkey/test/util/TestLogger.java b/src/org/labkey/test/util/TestLogger.java index 827377320b..f3b5415dc3 100644 --- a/src/org/labkey/test/util/TestLogger.java +++ b/src/org/labkey/test/util/TestLogger.java @@ -138,12 +138,12 @@ public static void log(String str) } /** - * Format an elapsed time to be suitable for log messages. - * Over one minute: - * " <1m 25s>" - * Over one minute: - * " <8.059s>" - * Less than on second: + * Format an elapsed time to be suitable for log messages.
+ * Over one minute:
+ * " <1m 25s>"
+ * Over one second:
+ * " <8.059s>"
+ * Less than one second:
* " <125ms>" * @param milliseconds Elapsed time in milliseconds * @return Formatted time From 99a9e3a08dacc4bbef5468450b4b812db5e14d3c Mon Sep 17 00:00:00 2001 From: Trey Chadick Date: Tue, 6 Jan 2026 11:46:36 -0800 Subject: [PATCH 2/4] Update expected error message for knitr report (#2835) Handle minor knitr formatting variations --- src/org/labkey/test/tests/AbstractKnitrReportTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/org/labkey/test/tests/AbstractKnitrReportTest.java b/src/org/labkey/test/tests/AbstractKnitrReportTest.java index 5236ed6dce..518707fd5f 100644 --- a/src/org/labkey/test/tests/AbstractKnitrReportTest.java +++ b/src/org/labkey/test/tests/AbstractKnitrReportTest.java @@ -175,7 +175,7 @@ protected void htmlFormat() Locator.tag("img").withAttribute("alt", "plot of chunk blood-pressure-scatter")), // new Locator.tag("pre").containing("## \"1\",249318596,\"2008-05-17\",86,36,129,76,64"), Locator.tag("pre").withText("## knitr says hello to HTML!"), - Locator.tag("pre").startsWith("## Error").containing(": non-numeric argument to binary operator"), + Locator.tag("pre").startsWith("## Error").containing("non-numeric argument to binary operator"), Locator.tag("p").startsWith("Well, everything seems to be working. Let's ask R what is the value of \u03C0? Of course it is 3.141"), nonceCheckSuccessLoc // Inline script should run }; From e38f0faae2281a0e0297060dd485e96234674e25 Mon Sep 17 00:00:00 2001 From: Trey Chadick Date: Thu, 15 Jan 2026 15:32:11 -0800 Subject: [PATCH 3/4] Add SimpleFormCommand and backport upgrade test conditions (#2845) --- .../labkey/remoteapi/SimpleFormCommand.java | 43 ++++++++++++++++ .../test/tests/upgrade/BaseUpgradeTest.java | 34 +++++++++++-- src/org/labkey/test/util/VersionRange.java | 50 +++++++++++++++++-- 3 files changed, 118 insertions(+), 9 deletions(-) create mode 100644 src/org/labkey/remoteapi/SimpleFormCommand.java diff --git a/src/org/labkey/remoteapi/SimpleFormCommand.java b/src/org/labkey/remoteapi/SimpleFormCommand.java new file mode 100644 index 0000000000..e49a74dbd0 --- /dev/null +++ b/src/org/labkey/remoteapi/SimpleFormCommand.java @@ -0,0 +1,43 @@ +package org.labkey.remoteapi; + +import org.apache.hc.client5.http.classic.methods.HttpPost; +import org.apache.hc.client5.http.classic.methods.HttpUriRequest; +import org.apache.hc.client5.http.entity.UrlEncodedFormEntity; +import org.apache.hc.core5.http.NameValuePair; +import org.apache.hc.core5.http.message.BasicNameValuePair; + +import java.io.IOException; +import java.net.URI; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +public class SimpleFormCommand extends Command +{ + private final Map _formData; + + public SimpleFormCommand(String controller, String action, Map formData) + { + super(controller, action); + _formData = formData; + } + + public CommandResponse execute(Connection connection) throws IOException, CommandException + { + return execute(connection, null); + } + + @Override + protected HttpUriRequest createRequest(URI uri) + { + HttpPost post = new HttpPost(uri); + + List args = new ArrayList<>(); + for (Map.Entry reportVal : _formData.entrySet()) + { + args.add(new BasicNameValuePair(reportVal.getKey(), reportVal.getValue())); + } + post.setEntity(new UrlEncodedFormEntity(args)); + return post; + } +} diff --git a/src/org/labkey/test/tests/upgrade/BaseUpgradeTest.java b/src/org/labkey/test/tests/upgrade/BaseUpgradeTest.java index 33fe474029..a7e3b20568 100644 --- a/src/org/labkey/test/tests/upgrade/BaseUpgradeTest.java +++ b/src/org/labkey/test/tests/upgrade/BaseUpgradeTest.java @@ -35,8 +35,9 @@ public abstract class BaseUpgradeTest extends BaseWebDriverTest { protected static final boolean isUpgradeSetupPhase = TestProperties.getBooleanProperty("webtest.upgradeSetup", true); - protected static final Version previousVersion = Optional.ofNullable(trimToNull(System.getProperty("webtest.upgradePreviousVersion"))) - .map(Version::new).orElse(TestProperties.getProductVersion()); + protected static final Version setupVersion = isUpgradeSetupPhase ? TestProperties.getProductVersion() : + Optional.ofNullable(trimToNull(System.getProperty("webtest.upgradePreviousVersion"))).map(Version::new) + .orElse(TestProperties.getProductVersion()); @Override protected boolean skipCleanup(boolean afterTest) @@ -70,6 +71,29 @@ public List getAssociatedModules() return Arrays.asList(); } + /** + * Checks if the setup for the current test was performed in a version prior to the specified version. + * + * @param version The version to check against. + * @return {@code true} if the setup version is earlier than the specified version. + */ + protected boolean wasSetupBefore(String version) + { + return !wasSetupWithin(version, null); + } + + /** + * Checks if the setup for the current test was performed within the specified version range (inclusive). + * + * @param earliestVersion The earliest version in the range (inclusive). + * @param latestVersion The latest version in the range (inclusive). + * @return {@code true} if the setup version is within the specified range. + */ + protected boolean wasSetupWithin(String earliestVersion, String latestVersion) + { + return VersionRange.versionRange(earliestVersion, latestVersion).contains(setupVersion); + } + /** * Annotates test methods that should only run when upgrading from particular LabKey versions, as specified in * {@code webtest.upgradePreviousVersion}.
@@ -104,7 +128,7 @@ private static class UpgradeVersionCheck implements TestRule String latestVersion = Optional.ofNullable(description.getAnnotation(LatestVersion.class)) .map(LatestVersion::value).orElse(null); - if (isUpgradeSetupPhase || previousVersion == null || (earliestVersion == null && latestVersion == null)) + if (isUpgradeSetupPhase || setupVersion == null || (earliestVersion == null && latestVersion == null)) { return base; // Run the test normally } @@ -114,8 +138,8 @@ private static class UpgradeVersionCheck implements TestRule @Override public void evaluate() throws Throwable { - Assume.assumeTrue("Test not valid when upgrading from version: " + previousVersion, - VersionRange.versionRange(earliestVersion, latestVersion).contains(previousVersion) + Assume.assumeTrue("Test not valid when upgrading from version: " + setupVersion, + VersionRange.versionRange(earliestVersion, latestVersion).contains(setupVersion) ); base.evaluate(); } diff --git a/src/org/labkey/test/util/VersionRange.java b/src/org/labkey/test/util/VersionRange.java index 0fe8220e5a..865974783e 100644 --- a/src/org/labkey/test/util/VersionRange.java +++ b/src/org/labkey/test/util/VersionRange.java @@ -1,26 +1,62 @@ package org.labkey.test.util; +/** + * Represents a range of LabKey versions. + * Used for checking if a specific version falls within an inclusive range. + */ public class VersionRange { - private final Version eariestVersion; + private final Version earliestVersion; private final Version latestVersion; - public VersionRange(Version eariestVersion, Version latestVersion) + /** + * Constructs a version range. + * + * @param earliestVersion The earliest version in the range (inclusive). If null, there is no lower bound. + * @param latestVersion The latest version in the range (inclusive). If null, there is no upper bound. + * @throws IllegalArgumentException if both versions are null, or if earliestVersion is after latestVersion. + */ + public VersionRange(Version earliestVersion, Version latestVersion) { - this.eariestVersion = eariestVersion; + this.earliestVersion = earliestVersion; this.latestVersion = latestVersion; + + if (earliestVersion == null && latestVersion == null) + throw new IllegalArgumentException("Version range requires at least one version"); + if (earliestVersion != null && latestVersion != null && earliestVersion.compareTo(latestVersion) > 0) + throw new IllegalArgumentException("%s is after %s".formatted(earliestVersion, latestVersion)); + } + /** + * Creates a version range starting from the specified version with no upper bound. + * + * @param version The earliest version (inclusive). + * @return A new {@link VersionRange}. + */ public static VersionRange from(String version) { return new VersionRange(new Version(version), null); } + /** + * Creates a version range ending at the specified version with no lower bound. + * + * @param version The latest version (inclusive). + * @return A new {@link VersionRange}. + */ public static VersionRange until(String version) { return new VersionRange(null, new Version(version)); } + /** + * Creates a version range with specified bounds. + * + * @param earliestVersion The earliest version (inclusive). If null, there is no lower bound. + * @param latestVersion The latest version (inclusive). If null, there is no upper bound. + * @return A new {@link VersionRange}. + */ public static VersionRange versionRange(String earliestVersion, String latestVersion) { Version earliest = earliestVersion == null ? null : new Version(earliestVersion); @@ -28,9 +64,15 @@ public static VersionRange versionRange(String earliestVersion, String latestVer return new VersionRange(earliest, latest); } + /** + * Checks if the specified version is within this range (inclusive). + * + * @param version The version to check. + * @return {@code true} if the version is within the range. + */ public boolean contains(Version version) { - return (eariestVersion == null || eariestVersion.compareTo(version) <= 0) && + return (earliestVersion == null || earliestVersion.compareTo(version) <= 0) && (latestVersion == null || latestVersion.compareTo(version) >= 0); } } From 88bb415cc3712e7bad119d1af0565f680212082f Mon Sep 17 00:00:00 2001 From: Trey Chadick Date: Fri, 16 Jan 2026 10:41:16 -0800 Subject: [PATCH 4/4] Close empty project menu before retrying open (#2846) --- src/org/labkey/test/components/core/ProjectMenu.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/org/labkey/test/components/core/ProjectMenu.java b/src/org/labkey/test/components/core/ProjectMenu.java index ecff1d476b..9263cce3b1 100644 --- a/src/org/labkey/test/components/core/ProjectMenu.java +++ b/src/org/labkey/test/components/core/ProjectMenu.java @@ -89,6 +89,8 @@ public ProjectMenu open() openMenu.run(); if (!Locator.tagWithClass("div", "folder-nav").existsIn(this)) { + // Menu opened but the folder list didn't load + close(); openMenu.run(); // retry } }