From 3cc052680a392e85e3863ea3ce6771eac94d8c50 Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Fri, 28 Aug 2026 14:27:24 -0700 Subject: [PATCH] Parameterize tests that loop over DbScopes DialectTestCase, GroupConcatTestCase, and Method.TestCase each looped over every DbScope in one @Test method, so a failure on one scope aborted every scope after it; each now runs as an independent JUnit case per scope via a shared JunitUtil.getDbScopesTestParameters() helper. Split out LabKeyScopeDialectTestCase and Method.IsDistinctFromMethodTestCase for the test methods that never depended on the loop, since JUnit's Parameterized runner requires every test in the class to share the same constructor; registered both new classes in CoreModule/QueryModule. --- api/src/org/labkey/api/data/DbScope.java | 98 ++++++++++++------- .../labkey/api/data/dialect/SqlDialect.java | 37 ++++--- api/src/org/labkey/api/util/JunitUtil.java | 44 +++++++++ core/src/org/labkey/core/CoreModule.java | 1 + query/src/org/labkey/query/QueryModule.java | 1 + query/src/org/labkey/query/sql/Method.java | 40 +++++--- 6 files changed, 160 insertions(+), 61 deletions(-) diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index 1dcd1542550..e1098b7d922 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -26,11 +26,15 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.junit.Assert; +import org.junit.Assume; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.labkey.api.action.ApiUsageException; import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.cache.Cache; import org.labkey.api.data.ConnectionWrapper.Closer; +import org.labkey.api.data.dialect.SimpleSqlDialect; import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.data.dialect.SqlDialect.DataSourcePropertyReader; import org.labkey.api.data.dialect.SqlDialectManager; @@ -49,6 +53,7 @@ import org.labkey.api.util.DeadlockPreventingException; import org.labkey.api.util.DebugInfoDumper; import org.labkey.api.util.GUID; +import org.labkey.api.util.JunitUtil; import org.labkey.api.util.LoggerWriter; import org.labkey.api.util.MemTracker; import org.labkey.api.util.ResultSetUtil; @@ -2145,7 +2150,8 @@ public static DbScope getDbScope(String dsName) /** * Some DbScopes shouldn't be exercised by junit tests (e.g., an external data source connected to LabKey Server via * the PostgreSQL wire protocol) - * Tests that use this should be annotated with '@TestWhen(TestWhen.When.DB_SCOPE)' + * Tests that use this should be annotated with '@TestWhen(TestWhen.When.DBSCOPE)' to ensure they run in suites that + * configure external data sources on TeamCity. * * @return A collection of DbScopes that are suitable for testing */ @@ -2962,46 +2968,57 @@ public void afterLoadTable(SchemaTableInfo ti) // Test dialects that are in-use; only for tests that require connecting to the database. @TestWhen(TestWhen.When.DBSCOPE) + @RunWith(Parameterized.class) public static class DialectTestCase extends Assert { + @Parameterized.Parameters(name = "{1}") + public static Collection schemas() + { + return JunitUtil.getDbScopesTestParameters(); + } + + private final DbScope scope; + + public DialectTestCase(DbScope scope, String displayName) + { + this.scope = scope; + } + @Test - public void testAllScopes() throws SQLException, IOException + public void testKeywords() throws SQLException, IOException { - for (DbScope scope : getDbScopesToTest()) - { - SqlDialect dialect = scope.getSqlDialect(); + SqlDialect dialect = scope.getSqlDialect(); - try (Connection conn = scope.getConnection()) - { - SqlExecutor executor = new SqlExecutor(scope, conn).setLogLevel(Level.OFF); // We're about to generate a lot of SQLExceptions - dialect.testDialectKeywords(executor); - dialect.testKeywordCandidates(executor); - } + try (Connection conn = scope.getConnection()) + { + SqlExecutor executor = new SqlExecutor(scope, conn).setLogLevel(Level.OFF); // We're about to generate a lot of SQLExceptions + dialect.testDialectKeywords(executor); + dialect.testKeywordCandidates(executor); } } @Test - public void testLabKeyScope() + public void testDateDiff() { - DbScope scope = getLabKeyScope(); SqlDialect dialect = scope.getSqlDialect(); + Assume.assumeFalse("Datediff not supported for " + dialect.getClass().getSimpleName(), dialect instanceof SimpleSqlDialect); - testDateDiff(scope, dialect, "2/1/2000", "1/1/2000", Calendar.DATE, 31); - testDateDiff(scope, dialect, "1/1/2001", "1/1/2000", Calendar.DATE, 366); + _testDateDiff(scope, dialect, "2/1/2000", "1/1/2000", Calendar.DATE, 31); + _testDateDiff(scope, dialect, "1/1/2001", "1/1/2000", Calendar.DATE, 366); - testDateDiff(scope, dialect, "2/1/2000", "1/1/2000", Calendar.MONTH, 1); - testDateDiff(scope, dialect, "2/1/2000", "1/31/2000", Calendar.MONTH, 1); - testDateDiff(scope, dialect, "1/1/2000", "1/1/2000", Calendar.MONTH, 0); - testDateDiff(scope, dialect, "1/31/2000", "1/1/2000", Calendar.MONTH, 0); - testDateDiff(scope, dialect, "12/31/2000", "1/1/2000", Calendar.MONTH, 11); - testDateDiff(scope, dialect, "1/1/2001", "1/1/2000", Calendar.MONTH, 12); - testDateDiff(scope, dialect, "1/31/2001", "1/1/2000", Calendar.MONTH, 12); + _testDateDiff(scope, dialect, "2/1/2000", "1/1/2000", Calendar.MONTH, 1); + _testDateDiff(scope, dialect, "2/1/2000", "1/31/2000", Calendar.MONTH, 1); + _testDateDiff(scope, dialect, "1/1/2000", "1/1/2000", Calendar.MONTH, 0); + _testDateDiff(scope, dialect, "1/31/2000", "1/1/2000", Calendar.MONTH, 0); + _testDateDiff(scope, dialect, "12/31/2000", "1/1/2000", Calendar.MONTH, 11); + _testDateDiff(scope, dialect, "1/1/2001", "1/1/2000", Calendar.MONTH, 12); + _testDateDiff(scope, dialect, "1/31/2001", "1/1/2000", Calendar.MONTH, 12); - testDateDiff(scope, dialect, "1/1/2000", "12/31/2000", Calendar.YEAR, 0); - testDateDiff(scope, dialect, "1/1/2001", "1/1/2000", Calendar.YEAR, 1); + _testDateDiff(scope, dialect, "1/1/2000", "12/31/2000", Calendar.YEAR, 0); + _testDateDiff(scope, dialect, "1/1/2001", "1/1/2000", Calendar.YEAR, 1); } - private void testDateDiff(DbScope scope, SqlDialect dialect, String date1, String date2, int part, int expected) + private void _testDateDiff(DbScope scope, SqlDialect dialect, String date1, String date2, int part, int expected) { SQLFragment sql = new SQLFragment("SELECT ("); sql.append(dialect.getDateDiff(part, "CAST('" + date1 + "' AS " + dialect.getDefaultDateTimeDataType() + ")", "CAST('" + date2 + "' AS " + dialect.getDefaultDateTimeDataType() + ")")); @@ -3013,24 +3030,33 @@ private void testDateDiff(DbScope scope, SqlDialect dialect, String date1, Strin } @TestWhen(TestWhen.When.DBSCOPE) + @RunWith(Parameterized.class) public static class GroupConcatTestCase extends Assert { + @Parameterized.Parameters(name = "{1}") + public static Collection schemas() + { + return JunitUtil.getDbScopesTestParameters(scope -> scope.getSqlDialect().supportsGroupConcat()); + } + + private final DbScope scope; + + public GroupConcatTestCase(DbScope scope, String displayName) + { + this.scope = scope; + } + @Test public void testGroupConcat() { - for (DbScope scope : getDbScopesToTest()) - { - SqlDialect dialect = scope.getSqlDialect(); - if (!dialect.supportsGroupConcat()) - continue; + SqlDialect dialect = scope.getSqlDialect(); - boolean caseInsensitiveCollation = dialect.isSqlServer(); + boolean caseInsensitiveCollation = dialect.isSqlServer(); - testGroupConcat(scope, dialect, false, false, "x Y z z y"); - testGroupConcat(scope, dialect, true, false, caseInsensitiveCollation ? "x Y z" : "x y Y z"); - testGroupConcat(scope, dialect, false, true, "x y Y z z"); - testGroupConcat(scope, dialect, true, true, caseInsensitiveCollation ? "x Y z" : "x y Y z"); - } + testGroupConcat(scope, dialect, false, false, "x Y z z y"); + testGroupConcat(scope, dialect, true, false, caseInsensitiveCollation ? "x Y z" : "x y Y z"); + testGroupConcat(scope, dialect, false, true, "x y Y z z"); + testGroupConcat(scope, dialect, true, true, caseInsensitiveCollation ? "x Y z" : "x y Y z"); } private void testGroupConcat(DbScope scope, SqlDialect dialect, boolean distinct, boolean sorted, String expected) diff --git a/api/src/org/labkey/api/data/dialect/SqlDialect.java b/api/src/org/labkey/api/data/dialect/SqlDialect.java index 805624000bf..582e5bc0e90 100644 --- a/api/src/org/labkey/api/data/dialect/SqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/SqlDialect.java @@ -23,6 +23,8 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.collections.CsvSet; @@ -61,6 +63,7 @@ import org.labkey.api.test.TestWhen; import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.HtmlString; +import org.labkey.api.util.JunitUtil; import org.labkey.api.util.MemTracker; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.SystemMaintenance; @@ -2416,21 +2419,22 @@ public SQLFragment array_element_like(SQLFragment a, String... values) // @TestWhen(TestWhen.When.DBSCOPE) + @RunWith(Parameterized.class) public static class DialectTestCase { - DbScope s; - SqlDialect d; + @Parameterized.Parameters(name = "{1}") + public static Collection schemas() + { + return JunitUtil.getDbScopesTestParameters(); + } - @Test - public void testScopes() + private final DbScope s; + private final SqlDialect d; + + public DialectTestCase(DbScope scope, String displayName) { - DbScope.getDbScopesToTest().forEach(scope -> - { - this.s = scope; - this.d = scope.getSqlDialect(); - testDialectStringHandler(); - testLikeOperator(); - }); + this.s = scope; + this.d = scope.getSqlDialect(); } void testEquals(String expected, SQLFragment sqlf) @@ -2439,13 +2443,14 @@ void testEquals(String expected, SQLFragment sqlf) { assertEquals(expected, new SqlSelector(s, sqlf).getObject(String.class)); } - catch (AssertionError|Exception ae) + catch (AssertionError | Exception ae) { throw new AssertionError("Expected [" + expected + "] Failed for dialect " + d.getClass().getName() + " on scope " + s.getDatabaseUrl() + ": " + sqlf.toDebugString(), ae); } } - void testDialectStringHandler() + @Test + public void testDialectStringHandler() { // quotes backslashes etc for (String v : Arrays.asList("", "'", "\"", "\\", "''", "\\'", "\\\\'", "'''", "><&/%\\' \"1~\\!@$&'()\"_+{}-=[],.#\u2603\u00E4\u00F6\u00FC\u00C5")) @@ -2457,7 +2462,8 @@ void testDialectStringHandler() testEquals(v, new SQLFragment("SELECT ").appendStringLiteral(v, d)); } - void testLikeOperator() + @Test + public void testLikeOperator() { String stringLiteralPrefix = d.isSqlServer() ? " N" : " "; assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'ABC%' ESCAPE '!'", d.appendCaseInsensitiveStartsWith(new SQLFragment("SELECT * FROM A WHERE Name"), "ABC").toDebugString(d)); @@ -2468,7 +2474,10 @@ void testLikeOperator() assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'_a!_![b]C%' ESCAPE '!'", d.appendCaseInsensitiveLikeClause(new SQLFragment("SELECT * FROM A WHERE Name"), "a_[b]C", "_", "%").toDebugString(d)); assertEquals("SELECT * FROM A WHERE Name " + d.getCaseInsensitiveLikeOperator() + stringLiteralPrefix + "'_a[_[[b]C!d%' ESCAPE '['", d.appendCaseInsensitiveLikeClause(new SQLFragment("SELECT * FROM A WHERE Name"), "a_[b]C!d", "_", "%", '[').toDebugString(d)); } + } + public static class LabKeyScopeDialectTestCase + { @Test public void testAutoIncrementQuery() { diff --git a/api/src/org/labkey/api/util/JunitUtil.java b/api/src/org/labkey/api/util/JunitUtil.java index 02830b17b4b..af7e4dfbe98 100644 --- a/api/src/org/labkey/api/util/JunitUtil.java +++ b/api/src/org/labkey/api/util/JunitUtil.java @@ -24,6 +24,7 @@ import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.DbScope; import org.labkey.api.module.Module; import org.labkey.api.settings.AppProps; import org.w3c.dom.Node; @@ -32,9 +33,11 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.LinkedList; +import java.util.List; import java.util.Map; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.CyclicBarrier; @@ -45,6 +48,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; import java.util.stream.Stream; @@ -272,4 +276,44 @@ public static void createRaces(final Runnable runnable, final int threads, final throw new AssumptionViolatedException(message + " Skipping test in production mode."); } } + + /** + * Gets `DbScope.getDbScopesToTest()`, structured for use with `@RunWith(Parameterized.class)`.
+ * Tests using these parameters should also be annotated with `@TestWhen(TestWhen.When.DBSCOPE)` to ensure they run + * in suites that configure external data sources on TeamCity. + * + * 
{@code
+     *     @TestWhen(TestWhen.When.DBSCOPE)
+     *     @RunWith(Parameterized.class)
+     *     public static class DbScopeTestCase
+     *     {
+     *         @Parameterized.Parameters(name = "{1}")
+     *         public static Collection schemas()
+     *         {
+     *             return JunitUtil.getDbScopesTestParameters();
+     *         }
+     *
+     *         private final DbScope scope;
+     *
+     *         public DbScopeTestCase(DbScope scope, String displayName)
+     *         {
+     *             this.scope = scope;
+     *         }
+     *
+     *         // @Test cases will me multiplied across all found db scopes
+     *     }
+     * }
+ */ + public static Collection getDbScopesTestParameters(Function filter) + { + return DbScope.getDbScopesToTest().stream() + .filter(filter::apply) + .map(scope -> new Object[]{scope, scope.getSqlDialect().getClass().getSimpleName()}) + .toList(); + } + + public static Collection getDbScopesTestParameters() + { + return getDbScopesTestParameters(_ -> true); + } } diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index dbba0e8e6dd..b127b9cc150 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -1478,6 +1478,7 @@ public TabDisplayMode getTabDisplayMode() SecurityApiActions.TestCase.class, SecurityController.TestCase.class, SqlDialect.DialectTestCase.class, + SqlDialect.LabKeyScopeDialectTestCase.class, SqlScriptController.TestCase.class, TableViewFormTestCase.class, UnknownSchemasTest.class, diff --git a/query/src/org/labkey/query/QueryModule.java b/query/src/org/labkey/query/QueryModule.java index 4419ac69ef5..e1f9d175a7e 100644 --- a/query/src/org/labkey/query/QueryModule.java +++ b/query/src/org/labkey/query/QueryModule.java @@ -417,6 +417,7 @@ public Set getSchemaNames() JdbcType.TestCase.class, MemberSet.TestCase.class, MetadataElementBase.TestCase.class, + Method.IsDistinctFromMethodTestCase.class, Method.TestCase.class, ExpressionAssistantAgentAction.TestCase.class, QNode.TestCase.class, diff --git a/query/src/org/labkey/query/sql/Method.java b/query/src/org/labkey/query/sql/Method.java index 9e06a7bbac5..a5d707dbf1b 100644 --- a/query/src/org/labkey/query/sql/Method.java +++ b/query/src/org/labkey/query/sql/Method.java @@ -21,6 +21,8 @@ import org.jetbrains.annotations.Nullable; import org.junit.Assert; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.BaseColumnInfo; import org.labkey.api.data.ColumnInfo; @@ -51,6 +53,7 @@ import org.labkey.api.settings.AppProps; import org.labkey.api.test.TestWhen; import org.labkey.api.util.GUID; +import org.labkey.api.util.JunitUtil; import org.labkey.query.QueryServiceImpl; import org.labkey.query.sql.antlr.SqlBaseLexer; @@ -58,6 +61,7 @@ import java.lang.reflect.Modifier; import java.text.DecimalFormat; import java.util.Calendar; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -2000,7 +2004,6 @@ public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments) } } - @TestWhen(TestWhen.When.DBSCOPE) public static class TestCase extends Assert { void assertIsSimpleString(String expected, SQLFragment s) @@ -2039,6 +2042,24 @@ public void testSimpleString() assertNotSimpleString(new SQLFragment("SELECT 'test'")); assertNotSimpleString(new SQLFragment("'test''string'")); } + } + + @TestWhen(TestWhen.When.DBSCOPE) + @RunWith(Parameterized.class) + public static class IsDistinctFromMethodTestCase extends Assert + { + @Parameterized.Parameters(name = "{1}") + public static Collection schemas() + { + return JunitUtil.getDbScopesTestParameters(); + } + + private final DbScope scope; + + public IsDistinctFromMethodTestCase(DbScope scope, String displayName) + { + this.scope = scope; + } // Exercises both the native and portable-fallback branches of IsDistinctFromMethodInfo.getSQL() against every // dialect that's actually connected in this environment, not just whichever dialect the current CI leg happens @@ -2058,17 +2079,14 @@ record Case(String a, String b, boolean distinct) {} new Case("1", "NULL", true) ); - for (DbScope scope : DbScope.getDbScopesToTest()) - { - SqlDialect d = scope.getSqlDialect(); + SqlDialect d = scope.getSqlDialect(); - for (Case c : cases) - { - assertIsDistinctFrom(scope, d, IS, c.a(), c.b(), c.distinct()); - assertIsDistinctFrom(scope, d, IS_NOT, c.a(), c.b(), !c.distinct()); - assertIsDistinctFromWhere(scope, d, IS, c.a(), c.b(), c.distinct()); - assertIsDistinctFromWhere(scope, d, IS_NOT, c.a(), c.b(), !c.distinct()); - } + for (Case c : cases) + { + assertIsDistinctFrom(scope, d, IS, c.a(), c.b(), c.distinct()); + assertIsDistinctFrom(scope, d, IS_NOT, c.a(), c.b(), !c.distinct()); + assertIsDistinctFromWhere(scope, d, IS, c.a(), c.b(), c.distinct()); + assertIsDistinctFromWhere(scope, d, IS_NOT, c.a(), c.b(), !c.distinct()); } }