nevzheng commented on code in PR #13517:
URL: https://github.com/apache/gravitino/pull/13517#discussion_r4274609051
##########
core/build.gradle.kts:
##########
@@ -105,11 +105,109 @@ artifacts {
add("testArtifacts", testJar)
}
-tasks.test {
- val policiesDocument = rootProject.file("docs/policies.md")
- inputs.file(policiesDocument)
- systemProperty("gravitino.repositoryRoot", rootProject.projectDir.path)
+// Core's tests run in four lanes: coreUnitTest, plus
coreH2Test/coreMySQLTest/corePostgreSQLTest.
+// A class joins a backend lane through its @CoreBackend.* annotation; see
CoreBackend.java.
+// Backend name -> the JUnit tag that admits a class to that lane. Keep in
sync with CoreBackend.
+val coreBackendTestTags =
+ linkedMapOf(
+ "h2" to "gravitino-core-h2-test",
+ "mysql" to "gravitino-core-mysql-test",
+ "postgresql" to "gravitino-core-postgresql-test"
+ )
+val coreTestBackendProperty = "gravitino.core.test.backend"
+
+fun registerCoreTestTask(
+ taskName: String,
+ backend: String? = null
+) = tasks.register<Test>(taskName) {
+ group = "verification"
+ description =
+ if (backend == null) {
+ "Runs core unit tests."
+ } else {
+ "Runs core database tests against $backend."
+ }
+
+ testClassesDirs = sourceSets["test"].output.classesDirs
+ classpath = sourceSets["test"].runtimeClasspath
+
+ reports.html.outputLocation.set(
+ rootProject.layout.buildDirectory.dir("reports/tests/core/$taskName")
+ )
+
+ useJUnitPlatform {
+ if (backend == null) {
+ // Whatever carries no backend tag (and no Docker tag) is the unit suite.
+ excludeTags(*coreBackendTestTags.values.toTypedArray(),
"gravitino-docker-test")
+ } else {
+ val ownBackendTag =
+ coreBackendTestTags[backend]
+ ?: throw GradleException("Unsupported core test backend: $backend")
+ // Plain tag include, applied by JUnit at discovery time, so classes not
tagged for this
+ // backend never show up in this lane's JUnit XML. A class tagged for
several backends
+ // runs under each of them.
+ includeTags(ownBackendTag)
+ }
+ }
+ if (backend != null) {
+ systemProperty(coreTestBackendProperty, backend)
+ // Read by the root setTestEnvironment inside doFirst, i.e. at execution
time, after every
+ // configuration action has run - so it doesn't depend on
register/configureEach ordering.
+ extensions.extraProperties["includeDockerTaggedTests"] = true
Review Comment:
**note (non-blocking):** Agreed, the safety net got thinner when `reconcile`
went. I'll make `:core:test` and its CI job independent of that flag in #13837,
which reshapes these tasks anyway, so a lost flag can't silently drop the
Docker-tagged H2 classes. Keeping this PR as approved rather than pushing again.
##########
core/src/test/java/org/apache/gravitino/storage/relational/TestCoreLaneMembership.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * 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.gravitino.storage.relational;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.lang.reflect.Modifier;
+import java.net.URL;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.platform.commons.annotation.Testable;
+import org.junit.platform.commons.support.AnnotationSupport;
+import org.junit.platform.commons.support.HierarchyTraversalMode;
+import org.junit.platform.commons.support.ReflectionSupport;
+
+/**
+ * Fails when a core test class runs in no lane: it carries {@code
gravitino-docker-test}, which
+ * keeps it out of {@code coreUnitTest}, but no {@link CoreBackend} tag, which
keeps it out of every
+ * database lane. Nothing else notices a class that leaves every lane at once.
+ */
+class TestCoreLaneMembership {
+
+ private static final String DOCKER_TAG = "gravitino-docker-test";
+
+ private static final Set<String> BACKEND_TAGS =
+ Set.of(CoreBackend.H2_TAG, CoreBackend.MYSQL_TAG,
CoreBackend.POSTGRESQL_TAG);
+
+ @Test
+ void everyDockerTaggedTestClassHasABackendLane() {
+ URL coreTestClasses = codeLocation(TestCoreLaneMembership.class);
+ List<String> orphans =
+ ReflectionSupport.findAllClassesInPackage(
+ "org.apache.gravitino",
+ c -> coreTestClasses.equals(codeLocation(c)) &&
declaresTests(c),
+ name -> true)
+ .stream()
+ .filter(
+ c -> {
+ Set<String> tags = effectiveTags(c);
+ return tags.contains(DOCKER_TAG)
+ && tags.stream().noneMatch(BACKEND_TAGS::contains);
+ })
+ .map(Class::getName)
+ .sorted()
+ .collect(Collectors.toList());
+
+ assertTrue(
+ orphans.isEmpty(),
+ "These classes carry gravitino-docker-test but no @CoreBackend.* tag,
so they run in no "
+ + "core test lane. Add @CoreBackend.H2/MySQL/PostgreSQL or
@CoreBackend.All: "
+ + orphans);
+ }
+
+ /**
+ * Whether JUnit would run {@code c}: concrete, with at least one {@code
@Test}-style method (all
+ * meta-annotated with {@link Testable}), own or inherited.
+ */
+ private static boolean declaresTests(Class<?> c) {
+ return !Modifier.isAbstract(c.getModifiers())
+ && !ReflectionSupport.findMethods(
+ c,
+ m -> AnnotationSupport.isAnnotated(m, Testable.class),
+ HierarchyTraversalMode.TOP_DOWN)
+ .isEmpty();
+ }
+
+ /**
+ * Own and inherited tags, plus the enclosing classes' tags for non-static
{@code @Nested}
+ * classes, which JUnit applies to them. Static member classes are
discovered as top-level classes
+ * and don't inherit their enclosing class's tags.
+ */
+ private static Set<String> effectiveTags(Class<?> c) {
+ Set<String> tags = new HashSet<>();
+ for (Class<?> k = c;
+ k != null;
+ k = Modifier.isStatic(k.getModifiers()) ? null :
k.getEnclosingClass()) {
+ AnnotationSupport.findRepeatableAnnotations(k, Tag.class).forEach(t ->
tags.add(t.value()));
+ }
+ return tags;
+ }
Review Comment:
**note (non-blocking):** Keeping as is for now. As you found, all seven
`gravitino-docker-test` uses in `core/src/test` are class-level, so nothing
escapes today. If a method-level one shows up, extending `effectiveTags` to the
test methods `declaresTests` already walks is the fix.
##########
build.gradle.kts:
##########
@@ -1019,7 +1024,17 @@ subprojects {
val extraArgs = project.property("extraJvmArgs") as List<String>
jvmArgs = listOf("-Xmx4G") + extraArgs
useJUnitPlatform()
- finalizedBy(tasks.getByName("jacocoTestReport"))
+ val isCoreSuiteTask =
+ project.path == ":core" &&
+ name in setOf(
+ "coreUnitTest",
+ "coreH2Test",
+ "coreMySQLTest",
+ "corePostgreSQLTest"
+ )
+ if (!isCoreSuiteTask) {
Review Comment:
**note (non-blocking):** Good catch. #13837 cuts the lanes to `test`,
`coreMySQLTest` and `corePostgreSQLTest`. I'll move the finalizer handling into
`core/build.gradle.kts` there, with `setFinalizedBy(emptyList())` on the
database tasks, so the root list goes away.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
