Merge 26.5 to develop (#2980)

Author: labkey-teamcity

build.gradle

@@ -182,9 +182,6 @@ project.tasks.named("uiTests").configure {
 project.parent.parent.tasks.named("ijConfigure").configure {
     dependsOn(initPropertiesTask)
 }
-project.tasks.withType(RunTestSuite).configureEach { it ->
-    scanForTestClasses = true // Required for Gradle 9.3
-}
 
 if (project.hasProperty('doPublishing'))
 {

src/org/labkey/test/Runner.java

@@ -129,21 +129,24 @@ private void updateRemainingTests(Test test, boolean failed, boolean errored)
         Class<?> testClass = getTestClass(test);
         _remainingTests.remove(testClass);
         if (failed)
-            _failedTests.add(test.toString());
+            _failedTests.add(getTestName(test));
         else if (errored)
-            _erroredTests.add(test.toString());
+            _erroredTests.add(getTestName(test));
         else
-            _passedTests.add(test.toString());
+            _passedTests.add(getTestName(test));
     }
 
     private static void writeRemainingTests()
     {
-        ArrayList<String> failedAndRemaining = new ArrayList<>();
-        failedAndRemaining.addAll(_failedTests);
-        failedAndRemaining.addAll(_erroredTests);
-        for (Class<?> clazz : _remainingTests)
-            failedAndRemaining.add(clazz.getName());
-        writeClasses(failedAndRemaining, getRemainingTestsFile());
+        if (!TestProperties.isTestRunningOnTeamCity()) // Not useful on TeamCity
+        {
+            ArrayList<String> failedAndRemaining = new ArrayList<>();
+            failedAndRemaining.addAll(_failedTests);
+            failedAndRemaining.addAll(_erroredTests);
+            for (Class<?> clazz : _remainingTests)
+                failedAndRemaining.add(clazz.getName());
+            writeClasses(failedAndRemaining, getRemainingTestsFile());
+        }
     }
 
     private static void writeClasses(List<String> tests, File file)
@@ -203,7 +206,7 @@ private static Class<?>[] readClasses(File recentlyFailedTestsFile, List<Class<?
                 filteredRecentlyFailedTests.add(item);
             }
         }
-        
+
         return filteredRecentlyFailedTests.toArray(new Class[0]);
     }
 
@@ -273,8 +276,7 @@ else if (test instanceof JUnit4TestAdapter adapter)
 
             if (_failedTests.size() + _erroredTests.size() < _maxTestFailures || _maxTestFailures <= 0)
             {
-                final Class<?> currentTestClass = getTestClass(test);
-                final String currentTestName = currentTestClass.getSimpleName();
+                final String currentTestName = getTestName(test);
 
                 TestListener classFailListener = new TestListener()
                 {
@@ -298,13 +300,16 @@ public void endTest(Test _test) { }
                 };
                 testResult.addListener(classFailListener);
 
-                logToServer("=== Starting " + currentTestName + getProgress() + " ===");
-                LOG.info("=============== Starting " + currentTestName + getProgress() + " =================");
+                if (!(test instanceof JUnitTest.RemoteTest))
+                {
+                    logToServer("=== Starting " + currentTestName + getProgress() + " ===");
+                    LOG.info("=============== Starting " + currentTestName + getProgress() + " =================");
+                }
 
                 // This stub matches the failure generated by JUnit when it fails during static setup/teardown (e.g. @BeforeClass)
                 // Without this TeamCity has no way of knowing when a setup/teardown failure has been resolved
-                final Test loggingStub = test instanceof JUnit4TestAdapter ?
-                        new TestSuite(currentTestClass) :
+                final Test loggingStub = test instanceof JUnit4TestAdapter jUnit4TestAdapter ?
+                        new TestSuite(jUnit4TestAdapter.getTestClass()) :
                         null;
 
                 if (loggingStub != null)
@@ -317,10 +322,13 @@ public void endTest(Test _test) { }
 
                 testResult.removeListener(classFailListener);
 
-                String result = failed.booleanValue() || errored.booleanValue() ? "Failed " : "Completed ";
-                TestLogger.resetLogger();
-                LOG.info("=============== " + result + currentTestName + getProgress() + " =================");
-                logToServer("=== " + result + currentTestName + getProgress() + " ===");
+                if (!(test instanceof JUnitTest.RemoteTest))
+                {
+                    String result = failed.booleanValue() || errored.booleanValue() ? "Failed " : "Completed ";
+                    TestLogger.resetLogger();
+                    LOG.info("=============== " + result + currentTestName + getProgress() + " =================");
+                    logToServer("=== " + result + currentTestName + getProgress() + " ===");
+                }
 
             }
             else
@@ -547,7 +555,7 @@ public String describe()
             }
             else if (test.countTestCases() > 0)
             {
-                suite.addTest(test);
+                flattenSuiteInto(suite, (TestSuite) test);
                 foundServerSideTest = true;
             }
         }
@@ -570,7 +578,7 @@ else if (specifiedSuite.startsWith("?") && specifiedSuite.length() > 1)
             }
             TestSuite dynamicSuite = JUnitTest.dynamicSuite(requestedSuites, excludedSuites);
             if (dynamicSuite.countTestCases() > 0)
-                suite.addTest(dynamicSuite);
+                flattenSuiteInto(suite, dynamicSuite);
         }
     }
 
@@ -603,9 +611,9 @@ public void run(TestResult testResult)
 
     private static void writeTimeReport()
     {
-        int width = 60;
+        int width = 64;
         long total = 0;
-        LOG.info("======================= Time Report ========================");
+        LOG.info(getCenteredText("Time Report", '=', width));
 
         Duration totalCrawlTime = Duration.ZERO;
         int totalUniquePages = 0;
@@ -619,7 +627,7 @@ private static void writeTimeReport()
         }
         for (Map.Entry<Test, Long> entry : _testStats.entrySet())
         {
-            String testName = entry.getKey().toString();
+            String testName = getTestName(entry.getKey());
             long duration = entry.getValue();
 
             long percent = Math.round(100.0 * (duration / (double) total));
@@ -630,7 +638,6 @@ private static void writeTimeReport()
                         (_erroredTests.contains(testName) ? "ERROR" : "not run"))) +
                 " - " +
                 formatDuration(duration) + " " + percentStr;
-            testName = testName.substring(testName.lastIndexOf('.') + 1);
 
             LOG.info(getFixedWidthString(testName, durationAndPercent, width));
 
@@ -686,7 +693,7 @@ private static void writeTimeReport()
         }
         if (!TeamCityUtils.getBuildStatistics().isEmpty())
         {
-            LOG.info("--------------------- Build Statistics ---------------------");
+            LOG.info(getCenteredText("Build Statistics", '-', width));
             for (String stat : TeamCityUtils.getBuildStatistics().keySet())
             {
                 List<Number> values = TeamCityUtils.getBuildStatistics().get(stat);
@@ -702,7 +709,7 @@ private static void writeTimeReport()
         Map<String, Collection<String>> actionWarnings = WebDriverWrapper.getActionWarnings();
         if (!actionWarnings.isEmpty())
         {
-            LOG.info("---------------------- Test Warnings -----------------------");
+            LOG.info(getCenteredText("Test Warnings", '-', width));
             for (String warning : actionWarnings.keySet())
             {
                 LOG.info("  " + warning + ":");
@@ -714,7 +721,7 @@ private static void writeTimeReport()
                 }
             }
         }
-        LOG.info("------------------------------------------------------------");
+        LOG.info("-".repeat(width));
         LOG.info(getFixedWidthString("Total duration:", formatDuration(total), width) + "\n");
         LOG.info("Completed " + FastDateFormat.getInstance("yyyy-MM-dd HH:mm").format(new Date()));
     }
@@ -744,10 +751,23 @@ private static String getRowString(String[] list, int columnWidth)
     private static String getFixedWidthString(String prefix, String suffix, int length)
     {
         int contentLength = prefix.length() + suffix.length();
-        int padding = Math.max(0, length - contentLength);
+        int padding = Math.max(1, length - contentLength);
         return prefix + " ".repeat(padding) + suffix;
     }
 
+    private static String getCenteredText(String text, char paddingChar, int width)
+    {
+        text = StringUtils.trimToEmpty(text);
+        if (text.length() > width)
+            return text;
+
+        text = text.isEmpty() ? "" : " " + text + " ";
+        int paddingLength = width - text.length();
+        int leftPadding = paddingLength / 2;
+        int rightPadding = paddingLength - leftPadding;
+        return Character.toString(paddingChar).repeat(leftPadding) + text + Character.toString(paddingChar).repeat(rightPadding);
+    }
+
     private static TestSet getCompositeTestSet(List<String> suitesColl)
     {
         if (suitesColl.isEmpty())
@@ -1021,7 +1041,7 @@ else if (testNames.isEmpty())
                 Test test = e.nextElement();
                 Class<?> testClass = getTestClass(test);
                 _remainingTests.add(testClass);
-                LOG.info("  " + testClass.getSimpleName());
+                LOG.info("  " + getTestName(test));
                 for (String testMethod : specifiedTestMethods.getOrDefault(testClass, Collections.emptyList()))
                 {
                     LOG.info("    ." + testMethod);
@@ -1126,10 +1146,43 @@ private static String getModuleNameFromPath(File path)
         return null;
     }
 
+    private static void flattenSuiteInto(TestSuite destination, TestSuite source)
+    {
+        if (TestProperties.isTestRunningOnTeamCity())
+        {
+            // Flatten test suites so that TeamCity will correlate with results from before Gradle 9.3
+            Enumeration<Test> tests = source.tests();
+            while (tests.hasMoreElements())
+            {
+                Test t = tests.nextElement();
+                if (t instanceof TestSuite nested)
+                    flattenSuiteInto(destination, nested);
+                else
+                    destination.addTest(t);
+            }
+        }
+        else
+        {
+            destination.addTest(source);
+        }
+    }
+
+    private static String getTestName(Test test)
+    {
+        if (test instanceof JUnit4TestAdapter testAdapter)
+            return testAdapter.getTestClass().getSimpleName();
+        else if (test instanceof JUnitTest.RemoteTest remoteTest)
+            return remoteTest.getName();
+        else
+            return test.getClass().getSimpleName();
+    }
+
     private static Class<?> getTestClass(Test test)
     {
-        if (test instanceof JUnit4TestAdapter)
-            return ((JUnit4TestAdapter) test).getTestClass();
+        if (test instanceof JUnit4TestAdapter testAdapter)
+            return testAdapter.getTestClass();
+        else if (test instanceof JUnitTest.RemoteTest)
+            return JUnitTest.class;
         else
             return test.getClass();
     }

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

@@ -303,14 +303,18 @@ public void saveFilterTest()
         FieldKey fieldKey = FieldKey.fromParts(LAST_NAME_COLUMN);
         String op = "Starts With";
         String value = "J";
-        String[] viewNames = {TRICKY_CHARACTERS + "view", "AAC", "aaa", "aad", "zzz"};
+        String[] viewNames = {TRICKY_CHARACTERS + "view", "AAC", "aaa", "aad", "zzz", "view,with,comma"};
 
         setColumns(LAST_NAME_COLUMN);
         for(String name : viewNames)
         {
             _customizeViewsHelper.openCustomizeViewPanel();
             _customizeViewsHelper.addFilter(fieldKey, op, value);
             _customizeViewsHelper.saveCustomView(name);
+            _customizeViewsHelper.openCustomizeViewPanel();
+            // GitHub Issue #936 : ensure custom view can be edited
+            assertElementNotPresent(Locator.tagWithClass("div", "alert-warning").withText(String.format("Custom Grid View '%s' not found.", name)));
+            _customizeViewsHelper.closePanel();
         }
 
         DataRegionTable drt = new DataRegionTable("query", getDriver());

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

@@ -84,7 +84,7 @@ public JUnitTest()
 
     public static TestSuite suite() throws Exception
     {
-        return JUnitTest._suite((p) -> true, false);
+        return JUnitTest._suite(_ -> true, false);
     }
 
     private static String getWhen(Map<String,Object> test)
@@ -136,12 +136,9 @@ private static void upgradeHelper(boolean skipInitialUserChecks)
         }
         catch (Throwable t)
         {
-            if (bootstrapBrowser.getWrappedDriver() != null)
-            {
-                ArtifactCollector artifactCollector = new ArtifactCollector(bootstrapBrowser, JUnitTest.class.getSimpleName());
-                artifactCollector.dumpPageSnapshot("ServerBootstrap", null);
-                artifactCollector.publishDumpedArtifacts();
-            }
+            ArtifactCollector artifactCollector = new ArtifactCollector(bootstrapBrowser, JUnitTest.class.getSimpleName());
+            artifactCollector.dumpPageSnapshot("ServerBootstrap", null);
+            artifactCollector.publishDumpedArtifacts();
             throw t;
         }
         finally
@@ -176,8 +173,7 @@ public static TestSuite dynamicSuite(Collection<String> categories, Collection<S
         }
         catch (Throwable t)
         {
-            LOG.error("Unable to fetch Remote JUnit tests");
-            t.printStackTrace();
+            LOG.error("Unable to fetch Remote JUnit tests", t);
             TestSuite testSuite = new TestSuite();
             testSuite.addTest(new Runner.ErrorTest(JUnitTest.class.getSimpleName(), t));
             return testSuite;
@@ -289,7 +285,7 @@ else if (responseBody.contains("<title>Upgrade Status</title>") ||
                         catch (Throwable t)
                         {
                             upgradeError = t;
-                            t.printStackTrace();
+                            LOG.warn("Error during upgrade/bootstrap", t);
                         }
                         TestSuite testSuite;
                         try
@@ -298,7 +294,7 @@ else if (responseBody.contains("<title>Upgrade Status</title>") ||
                         }
                         catch (Exception retryException)
                         {
-                            retryException.printStackTrace();
+                            LOG.warn("Error fetching remote test suite", retryException);
                             testSuite = new TestSuite();
                             testSuite.addTest(new Runner.ErrorTest("", retryException));
                         }
@@ -321,7 +317,7 @@ else if (responseBody.contains("<title>Upgrade Status</title>") ||
                 for (String key : json.keySet())
                 {
                     AtomicInteger ioeCounter = new AtomicInteger(0);
-                    TestSuite testsuite = new TestSuite(key);
+                    TestSuite moduleSuite = new TestSuite(key + " Tests");
                     JSONArray testClassArray = json.getJSONArray(key);
                     // Individual tests include both the class name and the requested timeout
                     for (int i = 0; i < testClassArray.length(); i++)
@@ -334,17 +330,20 @@ else if (responseBody.contains("<title>Upgrade Status</title>") ||
                             // Timeout is represented in seconds
                             int timeout = testClass.getInt("timeout");
                             if (accept.test(testClass.toMap()))
-                                testsuite.addTest(new RemoteTest(className, timeout, ioeCounter));
+                                moduleSuite.addTest(new RemoteTest(className, timeout, ioeCounter));
                         }
 
                     }
-                    if (!addedHeader && testsuite.countTestCases() > 0)
+                    if (moduleSuite.countTestCases() > 0)
                     {
-                        BaseJUnitTestWrapper.extraSetup = !skipInitialUserChecks;
-                        remotesuite.addTest(new JUnit4TestAdapter(JUnitHeader.class));
-                        addedHeader = true;
+                        if (!addedHeader)
+                        {
+                            BaseJUnitTestWrapper.extraSetup = !skipInitialUserChecks;
+                            remotesuite.addTest(new JUnit4TestAdapter(JUnitHeader.class));
+                            addedHeader = true;
+                        }
+                        remotesuite.addTest(moduleSuite);
                     }
-                    remotesuite.addTest(testsuite);
                 }
                 if (addedHeader)
                 {