You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "wuchong (via GitHub)" <gi...@apache.org> on 2026/08/09 20:17:33 UTC
[PR] [test] Add multi-version compatibility test selection [fluss]
wuchong opened a new pull request, #3913:
URL: https://github.com/apache/fluss/pull/3913
## Summary
- introduce a shared `@MultiVersionTest` marker for JUnit 5 and ScalaTest
- run the complete Flink test suite on 1.20 while selecting only marked tests on 1.18, 1.19, and 2.2
- auto-discover the JUnit execution condition so Maven and IntelliJ IDEA use the same filtering semantics
- skip an ITCase class lifecycle when it contains no selected multi-version tests
- restore Flink 1.19 to the CI stage now that its compatibility suite is filtered
Closes #3912
## Test Plan
- `./mvnw -pl fluss-test-utils -Dfluss.reuseForks=true clean test`
- `./mvnw -pl fluss-test-utils,fluss-flink/fluss-flink-common,fluss-flink/fluss-flink-1.18,fluss-flink/fluss-flink-1.19,fluss-flink/fluss-flink-1.20,fluss-flink/fluss-flink-2.2 -DskipTests -Dfluss.reuseForks=true test-compile`
- `git diff --check`
🤖 AI-assisted changes - reviewed by human developer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [test] Add multi-version compatibility test selection [fluss]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3913:
URL: https://github.com/apache/fluss/pull/3913#discussion_r3746416065
##########
fluss-test-utils/src/test/java/org/apache/fluss/testutils/common/MultiVersionTestConditionTest.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.fluss.testutils.common;
+
+import org.junit.jupiter.api.Test;
+
+/** Tests for {@link MultiVersionTestCondition}. */
+@RunMultiVersionTestsOnly
+class MultiVersionTestConditionTest extends MultiVersionTestConditionTestBase {
+
+ @Test
+ @MultiVersionTest
+ void testMarkedDeclaredMethodRuns() {}
+
+ @Test
+ void testUnmarkedDeclaredMethodDoesNotRun() {
+ throw new AssertionError("Unmarked declared test should have been disabled");
+ }
+}
+
+abstract class MultiVersionTestConditionTestBase {
+
+ @Test
+ @MultiVersionTest
+ void testMarkedInheritedMethodRuns() {}
+
+ @Test
+ void testUnmarkedInheritedMethodDoesNotRun() {
+ throw new AssertionError("Unmarked inherited test should have been disabled");
+ }
+}
+
+@RunMultiVersionTestsOnly
+class OnlyUnmarkedInheritedMultiVersionTestConditionTest
+ extends OnlyUnmarkedInheritedMultiVersionTestConditionTestBase {}
Review Comment:
This sentinel only fails if the unmarked test method executes. If the class-level precheck regresses and starts the class lifecycle while the method-level condition still disables this method, the test still passes, so the stated lifecycle-suppression behavior is not covered. Add a failing `@BeforeAll` (or equivalent lifecycle callback) to this class.
##########
fluss-test-utils/src/main/java/org/apache/fluss/testutils/common/MultiVersionTestCondition.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.fluss.testutils.common;
+
+import org.junit.jupiter.api.extension.ConditionEvaluationResult;
+import org.junit.jupiter.api.extension.ExecutionCondition;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.platform.commons.annotation.Testable;
+import org.junit.platform.commons.support.AnnotationSupport;
+
+import java.lang.reflect.Method;
+
+/**
+ * Filters JUnit tests not marked with {@link MultiVersionTest} in a non-default version module.
+ *
+ * <p>The filter can be enabled for a module with {@value #ENABLE_PROPERTY}, or for an individual
+ * test class with {@link RunMultiVersionTestsOnly}.
+ */
+public final class MultiVersionTestCondition implements ExecutionCondition {
+
+ /** JUnit configuration parameter that enables multi-version filtering for a test module. */
+ public static final String ENABLE_PROPERTY = "fluss.tests.multiversion.enabled";
+
+ private static final ConditionEvaluationResult ENABLED =
+ ConditionEvaluationResult.enabled("Multi-version filtering is not enabled");
+
+ private static final ConditionEvaluationResult ENABLED_CLASS =
+ ConditionEvaluationResult.enabled("Class contains selected multi-version tests");
+
+ private static final ConditionEvaluationResult ENABLED_MULTI_VERSION =
+ ConditionEvaluationResult.enabled("Test is marked as multi-version");
+
+ @Override
+ public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext context) {
+ if (!context.getTestClass().isPresent()) {
+ return ENABLED;
+ }
+
+ Class<?> testClass = context.getRequiredTestClass();
+ if (!isFilteringEnabled(context, testClass)) {
+ return ENABLED;
+ }
+
+ if (!context.getTestMethod().isPresent()) {
+ return hasSelectedTests(testClass)
+ ? ENABLED_CLASS
+ : ConditionEvaluationResult.disabled("Class has no multi-version tests");
+ }
+
+ Method testMethod = context.getRequiredTestMethod();
+ if (context.getTags().contains(MultiVersionTest.TAG)) {
+ return ENABLED_MULTI_VERSION;
+ }
+
+ return ConditionEvaluationResult.disabled(
+ String.format(
+ "Test '%s' is not marked with @MultiVersionTest", testMethod.getName()));
+ }
+
+ private static boolean isFilteringEnabled(ExtensionContext context, Class<?> testClass) {
+ boolean enabledByConfiguration =
+ context.getConfigurationParameter(ENABLE_PROPERTY)
+ .map(Boolean::parseBoolean)
+ .orElse(false);
+ return enabledByConfiguration
+ || AnnotationSupport.isAnnotated(testClass, RunMultiVersionTestsOnly.class);
+ }
+
+ private static boolean hasSelectedTests(Class<?> testClass) {
+ if (AnnotationSupport.isAnnotated(testClass, MultiVersionTest.class)) {
+ return true;
+ }
+
+ Class<?> currentClass = testClass;
+ while (currentClass != null && currentClass != Object.class) {
+ for (Method method : currentClass.getDeclaredMethods()) {
+ if (AnnotationSupport.isAnnotated(method, Testable.class)
+ && AnnotationSupport.isAnnotated(method, MultiVersionTest.class)) {
+ return true;
+ }
+ }
+ currentClass = currentClass.getSuperclass();
+ }
+ return false;
Review Comment:
The class-level precheck does not inspect nested test containers. JUnit evaluates the condition on the outer class first, so an outer class containing only a nested `@MultiVersionTest` class or method is disabled before that selected test can be discovered (for example, the connector already has nested-only test classes). Recurse into declared nested classes before concluding that the class has no selected tests, and add a nested regression case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [test] Add multi-version compatibility test selection [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on PR #3913:
URL: https://github.com/apache/fluss/pull/3913#issuecomment-5248225050
CI Job | Before | After | Time Reduced | Reduction
-- | -- | -- | -- | --
Java 11 / flink1 | 53m (without flink 1.19) | 41m (with flink 1.19) | 12m | 22.6%
Java 11 / flink2 | 36m | 17m | 19m | 52.8%
Total | 89m | 58m | 31m | 34.8%
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [test] Add multi-version compatibility test selection [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong merged PR #3913:
URL: https://github.com/apache/fluss/pull/3913
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org