diff --git a/.github/workflows/backend-integration-test-action.yml b/.github/workflows/backend-integration-test-action.yml index 3d530026cfd..33ecb0c447e 100644 --- a/.github/workflows/backend-integration-test-action.yml +++ b/.github/workflows/backend-integration-test-action.yml @@ -20,12 +20,16 @@ on: required: true description: 'run on embedded or deploy mode' type: string + shard: + required: true + description: 'Test shard defined in dev/ci/test-shards.sh' + type: string jobs: start-runner: - name: JDK${{ inputs.java-version }}-${{ inputs.test-mode }}-${{ inputs.backend }} + name: JDK${{ inputs.java-version }}-${{ inputs.test-mode }}-${{ inputs.backend }}-${{ inputs.shard }} runs-on: ubuntu-22.04 - timeout-minutes: 120 + timeout-minutes: 90 env: PLATFORM: ${{ inputs.architecture }} steps: @@ -58,7 +62,7 @@ jobs: wget https://nz2.archive.ubuntu.com/ubuntu/pool/main/o/openssl/libssl1.1_1.1.1f-1ubuntu2_amd64.deb sudo dpkg -i libssl1.1_1.1.1f-1ubuntu2_amd64.deb - - name: Backend Integration Test (JDK${{ inputs.java-version }}-${{ inputs.test-mode }}-${{ inputs.backend }}) + - name: Backend Integration Test (JDK${{ inputs.java-version }}-${{ inputs.test-mode }}-${{ inputs.backend }}-${{ inputs.shard }}) id: integrationTest run: | EXCLUDE_CONTRIB_TESTS="" @@ -66,7 +70,10 @@ jobs: EXCLUDE_CONTRIB_TESTS="$EXCLUDE_CONTRIB_TESTS -x :catalogs-contrib:$module:test" done - ./gradlew test -PskipTests -PtestMode=${{ inputs.test-mode }} -PjdbcBackend=${{ inputs.backend }} -PskipDockerTests=false -PskipWeb=true \ + shard_args_text="$(dev/ci/test-shards.sh backend-it "${{ inputs.shard }}")" + mapfile -t shard_args <<< "${shard_args_text}" + + ./gradlew "${shard_args[@]}" -PskipTests -PtestMode=${{ inputs.test-mode }} -PjdbcBackend=${{ inputs.backend }} -PskipDockerTests=false -PskipWeb=true \ -x :web:web:test -x :web:integration-test:test -x :web-v2:web:test -x :web-v2:integration-test:test -x :clients:client-python:test \ -x :flink-connector:flink-common:test \ -x :flink-connector:flink-1.18:test -x :flink-connector:flink-runtime-1.18:test \ @@ -85,7 +92,7 @@ jobs: uses: actions/upload-artifact@v7 if: ${{ (failure() && steps.integrationTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }} with: - name: integrate-test-reports-${{ inputs.java-version }}-${{ inputs.test-mode }}-${{ inputs.backend }} + name: integrate-test-reports-${{ inputs.java-version }}-${{ inputs.test-mode }}-${{ inputs.backend }}-${{ inputs.shard }} path: | build/reports iceberg/iceberg-rest-server/build/*.log diff --git a/.github/workflows/backend-integration-test.yml b/.github/workflows/backend-integration-test.yml index 5e4f45aa831..a93914fdb61 100644 --- a/.github/workflows/backend-integration-test.yml +++ b/.github/workflows/backend-integration-test.yml @@ -50,41 +50,25 @@ jobs: - gradle.properties - gradlew - settings.gradle.kts + - name: List backend integration test shards + id: shards + run: echo "backend_it_shards=$(dev/ci/test-shards.sh backend-it --list)" >> "${GITHUB_OUTPUT}" outputs: source_changes: ${{ steps.filter.outputs.source_changes }} + backend_it_shards: ${{ steps.shards.outputs.backend_it_shards }} - BackendIT-on-push: + BackendIT: needs: changes - if: (github.event_name == 'push' && needs.changes.outputs.source_changes == 'true') - strategy: - matrix: - architecture: [linux/amd64] - java-version: [ 17 ] - backend: [ h2, mysql, postgresql ] - test-mode: [ embedded, deploy ] - exclude: - - test-mode: 'embedded' - backend: 'mysql' - - test-mode: 'embedded' - backend: 'postgresql' - - test-mode: 'deploy' - backend: 'h2' - uses: ./.github/workflows/backend-integration-test-action.yml - with: - architecture: ${{ matrix.architecture }} - java-version: ${{ matrix.java-version }} - backend: ${{ matrix.backend }} - test-mode: ${{ matrix.test-mode }} - - BackendIT-on-pr: - needs: changes - if: (github.event_name == 'pull_request' && needs.changes.outputs.source_changes == 'true') + if: needs.changes.outputs.source_changes == 'true' strategy: + fail-fast: false matrix: architecture: [ linux/amd64 ] java-version: [ 17 ] backend: [ h2, mysql, postgresql ] test-mode: [ embedded, deploy ] + # Shards are defined in dev/ci/test-shards.sh. + shard: ${{ fromJSON(needs.changes.outputs.backend_it_shards) }} exclude: - test-mode: 'embedded' backend: 'mysql' @@ -98,3 +82,4 @@ jobs: java-version: ${{ matrix.java-version }} backend: ${{ matrix.backend }} test-mode: ${{ matrix.test-mode }} + shard: ${{ matrix.shard }} diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cc4fe47ad8a..31a769daaea 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -93,7 +93,11 @@ jobs: fi echo "maintenance_module_only_changes=${maintenance_module_only}" >> "${GITHUB_OUTPUT}" + - name: List build shards + id: shards + run: echo "build_shards=$(dev/ci/test-shards.sh build --list)" >> "${GITHUB_OUTPUT}" outputs: + build_shards: ${{ steps.shards.outputs.build_shards }} source_changes: ${{ steps.filter.outputs.source_changes }} spark_connector_changes: ${{ steps.filter.outputs.spark_connector_changes }} mcp_server_changes: ${{ steps.filter.outputs.mcp_server_changes }} @@ -147,17 +151,18 @@ jobs: spark-connector/**/*.log build: - # The type of runner that the job will run on + name: build (${{ matrix.java-version }}, ${{ matrix.shard }}) runs-on: ubuntu-latest strategy: + fail-fast: false matrix: java-version: [ 17 ] + # Shards are defined in dev/ci/test-shards.sh. + shard: ${{ fromJSON(needs.changes.outputs.build_shards) }} timeout-minutes: 120 needs: changes if: needs.changes.outputs.source_changes == 'true' - # Steps represent a sequence of tasks that will be executed as part of the job steps: - # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it - uses: actions/checkout@v4 - uses: ./.github/actions/setup-java-toolchains @@ -165,6 +170,7 @@ jobs: java-version: ${{ matrix.java-version }} - name: Test publish to local + if: matrix.shard == 'others' run: ./gradlew publishToMavenLocal -PskipWeb=true -x test - name: Free up disk space @@ -179,6 +185,10 @@ jobs: - name: Build with Gradle run: | if [ "${{ needs.changes.outputs.maintenance_module_only_changes }}" = "true" ]; then + if [ "${{ matrix.shard }}" != "others" ]; then + echo "Only maintenance modules changed; they are built by the 'others' shard." + exit 0 + fi ./gradlew \ :maintenance:optimizer-api:build \ :maintenance:updaters:build \ @@ -190,12 +200,20 @@ jobs: exit 0 fi + shard_args_text="$(dev/ci/test-shards.sh build "${{ matrix.shard }}")" + mapfile -t shard_args <<< "${shard_args_text}" + + skip_docker_tests=false + case "${{ matrix.shard }}" in + core-unit|core-h2) skip_docker_tests=true ;; + esac + gradle_args=( - build + "${shard_args[@]}" --max-workers=2 -PskipWeb=true -PskipITs - -PskipDockerTests=false + "-PskipDockerTests=${skip_docker_tests}" -x :clients:client-python:build -x :catalogs-contrib:catalog-jdbc-clickhouse:test -x :catalogs-contrib:catalog-jdbc-hologres:test @@ -217,13 +235,238 @@ jobs: ./gradlew "${gradle_args[@]}" + - name: Resolve core test lane + id: core-lane + if: >- + success() && + needs.changes.outputs.maintenance_module_only_changes != 'true' && + startsWith(matrix.shard, 'core-') + env: + CORE_SHARD: ${{ matrix.shard }} + run: | + lane="${CORE_SHARD#core-}" + task_path="$(dev/ci/test-shards.sh build "${CORE_SHARD}")" + if [ -z "${lane}" ] || [ "${lane}" = "${CORE_SHARD}" ] || \ + [ -z "${task_path}" ] || [[ "${task_path}" == *$'\n'* ]] || \ + [[ "${task_path}" != :core:* ]]; then + echo "Invalid core shard mapping: ${CORE_SHARD} -> ${task_path}" >&2 + exit 1 + fi + task="${task_path#:core:}" + if [ -z "${task}" ] || [[ "${task}" == *:* ]]; then + echo "Invalid core task path: ${task_path}" >&2 + exit 1 + fi + echo "lane=${lane}" >> "${GITHUB_OUTPUT}" + echo "task=${task}" >> "${GITHUB_OUTPUT}" + + - name: Generate core test manifest + if: steps.core-lane.outcome == 'success' + env: + CORE_LANE: ${{ steps.core-lane.outputs.lane }} + CORE_TASK: ${{ steps.core-lane.outputs.task }} + run: | + mkdir -p core/build/test-manifests + python3 dev/ci/core_test_identity.py manifest \ + --lane "${CORE_LANE}" \ + --results "core/build/test-results/${CORE_TASK}" \ + --output "core/build/test-manifests/${CORE_LANE}.json" + + - name: Validate core test evidence + if: steps.core-lane.outcome == 'success' + env: + CORE_LANE: ${{ steps.core-lane.outputs.lane }} + CORE_TASK: ${{ steps.core-lane.outputs.task }} + run: | + test -s "core/build/test-manifests/${CORE_LANE}.json" + junit_xml="$(find "core/build/test-results/${CORE_TASK}" \ + -type f -name 'TEST-*.xml' -print -quit)" + test -n "${junit_xml}" + test -s "${junit_xml}" + test -s "build/reports/tests/core/${CORE_TASK}/index.html" + test -s "core/build/jacoco/${CORE_TASK}.exec" + + - name: Upload core test evidence + if: steps.core-lane.outcome == 'success' + uses: actions/upload-artifact@v7 + with: + name: core-${{ steps.core-lane.outputs.lane }}-test-evidence + path: | + core/build/test-manifests/${{ steps.core-lane.outputs.lane }}.json + core/build/test-results/${{ steps.core-lane.outputs.task }} + build/reports/tests/core/${{ steps.core-lane.outputs.task }} + core/build/jacoco/${{ steps.core-lane.outputs.task }}.exec + if-no-files-found: error + retention-days: 1 + + - name: Upload coverage data + if: >- + github.event_name == 'pull_request' && + !startsWith(matrix.shard, 'core-') + uses: actions/upload-artifact@v7 + with: + name: jacoco-${{ matrix.shard }} + # build.gradle.kts anchors the artifact at the repository root, so the report paths keep + # their module prefix, which jacoco_report.py uses to name modules. + path: | + build.gradle.kts + **/build/reports/jacoco/test/jacocoTestReport.xml + if-no-files-found: ignore + retention-days: 1 + + - name: Upload unit tests report + uses: actions/upload-artifact@v7 + if: failure() + with: + name: unit test report ${{ matrix.shard }} + path: | + build/reports + build/reports/tests/core + core/build/test-results + core/build/test-manifests + core/build/jacoco/*.exec + catalogs-contrib/**/*.log + catalogs-contrib/**/*.tar + catalogs/**/*.log + catalogs/**/*.tar + + core-test-contract: + runs-on: ubuntu-latest + timeout-minutes: 10 + needs: [ changes, build ] + if: >- + always() && + needs.changes.outputs.source_changes == 'true' && + needs.changes.outputs.maintenance_module_only_changes != 'true' && + needs.build.result == 'success' + steps: + - uses: actions/checkout@v4 + + - name: Test core identity tool + run: python3 -B -m unittest discover -s dev/ci/tests -p 'test_core_test_identity.py' + + - name: Download unit evidence + uses: actions/download-artifact@v4 + with: + name: core-unit-test-evidence + path: core-test-evidence/unit + + - name: Download H2 evidence + uses: actions/download-artifact@v4 + with: + name: core-h2-test-evidence + path: core-test-evidence/h2 + + - name: Download MySQL evidence + uses: actions/download-artifact@v4 + with: + name: core-mysql-test-evidence + path: core-test-evidence/mysql + + - name: Download PostgreSQL evidence + uses: actions/download-artifact@v4 + with: + name: core-postgresql-test-evidence + path: core-test-evidence/postgresql + + - name: Reconcile core test identities + run: | + mkdir -p core/build/test-manifests + python3 dev/ci/core_test_identity.py reconcile \ + --manifests \ + core-test-evidence/unit/core/build/test-manifests/unit.json \ + core-test-evidence/h2/core/build/test-manifests/h2.json \ + core-test-evidence/mysql/core/build/test-manifests/mysql.json \ + core-test-evidence/postgresql/core/build/test-manifests/postgresql.json \ + --output core/build/test-manifests/summary.json + + - name: Upload core test contract + uses: actions/upload-artifact@v7 + with: + name: core-test-contract + path: core/build/test-manifests/summary.json + if-no-files-found: error + retention-days: 1 + + coverage: + runs-on: ubuntu-latest + timeout-minutes: 30 + needs: [ changes, build, core-test-contract ] + if: >- + always() && + github.event_name == 'pull_request' && + needs.build.result == 'success' && + (needs.changes.outputs.maintenance_module_only_changes == 'true' || + needs['core-test-contract'].result == 'success') + steps: + - uses: actions/checkout@v4 + + - uses: ./.github/actions/setup-java-toolchains + if: needs.changes.outputs.maintenance_module_only_changes != 'true' + with: + java-version: 17 + - name: Fetch base branch for coverage diff - if: github.event_name == 'pull_request' run: git fetch origin ${{ github.base_ref }} --depth=1 + - name: Download coverage data + uses: actions/download-artifact@v4 + with: + pattern: jacoco-* + merge-multiple: true + + - name: Download unit evidence + if: needs.changes.outputs.maintenance_module_only_changes != 'true' + uses: actions/download-artifact@v4 + with: + name: core-unit-test-evidence + path: core-test-evidence/unit + + - name: Download H2 evidence + if: needs.changes.outputs.maintenance_module_only_changes != 'true' + uses: actions/download-artifact@v4 + with: + name: core-h2-test-evidence + path: core-test-evidence/h2 + + - name: Download MySQL evidence + if: needs.changes.outputs.maintenance_module_only_changes != 'true' + uses: actions/download-artifact@v4 + with: + name: core-mysql-test-evidence + path: core-test-evidence/mysql + + - name: Download PostgreSQL evidence + if: needs.changes.outputs.maintenance_module_only_changes != 'true' + uses: actions/download-artifact@v4 + with: + name: core-postgresql-test-evidence + path: core-test-evidence/postgresql + + - name: Generate combined core coverage + if: needs.changes.outputs.maintenance_module_only_changes != 'true' + run: | + mkdir -p core/build/jacoco + install -m 0644 \ + core-test-evidence/unit/core/build/jacoco/coreUnitTest.exec \ + core/build/jacoco/coreUnitTest.exec + install -m 0644 \ + core-test-evidence/h2/core/build/jacoco/coreH2Test.exec \ + core/build/jacoco/coreH2Test.exec + install -m 0644 \ + core-test-evidence/mysql/core/build/jacoco/coreMySQLTest.exec \ + core/build/jacoco/coreMySQLTest.exec + install -m 0644 \ + core-test-evidence/postgresql/core/build/jacoco/corePostgreSQLTest.exec \ + core/build/jacoco/corePostgreSQLTest.exec + ./gradlew \ + :core:jacocoTestReport \ + -PcoreSuiteCoverage=true \ + -PskipWeb=true + test -s core/build/reports/jacoco/test/jacocoTestReport.xml + - name: Generate Coverage Report id: coverage - if: github.event_name == 'pull_request' run: | python3 dev/ci/jacoco_report.py \ --base-ref "${{ github.base_ref }}" \ @@ -234,32 +477,21 @@ jobs: --output coverage-report.md - name: Save PR number - if: github.event_name == 'pull_request' && steps.coverage.outputs.has_reports == 'true' + if: steps.coverage.outputs.has_reports == 'true' run: echo "${{ github.event.pull_request.number }}" > pr-number.txt - name: Upload Coverage Report - if: github.event_name == 'pull_request' && steps.coverage.outputs.has_reports == 'true' + if: steps.coverage.outputs.has_reports == 'true' uses: actions/upload-artifact@v7 with: name: coverage-report path: | coverage-report.md pr-number.txt + core/build/reports/jacoco/test/jacocoTestReport.xml - name: Output Coverage Info - if: github.event_name == 'pull_request' && steps.coverage.outputs.has_reports == 'true' + if: steps.coverage.outputs.has_reports == 'true' run: | echo "Total coverage ${{ steps.coverage.outputs.coverage-overall }}" echo "Changed Files coverage ${{ steps.coverage.outputs.coverage-changed-files }}" - - - name: Upload unit tests report - uses: actions/upload-artifact@v7 - if: failure() - with: - name: unit test report - path: | - build/reports - catalogs-contrib/**/*.log - catalogs-contrib/**/*.tar - catalogs/**/*.log - catalogs/**/*.tar diff --git a/build.gradle.kts b/build.gradle.kts index f6bf7855ea1..7ba3b64cdd8 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -514,9 +514,12 @@ allprojects { val dockerTest = project.rootProject.extra["dockerTest"] as? Boolean ?: false param.environment("dockerTest", dockerTest.toString()) + val includeDockerTaggedTests = + param.extensions.extraProperties.properties["includeDockerTaggedTests"] as? Boolean + ?: dockerTest val dorisMultiVersion = project.hasProperty("dorisMultiVersionTest") param.useJUnitPlatform { - if (!dockerTest) { + if (!includeDockerTaggedTests) { excludeTags("gravitino-docker-test") } if (!dorisMultiVersion) { @@ -1019,7 +1022,17 @@ subprojects { val extraArgs = project.property("extraJvmArgs") as List jvmArgs = listOf("-Xmx4G") + extraArgs useJUnitPlatform() - finalizedBy(tasks.getByName("jacocoTestReport")) + val isCoreSuiteTask = + project.path == ":core" && + name in setOf( + "coreUnitTest", + "coreH2Test", + "coreMySQLTest", + "corePostgreSQLTest" + ) + if (!isCoreSuiteTask) { + finalizedBy(tasks.getByName("jacocoTestReport")) + } } } diff --git a/core/build.gradle.kts b/core/build.gradle.kts index a8511341f00..2ac888f25ae 100644 --- a/core/build.gradle.kts +++ b/core/build.gradle.kts @@ -1,4 +1,7 @@ import net.ltgt.gradle.errorprone.errorprone +import org.gradle.api.tasks.testing.Test +import org.gradle.testing.jacoco.plugins.JacocoTaskExtension +import org.gradle.testing.jacoco.tasks.JacocoReport /* * Licensed to the Apache Software Foundation (ASF) under one @@ -103,7 +106,164 @@ artifacts { add("testArtifacts", testJar) } +// Core's tests run in one of four Gradle lanes: coreUnitTest (default, no Docker) and +// coreH2Test/coreMySQLTest/corePostgreSQLTest (one per backend, coreMySQLTest and +// corePostgreSQLTest need Docker). Lane membership is decided purely by which of the three +// backend tags below a test class carries - see CoreBackend in +// core/src/test/java/org/apache/gravitino/storage/relational/CoreBackend.java for the typed +// annotations (@CoreBackend.H2/.MySQL/.PostgreSQL/.All) that set them, instead of writing raw +// @Tag("...") strings by hand: +// @CoreBackend.H2 -> runs only in coreH2Test +// @CoreBackend.H2 @CoreBackend.MySQL -> runs in coreH2Test and coreMySQLTest +// @CoreBackend.All -> runs in all three backend lanes +// (no CoreBackend annotation at all) -> a plain unit test, runs in coreUnitTest +// A class needing Docker but carrying no backend tag runs in no lane at all - check locally +// with `./gradlew :core:coreTestLaneOf -PclassName=`. +// +// Backend name -> JUnit tag that admits a test class to that backend's lane. Adding a backend +// here is enough to teach the lane filtering below about it; also add it to CoreBackend.java. +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(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 + + inputs.property("coreTestSuite", backend ?: "unit") + inputs.property("coreTestBackend", backend ?: "none") + // Distinct from the extensions.extraProperties["includeDockerTaggedTests"] flag set below, + // which is a different mechanism (read by root build.gradle.kts's shared test-environment + // setup to decide JUnit tag filtering) - this is only a Gradle up-to-date-check input. + inputs.property("coreTestIncludesDockerTaggedTests", backend != null) + reports.junitXml.outputLocation.set(layout.buildDirectory.dir("test-results/$taskName")) + reports.html.outputLocation.set( + rootProject.layout.buildDirectory.dir("reports/tests/core/$taskName") + ) + + extensions.configure { + destinationFile = layout.buildDirectory.file("jacoco/$taskName.exec").get().asFile + } + + 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) + extensions.extraProperties["includeDockerTaggedTests"] = true + + // Database tests mutate process-wide state and must remain sequential within each lane. + maxParallelForks = 1 + systemProperty("junit.jupiter.execution.parallel.enabled", "false") + + if (backend != "h2") { + doFirst { + if (rootProject.extra["dockerTest"] != true) { + throw GradleException( + "$path requires Docker; use -PskipDockerTests=false with Docker running." + ) + } + } + } + } +} + +registerCoreTestTask("coreUnitTest") +registerCoreTestTask("coreH2Test", "h2") +registerCoreTestTask("coreMySQLTest", "mysql") +registerCoreTestTask("corePostgreSQLTest", "postgresql") + +tasks.register("coreTestLaneOf") { + group = "verification" + description = "Prints which core database test lane(s) a class runs in, from its tags, " + + "without running anything. Usage: -PclassName=" + dependsOn(tasks.named("testClasses")) + classpath = sourceSets["test"].runtimeClasspath + mainClass.set("org.apache.gravitino.storage.relational.CoreTestLaneOf") + doFirst { + val className = project.findProperty("className") as? String + ?: throw GradleException( + "Usage: ./gradlew :core:coreTestLaneOf -PclassName=" + ) + args(className) + } +} + +val coreSuiteCoverage = + providers.gradleProperty("coreSuiteCoverage").map(String::toBoolean).orElse(false) +val coreSuiteTaskNames = + listOf("coreUnitTest", "coreH2Test", "coreMySQLTest", "corePostgreSQLTest") +val coreSuiteExecutionData = + coreSuiteTaskNames.map { layout.buildDirectory.file("jacoco/$it.exec") } +val validateCoreSuiteCoverage by tasks.registering { + inputs.files(coreSuiteExecutionData) + + doLast { + val missingExecutionData = + coreSuiteExecutionData + .map { it.get().asFile } + .filterNot { it.isFile && it.length() > 0L } + if (missingExecutionData.isNotEmpty()) { + throw GradleException( + "Missing core JaCoCo execution data: ${missingExecutionData.joinToString()}" + ) + } + } +} + +tasks.named("jacocoTestReport") { + if (coreSuiteCoverage.get()) { + dependsOn(tasks.named("classes"), validateCoreSuiteCoverage) + executionData.setFrom(coreSuiteExecutionData) + } +} + +// :core:test is the java plugin's built-in `test` task, kept registered (and working) only for +// backward compatibility - IDEs and other tooling may still target it by convention. It is +// deprecated in place, not removed: +// - Dev CUJ: a contributor running tests locally should target one of the four lanes registered +// above (coreUnitTest / coreH2Test / coreMySQLTest / corePostgreSQLTest), never :core:test - +// it predates the lane split and does not correspond to any CI lane. Check where a class runs +// with `./gradlew :core:coreTestLaneOf -PclassName=...` instead of guessing. +// - CI CUJ: no change needed here. CI never invokes :core:test - dev/ci/test-shards.sh emits +// `-x :core:test` for the `others` shard, so the warning below only ever fires for a developer +// running it directly. tasks.test { + doFirst { + logger.warn( + "WARNING: :core:test is deprecated and does not correspond to any CI lane. Use " + + "coreUnitTest, coreH2Test, coreMySQLTest, or corePostgreSQLTest instead - run " + + "./gradlew :core:coreTestLaneOf -PclassName= to check which " + + "one(s) a class belongs to." + ) + } val testMode = project.properties["testMode"] as? String ?: "embedded" if (testMode == "embedded") { environment("GRAVITINO_HOME", project.rootDir.path) diff --git a/core/src/test/java/org/apache/gravitino/stats/storage/TestJdbcPartitionStatisticStorageIT.java b/core/src/test/java/org/apache/gravitino/stats/storage/TestJdbcPartitionStatisticStorageIT.java index b1cac18a986..38d421bd547 100644 --- a/core/src/test/java/org/apache/gravitino/stats/storage/TestJdbcPartitionStatisticStorageIT.java +++ b/core/src/test/java/org/apache/gravitino/stats/storage/TestJdbcPartitionStatisticStorageIT.java @@ -58,6 +58,7 @@ import org.apache.gravitino.stats.PartitionStatisticsUpdate; import org.apache.gravitino.stats.StatisticValue; import org.apache.gravitino.stats.StatisticValues; +import org.apache.gravitino.storage.relational.CoreBackend; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Nested; @@ -93,7 +94,8 @@ public class TestJdbcPartitionStatisticStorageIT { /** * Abstract base class containing all test logic. Each database-specific test class extends this - * and implements the database setup. + * and implements the database setup. Each subclass carries the tag of the one backend lane it + * runs in. */ @TestInstance(TestInstance.Lifecycle.PER_CLASS) abstract static class BaseJdbcPartitionStatisticStorageTest { @@ -585,6 +587,7 @@ protected void cleanupAllStatistics() throws IOException { /** MySQL-specific tests using Docker container. */ @Nested + @CoreBackend.MySQL @Tag("gravitino-docker-test") static class MySQLTest extends BaseJdbcPartitionStatisticStorageTest { @@ -655,6 +658,7 @@ private void createMySQLSchema() throws SQLException { /** PostgreSQL-specific tests using Docker container. */ @Nested + @CoreBackend.PostgreSQL @Tag("gravitino-docker-test") static class PostgreSQLTest extends BaseJdbcPartitionStatisticStorageTest { @@ -728,6 +732,7 @@ private void createPostgreSQLSchema() throws SQLException { /** H2-specific tests using embedded in-memory database. */ @Nested + @CoreBackend.H2 static class H2Test extends BaseJdbcPartitionStatisticStorageTest { private static final String H2_JDBC_URL = diff --git a/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java b/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java index cac53eed61c..7cb53318d56 100644 --- a/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java +++ b/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java @@ -49,6 +49,7 @@ import java.sql.Statement; import java.time.Instant; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -92,6 +93,8 @@ import org.apache.gravitino.meta.TopicEntity; import org.apache.gravitino.meta.UserEntity; import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.storage.relational.BackendTestSelector; +import org.apache.gravitino.storage.relational.CoreBackend; import org.apache.gravitino.storage.relational.RelationalBackend; import org.apache.gravitino.storage.relational.RelationalEntityStore; import org.apache.gravitino.storage.relational.RelationalGarbageCollector; @@ -107,6 +110,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@CoreBackend.All abstract class AbstractEntityStorageTest { protected static final Logger LOG = LoggerFactory.getLogger(AbstractEntityStorageTest.class); @@ -116,14 +120,18 @@ abstract class AbstractEntityStorageTest { protected static final String H2_FILE = DB_DIR + ".mv.db"; static Object[][] storageProvider() { - return new Object[][] { - {"h2", true}, - {"h2", false}, - {"mysql", true}, - {"mysql", false}, - {"postgresql", true}, - {"postgresql", false} - }; + Object[][] backends = + new Object[][] { + {"h2", true}, + {"h2", false}, + {"mysql", true}, + {"mysql", false}, + {"postgresql", true}, + {"postgresql", false} + }; + return Arrays.stream(backends) + .filter(arguments -> BackendTestSelector.isSelected((String) arguments[0])) + .toArray(Object[][]::new); } @AfterEach diff --git a/core/src/test/java/org/apache/gravitino/storage/TestBackendTestSelector.java b/core/src/test/java/org/apache/gravitino/storage/TestBackendTestSelector.java new file mode 100644 index 00000000000..642d70a1110 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/TestBackendTestSelector.java @@ -0,0 +1,138 @@ +/* + * 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; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Optional; +import org.apache.gravitino.storage.relational.BackendTestExtension; +import org.apache.gravitino.storage.relational.BackendTestSelector; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.TestTemplateInvocationContext; +import org.junit.jupiter.api.parallel.ResourceAccessMode; +import org.junit.jupiter.api.parallel.ResourceLock; +import org.junit.jupiter.api.parallel.Resources; +import org.mockito.Mockito; + +/** Tests backend selection for the core database test suites. */ +@ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ_WRITE) +public class TestBackendTestSelector { + + private static final String BACKEND_PROPERTY = "gravitino.core.test.backend"; + + private Optional originalBackend = Optional.empty(); + + @BeforeEach + void saveAndClearBackendProperty() { + originalBackend = Optional.ofNullable(System.getProperty(BACKEND_PROPERTY)); + System.clearProperty(BACKEND_PROPERTY); + } + + @AfterEach + void restoreBackendProperty() { + System.clearProperty(BACKEND_PROPERTY); + originalBackend.ifPresent(value -> System.setProperty(BACKEND_PROPERTY, value)); + } + + @Test + void testAbsentSelectionPreservesLegacyBehavior() { + assertEquals(Optional.empty(), BackendTestSelector.selectedBackend()); + assertTrue(BackendTestSelector.isSelected("h2")); + assertTrue(BackendTestSelector.isSelected("mysql")); + assertTrue(BackendTestSelector.isSelected("postgresql")); + } + + @Test + void testSelectionIsNormalizedAndValidated() { + System.setProperty(BACKEND_PROPERTY, " MySQL "); + + assertEquals(Optional.of("mysql"), BackendTestSelector.selectedBackend()); + assertTrue(BackendTestSelector.isSelected("MYSQL")); + assertFalse(BackendTestSelector.isSelected("h2")); + + System.setProperty(BACKEND_PROPERTY, "unsupported"); + assertThrows(IllegalArgumentException.class, BackendTestSelector::selectedBackend); + } + + @Test + void testTemplateProviderUsesSelectedBackendAndMethodName() throws NoSuchMethodException { + System.setProperty(BACKEND_PROPERTY, "mysql"); + BackendTestExtension extension = new BackendTestExtension(); + + String firstDisplayName = + selectedInvocation(extension, "firstTemplateMethod").getDisplayName(1); + String secondDisplayName = + selectedInvocation(extension, "secondTemplateMethod").getDisplayName(1); + + assertEquals("firstTemplateMethod[MYSQL Backend]", firstDisplayName); + assertEquals("secondTemplateMethod[MYSQL Backend]", secondDisplayName); + assertNotEquals(firstDisplayName, secondDisplayName); + } + + @Test + void testStorageProviderPreservesLegacyMatrix() { + assertArrayEquals( + new Object[][] { + {"h2", true}, + {"h2", false}, + {"mysql", true}, + {"mysql", false}, + {"postgresql", true}, + {"postgresql", false} + }, + AbstractEntityStorageTest.storageProvider()); + } + + @Test + void testStorageProviderUsesSelectedBackend() { + System.setProperty(BACKEND_PROPERTY, "postgresql"); + + assertArrayEquals( + new Object[][] {{"postgresql", true}, {"postgresql", false}}, + AbstractEntityStorageTest.storageProvider()); + } + + private static TestTemplateInvocationContext selectedInvocation( + BackendTestExtension extension, String methodName) throws NoSuchMethodException { + ExtensionContext context = Mockito.mock(ExtensionContext.class); + Mockito.when(context.getRequiredTestMethod()) + .thenReturn(TemplateMethods.class.getDeclaredMethod(methodName)); + + List invocations = + extension.provideTestTemplateInvocationContexts(context).toList(); + + assertEquals(1, invocations.size()); + return invocations.get(0); + } + + private static class TemplateMethods { + void firstTemplateMethod() {} + + void secondTemplateMethod() {} + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java b/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java index a9377c8b599..7dc09519136 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java @@ -37,6 +37,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Stream; @@ -108,6 +109,14 @@ public boolean supportsTestTemplate(ExtensionContext context) { @Override public Stream provideTestTemplateInvocationContexts( ExtensionContext context) { + String testMethodName = context.getRequiredTestMethod().getName(); + Optional selectedBackend = BackendTestSelector.selectedBackend(); + if (selectedBackend.isPresent()) { + LOG.info("Running tests with the selected {} backend.", selectedBackend.get()); + return Stream.of(selectedBackend.get()) + .map(backendType -> new BackendInvocationContext(testMethodName, backendType)); + } + List backendsToTest = new ArrayList<>(); backendsToTest.add("h2"); // Always test with H2 @@ -121,19 +130,25 @@ public Stream provideTestTemplateInvocationContex "Running tests with H2 backend only. Set env var 'dockerTest=true' to include all backends."); } - return backendsToTest.stream().map(BackendInvocationContext::new); + return backendsToTest.stream() + .map(backendType -> new BackendInvocationContext(testMethodName, backendType)); } private static class BackendInvocationContext implements TestTemplateInvocationContext { + private final String testMethodName; private final String backendType; - public BackendInvocationContext(String backendType) { + public BackendInvocationContext(String testMethodName, String backendType) { + this.testMethodName = testMethodName; this.backendType = backendType; } @Override public String getDisplayName(int invocationIndex) { - return String.format("[%s Backend]", backendType.toUpperCase()); + // No trailing "()" here: @TestTemplate methods can declare parameters (e.g. an injected + // DatabaseTestContext), and a hardcoded empty parameter list would misrepresent the + // method's actual signature in JUnit XML/HTML reports. + return String.format("%s[%s Backend]", testMethodName, backendType.toUpperCase()); } @Override diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestSelector.java b/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestSelector.java new file mode 100644 index 00000000000..3f794dbea80 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestSelector.java @@ -0,0 +1,71 @@ +/* + * 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 java.util.Locale; +import java.util.Optional; +import java.util.Set; + +/** Selects a single JDBC backend for the core database test suites. */ +public final class BackendTestSelector { + + private static final String BACKEND_PROPERTY = "gravitino.core.test.backend"; + private static final Set SUPPORTED_BACKENDS = Set.of("h2", "mysql", "postgresql"); + + private BackendTestSelector() {} + + /** + * Returns the selected backend, or an empty value when the legacy all-applicable-backends + * behavior should be used. + * + * @return the normalized selected backend + * @throws IllegalArgumentException if the configured backend is unsupported + */ + public static Optional selectedBackend() { + String configuredBackend = System.getProperty(BACKEND_PROPERTY); + if (configuredBackend == null) { + return Optional.empty(); + } + + return Optional.of(validate(configuredBackend)); + } + + /** + * Returns whether a backend should run under the current selection. + * + * @param backend backend to test + * @return true when no backend is selected or the backend matches the selection + * @throws IllegalArgumentException if either backend value is unsupported + */ + public static boolean isSelected(String backend) { + String normalizedBackend = validate(backend); + return selectedBackend().map(normalizedBackend::equals).orElse(true); + } + + private static String validate(String backend) { + String normalizedBackend = backend.trim().toLowerCase(Locale.ROOT); + if (!SUPPORTED_BACKENDS.contains(normalizedBackend)) { + throw new IllegalArgumentException( + String.format( + "Unsupported core test backend '%s'; expected one of %s", + backend, SUPPORTED_BACKENDS)); + } + return normalizedBackend; + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/CoreBackend.java b/core/src/test/java/org/apache/gravitino/storage/relational/CoreBackend.java new file mode 100644 index 00000000000..2e2d122ee32 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/CoreBackend.java @@ -0,0 +1,109 @@ +/* + * 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 java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.junit.jupiter.api.Tag; + +/** + * Namespace for the annotations that select which core database test lane(s) a class runs in. + * + *

The core test suite is split into one Gradle task per backend ({@code coreH2Test}, {@code + * coreMySQLTest}, {@code corePostgreSQLTest}), and each lane simply includes the classes carrying + * its backend tag. A class runs under exactly the lanes it is tagged for - there is no separate "is + * this a database test" gatekeeper tag to keep in sync: + * + *

    + *
  • {@link H2}, {@link MySQL}, {@link PostgreSQL} pin a class to one lane; stack more than one + * to run under several lanes, e.g. {@code @CoreBackend.H2 @CoreBackend.MySQL}. A class tagged + * for some but not all backends must satisfy the CI-legality constraint below. + *
  • {@link All} runs a class under all three lanes. + *
  • A class with none of these is a unit test and runs in {@code coreUnitTest} - unless it also + * carries {@code @Tag("gravitino-docker-test")}, in which case it runs in no lane at all. + * Check with {@code ./gradlew :core:coreTestLaneOf -PclassName=}. + *
+ * + *

Each is a plain JUnit composed annotation: meta-annotated with {@link Tag} and nothing else, + * so it is exactly equivalent to writing the raw {@code @Tag} string(s) on the class. JUnit expands + * it while scanning class annotations during test discovery (see {@code + * org.junit.platform.commons.support.AnnotationSupport#findRepeatableAnnotations}), so {@code + * core/build.gradle.kts} only needs the tag strings themselves and no execution-time condition is + * involved. {@link #H2_TAG}, {@link #MYSQL_TAG}, and {@link #POSTGRESQL_TAG} must stay in sync with + * {@code coreBackendTestTags} in {@code core/build.gradle.kts}. + * + *

Because {@code dev/ci/core_test_identity.py}'s {@code reconcile} step (invoked from {@code + * .github/workflows/build.yml}, not from {@code core/build.gradle.kts}) requires the + * h2/mysql/postgresql lanes to run the exact same set of normalized test identities, a single- or + * multi- (but not all-) backend class is only CI-legal as a normalized sibling of matching classes + * in the other backend(s) it omits - see {@code TestJdbcPartitionStatisticStorageIT}'s {@code + * H2Test}/{@code MySQLTest}/{@code PostgreSQLTest} nested classes for the pattern this currently + * requires. + */ +public final class CoreBackend { + + /** The JUnit tag that selects the H2 lane. */ + public static final String H2_TAG = "gravitino-core-h2-test"; + + /** The JUnit tag that selects the MySQL lane. */ + public static final String MYSQL_TAG = "gravitino-core-mysql-test"; + + /** The JUnit tag that selects the PostgreSQL lane. */ + public static final String POSTGRESQL_TAG = "gravitino-core-postgresql-test"; + + private CoreBackend() {} + + /** Pins a core database test class to the H2 lane ({@code coreH2Test}). */ + @Documented + @Inherited + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @Tag(H2_TAG) + public @interface H2 {} + + /** Pins a core database test class to the MySQL lane ({@code coreMySQLTest}). */ + @Documented + @Inherited + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @Tag(MYSQL_TAG) + public @interface MySQL {} + + /** Pins a core database test class to the PostgreSQL lane ({@code corePostgreSQLTest}). */ + @Documented + @Inherited + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @Tag(POSTGRESQL_TAG) + public @interface PostgreSQL {} + + /** Runs a core database test class under every backend lane. */ + @Documented + @Inherited + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @Tag(H2_TAG) + @Tag(MYSQL_TAG) + @Tag(POSTGRESQL_TAG) + public @interface All {} +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/CoreTestLaneOf.java b/core/src/test/java/org/apache/gravitino/storage/relational/CoreTestLaneOf.java new file mode 100644 index 00000000000..a9759a37bf3 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/CoreTestLaneOf.java @@ -0,0 +1,93 @@ +/* + * 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 java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Tag; +import org.junit.platform.commons.support.AnnotationSupport; + +/** + * A local, discovery-only check: prints which core database test lane(s) a class runs in, from its + * tags, without running anything. Reads a class's tags with the same lookup the Jupiter engine uses + * at discovery time (see {@link CoreBackend}), so the answer matches what {@code + * core/build.gradle.kts}'s lane tasks would actually do. + * + *

Run with {@code ./gradlew :core:coreTestLaneOf -PclassName=}. + */ +public final class CoreTestLaneOf { + + private CoreTestLaneOf() {} + + /** + * Entry point. + * + * @param args exactly one fully-qualified class name to inspect + */ + public static void main(String[] args) { + if (args.length != 1) { + System.err.println("Usage: CoreTestLaneOf "); + System.exit(1); + return; + } + + Class testClass; + try { + testClass = Class.forName(args[0]); + } catch (ClassNotFoundException e) { + System.err.println("Class not found on the test classpath: " + args[0]); + System.exit(1); + return; + } + + Set tags = + AnnotationSupport.findRepeatableAnnotations(testClass, Tag.class).stream() + .map(Tag::value) + .collect(Collectors.toSet()); + + List lanes = new ArrayList<>(); + if (tags.contains(CoreBackend.H2_TAG)) { + lanes.add("coreH2Test"); + } + if (tags.contains(CoreBackend.MYSQL_TAG)) { + lanes.add("coreMySQLTest"); + } + if (tags.contains(CoreBackend.POSTGRESQL_TAG)) { + lanes.add("corePostgreSQLTest"); + } + + System.out.println(args[0] + " tags: " + tags); + if (!lanes.isEmpty()) { + System.out.println(args[0] + " runs in: " + String.join(", ", lanes)); + return; + } + + if (tags.contains("gravitino-docker-test")) { + System.out.println( + args[0] + + " carries gravitino-docker-test but no backend tag - it will NOT run in ANY" + + " lane. Add @CoreBackend.H2/@CoreBackend.MySQL/@CoreBackend.PostgreSQL or" + + " @CoreBackend.All."); + } else { + System.out.println(args[0] + " carries no backend tag - runs in coreUnitTest."); + } + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestCoreDatabaseLaneAnnotations.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestCoreDatabaseLaneAnnotations.java new file mode 100644 index 00000000000..2aeb7cc0f93 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestCoreDatabaseLaneAnnotations.java @@ -0,0 +1,141 @@ +/* + * 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.assertEquals; + +import java.lang.annotation.Annotation; +import java.lang.annotation.Documented; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Tags; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.platform.commons.support.AnnotationSupport; + +/** + * Pins the contract of {@link CoreBackend.H2}, {@link CoreBackend.MySQL}, {@link + * CoreBackend.PostgreSQL}, and {@link CoreBackend.All}: each must expand to exactly the tag(s) + * {@code core/build.gradle.kts} filters on, for the annotated class and for its subclasses, using + * the same annotation lookup the Jupiter engine runs at discovery time - and none may carry an + * execution-time hook, since that would repeat the mistake a prior, reverted design made. + */ +class TestCoreDatabaseLaneAnnotations { + + private static Stream annotationsAndExpectedTags() { + return Stream.of( + new Object[] {H2Annotated.class, List.of(CoreBackend.H2_TAG)}, + new Object[] {MySQLAnnotated.class, List.of(CoreBackend.MYSQL_TAG)}, + new Object[] {PostgreSQLAnnotated.class, List.of(CoreBackend.POSTGRESQL_TAG)}, + new Object[] { + AllAnnotated.class, + List.of(CoreBackend.H2_TAG, CoreBackend.MYSQL_TAG, CoreBackend.POSTGRESQL_TAG) + }); + } + + // Default JUnit display names include argument toStrings - for expectedTags that would print a + // tag string like "[gravitino-core-h2-test]" into this class's own JUnit XML, which is itself + // part of the coreUnitTest lane, and core_test_identity.py's manifest step rejects any standalone + // backend token there as a foreign-lane marker. Naming on {0} (the class under test) only avoids + // that: its simple name (e.g. H2Annotated) has no such token, since "h2"/"mysql"/"postgresql" is + // never followed by a non-letter there. + @ParameterizedTest(name = "{index}: {0}") + @MethodSource("annotationsAndExpectedTags") + void testExpandsToExpectedTags(Class annotatedClass, List expectedTags) { + assertEquals(expectedTags, tagsOf(annotatedClass)); + } + + @Test + void testStackingTwoAnnotationsRunsUnderBothLanes() { + assertEquals(List.of(CoreBackend.H2_TAG, CoreBackend.MYSQL_TAG), tagsOf(H2AndMySQL.class)); + } + + @Test + void testSubclassInheritsTags() { + assertEquals(List.of(CoreBackend.H2_TAG), tagsOf(H2Subclass.class)); + assertEquals( + List.of(CoreBackend.H2_TAG, CoreBackend.MYSQL_TAG, CoreBackend.POSTGRESQL_TAG), + tagsOf(AllSubclass.class)); + } + + @Test + void testNoneOfThemCarryAnExecutionTimeHook() { + // No @ExtendWith or similar on any of them: lane filtering stays a discovery-time tag + // match, so classes left out of a lane never appear in that lane's JUnit XML. + Set> tagOnlyMetaAnnotations = + Set.of( + Documented.class, + Inherited.class, + Retention.class, + Target.class, + Tag.class, + Tags.class); + for (Class annotationType : + List.of( + CoreBackend.H2.class, + CoreBackend.MySQL.class, + CoreBackend.PostgreSQL.class, + CoreBackend.All.class)) { + Set> metaAnnotations = + Arrays.stream(annotationType.getAnnotations()) + .map(Annotation::annotationType) + .collect(Collectors.toSet()); + assertEquals( + Set.of(), + metaAnnotations.stream() + .filter(type -> !tagOnlyMetaAnnotations.contains(type)) + .collect(Collectors.toSet()), + annotationType.getSimpleName() + " carries an unexpected, non-tag meta-annotation"); + } + } + + private static List tagsOf(Class testClass) { + return AnnotationSupport.findRepeatableAnnotations(testClass, Tag.class).stream() + .map(Tag::value) + .collect(Collectors.toList()); + } + + @CoreBackend.H2 + private static class H2Annotated {} + + @CoreBackend.MySQL + private static class MySQLAnnotated {} + + @CoreBackend.PostgreSQL + private static class PostgreSQLAnnotated {} + + @CoreBackend.All + private static class AllAnnotated {} + + @CoreBackend.H2 + @CoreBackend.MySQL + private static class H2AndMySQL {} + + private static class H2Subclass extends H2Annotated {} + + private static class AllSubclass extends AllAnnotated {} +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java index 1e4fe8c884f..009c39d99f7 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java @@ -84,6 +84,7 @@ import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; +@CoreBackend.All @TestInstance(TestInstance.Lifecycle.PER_CLASS) @ExtendWith({ BackendTestExtension.class, diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaService.java index 9ddbfe51c4b..abd5802a537 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaService.java @@ -35,7 +35,6 @@ import org.apache.gravitino.utils.NameIdentifierUtil; import org.apache.gravitino.utils.NamespaceUtil; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestTemplate; public class TestJobMetaService extends TestJDBCBackend { @@ -492,51 +491,4 @@ public void testUpdateJobWithMismatchedIdThrowsIllegalArgumentException() throws .withFinishedAt(oldJob.finishedAt()) .build())); } - - @Test - public void testUpdateJobWithMalformedIdentifierThrowsNoSuchEntityException() { - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .updateJob(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"), e -> e)); - - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .updateJob( - NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX), e -> e)); - } - - @Test - public void testGetJobWithMalformedIdentifierThrowsNoSuchEntityException() { - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .getJobByIdentifier(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"))); - - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .getJobByIdentifier( - NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX))); - } - - @Test - public void testDeleteJobWithMalformedIdentifierThrowsNoSuchEntityException() { - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .deleteJob(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"))); - - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .deleteJob(NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX))); - } } diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaServiceValidation.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaServiceValidation.java new file mode 100644 index 00000000000..2d7f6ea6176 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaServiceValidation.java @@ -0,0 +1,78 @@ +/* + * 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.service; + +import org.apache.gravitino.exceptions.NoSuchEntityException; +import org.apache.gravitino.job.JobHandle; +import org.apache.gravitino.utils.NameIdentifierUtil; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** Tests job identifier validation that does not require a JDBC backend. */ +public class TestJobMetaServiceValidation { + + private static final String METALAKE_NAME = "metalake_test_job_meta_service"; + + @Test + void testUpdateJobWithMalformedIdentifierThrowsNoSuchEntityException() { + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .updateJob(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"), e -> e)); + + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .updateJob( + NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX), e -> e)); + } + + @Test + void testGetJobWithMalformedIdentifierThrowsNoSuchEntityException() { + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .getJobByIdentifier(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"))); + + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .getJobByIdentifier( + NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX))); + } + + @Test + void testDeleteJobWithMalformedIdentifierThrowsNoSuchEntityException() { + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .deleteJob(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"))); + + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .deleteJob(NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX))); + } +} diff --git a/design-docs/testing/core-db-split.md b/design-docs/testing/core-db-split.md new file mode 100644 index 00000000000..7c590d07ea7 --- /dev/null +++ b/design-docs/testing/core-db-split.md @@ -0,0 +1,671 @@ + + +# SPIP: Split core tests into explicit database lanes + +| | | +|---|---| +| **Status** | Implemented on PR [apache/gravitino#13517](https://github.com/apache/gravitino/pull/13517); this document records the final design retroactively | +| **Scope** | `core` module test execution, `core/build.gradle.kts`, `dev/ci/core_test_identity.py`, `.github/workflows/build.yml` | +| **Format** | Apache Spark SPIP (Heilmeier Catechism) | + +## Q1. What are you trying to do? + +Make it explicit, per test class, which database(s) a core test runs against, and make the +build honor that declaration exactly. + +Concretely: + +- Replace the single `:core:test` run with four Gradle lanes: `coreUnitTest` (no database, no + Docker), `coreH2Test`, `coreMySQLTest`, and `corePostgreSQLTest`. +- Give contributors one typed way to put a class into a lane: `@CoreBackend.H2`, + `@CoreBackend.MySQL`, `@CoreBackend.PostgreSQL`, or `@CoreBackend.All`. No raw tag strings. +- Guarantee that a class not tagged for a lane leaves no trace in that lane's output, so CI can + prove the three database lanes ran the same test contract. +- Give a developer a way to ask "which lane does my class run in?" without running any test. +- Warn a developer off the old, unsplit `:core:test` entry point without deleting it. + +## Q2. What problem is this proposal NOT designed to solve? + +These are deliberate exclusions, not oversights. + +- **No CI guard for the orphan case.** A class that carries `gravitino-docker-test` but no + `@CoreBackend.*` annotation is excluded from `coreUnitTest` (by the Docker tag) and from every + backend lane (no backend tag), so it runs nowhere. The design does not add a CI step that + scans for this. Mitigation is developer self-service: `./gradlew :core:coreTestLaneOf` + prints an explicit warning for exactly this shape, and the build script comment on + `coreBackendTestTags` documents it. +- **No cross-lane report aggregation.** Each lane writes its own JUnit XML, HTML report, and + JaCoCo `.exec` under a lane-specific path. CI uploads them as four separate evidence artifacts + and only the JaCoCo data is merged (for coverage). There is no tool that merges the four JUnit + reports into one; readers open the lane they care about. +- **Not removing `:core:test`.** It is the `java` plugin's built-in `test` task; other tooling + (IDEs, scripts) may still target it by convention, so it stays registered and functional rather + than being deleted. It is deprecated *in place* instead (see Q4/Appendix C): running it directly + now prints a warning naming the four real lanes, but it still runs and still passes. +- **Not changing how a test selects its backend at runtime.** `BackendTestSelector` (reads the + `gravitino.core.test.backend` system property), `BackendTestExtension` (`@TestTemplate` + invocation contexts), and the `storageProvider()` parameter pattern are unchanged. The lane + decides *whether* a class runs; these decide *what it does* once it runs. + +## Q3. How is it done today, and what are the limits of current practice? + +Before this PR, `core` ran every test through one `:core:test` task. H2-backed, MySQL-backed, +and PostgreSQL-backed tests were all discovered by the same task, and the only filtering was +the repository-wide `excludeTags("gravitino-docker-test")` applied when Docker was unavailable. +There was no notion of a backend lane at all, so there was nothing for CI to reconcile. + +The first iteration on this PR branch introduced four lane tasks but decided membership with a +gatekeeper tag plus modifier tags: + +```kotlin +// state at 654cfe33a, since replaced +useJUnitPlatform { + if (backend == null) { + excludeTags(coreDatabaseTestTag, "gravitino-docker-test") + } else { + includeTags(coreDatabaseTestTag) // gatekeeper + when (backend) { + "h2" -> excludeTags(coreMySQLTestTag, corePostgreSQLTestTag) + "mysql" -> excludeTags(coreH2TestTag, corePostgreSQLTestTag) + "postgresql" -> excludeTags(coreH2TestTag, coreMySQLTestTag) + } + } +} +``` + +A class had to carry `gravitino-core-database-test` to enter any database lane, and then +optionally carried per-backend tags whose *absence* meant "all backends". This had three +concrete failure modes, all silent: + +1. **Gatekeeper desync.** A class with correct per-backend tags but no gatekeeper tag was + invisible to every lane. Nothing failed; the tests just never ran. +2. **Two tags meant zero lanes.** Because each lane *excluded* the other two backends' tags, a + class tagged for both H2 and MySQL was excluded from H2 (carries MySQL tag), from MySQL + (carries H2 tag), and from PostgreSQL. An intermediate fix (46eef2130) replaced the + `when` with a boolean tag expression to make multi-tagged classes run under each lane, but + the expression was hard to read and still depended on the gatekeeper. +3. **Typos compiled.** Tags were raw `@Tag("gravitino-core-h2-test")` string literals. A + misspelling was a valid, unrelated tag, so the class silently dropped out of its lane. + +There was also no way to answer "where will this class run?" other than running a lane and +grepping the resulting XML. + +## Q4. What is new in your approach, and why do you think it will be successful? + +### Three peer tags, nothing else + +The gatekeeper is gone. Lane membership is decided by exactly three JUnit tags, defined once +in `core/build.gradle.kts` and mirrored as constants in `CoreBackend`: + +```kotlin +val coreBackendTestTags = linkedMapOf( + "h2" to "gravitino-core-h2-test", + "mysql" to "gravitino-core-mysql-test", + "postgresql" to "gravitino-core-postgresql-test" +) +``` + +Each backend lane is a plain `includeTags(ownBackendTag)`. The unit lane is +`excludeTags(, "gravitino-docker-test")`. "Runs on all backends" means carrying all +three tags explicitly, never carrying none. A class tagged for two backends runs in both, because +each lane only looks for its own tag and never excludes another backend's. + +### Typed annotations that are pure `@Tag` composition + +Contributors never write tag strings. `CoreBackend` is a final namespace class with four nested +marker annotations; each is meta-annotated with `@Tag` (one or three) and the standard +`@Documented @Inherited @Retention(RUNTIME) @Target(TYPE)` set, and nothing else. This is the +single most important property of the design: **filtering happens at JUnit discovery time**. +Gradle's `includeTags`/`excludeTags` become a JUnit Platform `PostDiscoveryFilter`, which reads +class tags reflectively via `AnnotationSupport.findRepeatableAnnotations(clazz, Tag.class)`. +Because the annotations carry no `@ExtendWith` or other execution-time hook, an excluded class is +pruned from the discovered test plan before execution begins and therefore produces no +`` element, not even a `` one, in the lane's JUnit XML. + +That hard-zero property is what `dev/ci/core_test_identity.py reconcile` depends on (Appendix +B). An earlier redesign on this same branch (`@DatabaseTest` + a `BackendLaneCondition` +`ExecutionCondition`, commit efee6a4c9) was reverted (547e9bdf0) precisely because an +`ExecutionCondition` runs after discovery: the disabled class still appeared in every lane's XML +as ``, the manifest step saw foreign backend markers, and reconcile failed. + +`TestCoreDatabaseLaneAnnotations` pins this contract as a regression guard: it asserts the exact +tag expansion of each annotation, that stacking two annotations yields both tags, that subclasses +inherit the tags, and that none of the four annotation types carries any meta-annotation outside +`{Documented, Inherited, Retention, Target, Tag, Tags}`. Re-introducing an execution-time hook +fails that test. + +### A local, discovery-only lane check + +`./gradlew :core:coreTestLaneOf -PclassName=` runs `CoreTestLaneOf`, which loads the class, +calls the same `AnnotationSupport.findRepeatableAnnotations` lookup the engine uses, maps the +resulting tag set to lane task names, and prints them. If the class carries +`gravitino-docker-test` but no backend tag, it prints an explicit "will NOT run in ANY lane" +warning. Because it re-derives the answer through the identical mechanism, its output is +trustworthy without running a single test. + +### `:core:test` deprecated in place, not removed + +`:core:test` is the `java` plugin's built-in `test` task, auto-registered before this module's +build script runs. Removing it outright risks breaking any tooling (IDE run buttons, scripts) +that targets `::test` by convention across the whole repo, not just `core`. Instead its +`tasks.test { ... }` configuration block gained a `doFirst` that logs a warning naming the four +lanes and `coreTestLaneOf` every time it runs, so it still works exactly as before but visibly +tells a developer it is the wrong task. No CI change was needed: `dev/ci/test-shards.sh` already +emits `-x :core:test` for the `others` shard, so the warning only ever fires for someone running +it directly. See Appendix C for the day-to-day commands this replaces. + +### Alternatives considered (brief) + +- **Parameterized annotation** (`@CoreDBTest(backends = {H2, MYSQL})`): mechanically possible + with a custom `PostDiscoveryFilter` that reads the attribute, but rejected. JUnit's built-in + tag resolution only reads an annotation type's fixed meta-annotations, never a per-usage + attribute, so it would need a bespoke filter to interpret. More importantly, + `core_test_identity.py reconcile` requires a fixed, small set of lanes with exact identity + equality across the three database lanes; a free-form attribute-driven subset per class would + undermine that invariant rather than express it. +- **Flat top-level annotation names** (`@CoreH2Test`, `@CoreMySQLTest`, ...): rejected after a + real, reproduced compile error. `TestJdbcPartitionStatisticStorageIT` already declares nested + classes named `H2Test`, `MySQLTest`, and `PostgreSQLTest`; an unqualified annotation import of + the same simple name was shadowed by the nested class, producing + `H2Test cannot be converted to Annotation`. Namespacing under `CoreBackend` makes the + reference always qualified, so the collision is structurally impossible, and it matches the + package's existing "Backend" vocabulary (`BackendTestExtension`, `BackendTestSelector`). +- **Execution-time condition** (`@DatabaseTest` + `ExecutionCondition`): implemented, reverted; + see above. + +### Why it will work + +The design has already been run, not just reasoned about. Real `:core:coreH2Test`, +`:core:coreUnitTest`, and `:core:coreTestLaneOf` invocations on the branch confirmed that only +the matching nested class's XML is produced for `TestJdbcPartitionStatisticStorageIT`, that an +`@CoreBackend.All` class is excluded from `coreUnitTest` ("No tests found for given includes") +and included in `coreH2Test`, that the manifest step passes on the lane output, and that +`coreTestLaneOf` reports lanes for a real class and warns for a synthetic orphan. + +## Q5. Who cares? If you are successful, what difference will it make? + +- **Contributors adding core storage tests** get a two-line, compile-checked way to declare + where a test runs, and a five-second local command to confirm it. The "my test never ran and + nobody noticed" class of bug is reduced to one remaining shape (the orphan), which the local + tool names explicitly. +- **Reviewers** can read lane membership off the class declaration instead of reconstructing it + from a tag expression in Gradle. +- **CI maintainers** get a lane filter that is three trivial include/exclude lines, a + reconcile step whose hard-zero precondition is guaranteed by construction, and a regression + test that fails if anyone reintroduces an execution-time hook. +- **The project** keeps the ability to prove, on every PR, that H2, MySQL, and PostgreSQL ran + the identical normalized test contract, and that unit and database identities are disjoint. + +## Q6. What are the risks? + +- **Orphan classes run nowhere and nothing in CI says so.** A `gravitino-docker-test` class with + no `@CoreBackend.*` annotation is dropped by every lane. Accepted by design (Q2); mitigated by + `coreTestLaneOf`'s explicit warning and by documentation, not enforcement. +- **Tag string drift.** `CoreBackend.H2_TAG` / `MYSQL_TAG` / `POSTGRESQL_TAG` must stay equal to + the values in `coreBackendTestTags`. They are declared in two places (Kotlin build script and + Java test source) with no shared source. Mitigated by a comment on each side naming the other; + a mismatch surfaces as an empty lane ("No tests found for given includes") rather than silent + success, because the lane would then include a tag no class carries. +- **`:core:test` still works and still means "everything".** It is deprecated in place, not + removed (Q4), so a developer can still run the unsplit task locally and get results that do not + correspond to any CI lane; the `doFirst` warning is advisory, not a hard failure, so it is easy + to miss in a noisy log. +- **Sharded reports.** With no aggregation tooling, someone looking for "all core test + results" must open up to four reports. Accepted as scope simplification. +- **Partial-backend classes are only CI-legal as normalized siblings.** A class annotated with a + single backend (or two) passes reconcile only if matching classes exist for the backends it + omits and `core_test_identity.py`'s normalization maps them to the same identity (today: the + `TestJdbcPartitionStatisticStorageIT$H2Test/$MySQLTest/$PostgreSQLTest` shape, handled by + `STATS_BACKEND_CLASS_RE`). A new single-backend class without siblings fails reconcile with + "Database identity mismatch". This is the intended contract, but it is a constraint contributors + must know; it is documented in `CoreBackend`'s Javadoc. + +## Q7. How long will it take? + +**Done, on the PR branch:** + +- `CoreBackend` annotation namespace (`H2`, `MySQL`, `PostgreSQL`, `All`). +- Four Gradle lane tasks via `registerCoreTestTask`, with the three-peer-tag filter. +- `coreTestLaneOf` JavaExec task and `CoreTestLaneOf` main class. +- `TestCoreDatabaseLaneAnnotations` regression guard. +- Migration of all three existing usage patterns (`AbstractEntityStorageTest`, + `TestJDBCBackend`, `TestJdbcPartitionStatisticStorageIT`) and removal of the gatekeeper tag. +- Removal of the dead `compare-legacy` subcommand from `core_test_identity.py`. +- CI wiring: `build` matrix shards `core-unit`/`core-h2`/`core-mysql`/`core-postgresql` each run + one lane and upload evidence; `core-test-contract` (`needs: [changes, build]`) runs the tool's + own unit tests, downloads the four evidence artifacts, and reconciles. +- `:core:test` deprecated in place: a `doFirst` warning on its `tasks.test` block names the four + lanes and `coreTestLaneOf`; the task still runs and still passes. + +Contributor-facing documentation for the lanes lives in this document (Appendix C) rather than in +`docs/how-to-test.md`, which is a repo-wide doc unrelated to this split (it only documents the +root `./gradlew test` task and does not mention `core` or its lanes) - no edit there was needed. + +Found and fixed during review, before merge: `TestCoreDatabaseLaneAnnotations`'s +`@ParameterizedTest` used the default display name, which embeds each case's expected-tags +argument (e.g. `[gravitino-core-h2-test]`) into its own JUnit XML - since that test carries no +`@CoreBackend.*` annotation itself, it runs in `coreUnitTest`, and `manifest`'s unit-lane check +rejects any standalone backend token there as a foreign marker. Reproduced directly +(`./gradlew :core:coreUnitTest --tests ...TestCoreDatabaseLaneAnnotations` then `manifest --lane +unit` failed with "contains an explicit backend marker ['h2']"), fixed by naming on the class +under test only (`@ParameterizedTest(name = "{index}: {0}")`), re-verified clean, and confirmed +by scanning all 1968 `coreUnitTest` testcases through the real `normalize_identity` function with +zero marker errors. + +**Optional follow-up, not required to ship:** `STATS_BACKEND_CLASS_RE` (B.2/B.3) is hard-coded to +one outer class name, so it does not generalize to a second nested-per-backend class without +editing the regex. Appendix C now tells contributors to prefer `@CoreBackend.All` and flags this +constraint explicitly rather than silently teaching a pattern that fails `reconcile`; generalizing +the regex to any outer class name is a real improvement but touches CI-wired parsing logic and +needs its own fixtures/tests, so it was left out of this pass. + +**Remaining, in scope, not yet done:** + +- Reply to the open review thread on #13517 and update the PR description to match the final + design. + +## Q8. What are the mid-term and final "exams" to check for success? + +Each criterion is concrete and checkable. + +**Mid-term (already verifiable on the branch):** + +1. `./gradlew :core:coreUnitTest -PskipITs --tests '*TestCoreDatabaseLaneAnnotations*'` passes: each + annotation expands to exactly its expected tags, stacking and inheritance hold, and no + annotation carries a non-tag meta-annotation. +2. `./gradlew :core:coreH2Test` produces JUnit XML under `core/build/test-results/coreH2Test/` + containing no `` whose classname or name carries a `mysql` or `postgresql` marker + (`python3 dev/ci/core_test_identity.py manifest --lane h2 ...` exits 0). +3. `./gradlew :core:coreUnitTest` on an `@CoreBackend.All` class reports "No tests found for + given includes" for that class. +4. `./gradlew :core:coreTestLaneOf -PclassName=org.apache.gravitino.storage.relational.TestJDBCBackend` + prints `runs in: coreH2Test, coreMySQLTest, corePostgreSQLTest` in under five seconds once + test classes are compiled, without executing any test. +5. `coreTestLaneOf` on a class tagged only `gravitino-docker-test` prints the "will NOT run in + ANY lane" warning. +6. `./gradlew :core:test -PskipITs --tests ` still passes and now also logs the + `:core:test is deprecated ...` warning naming the four lanes and `coreTestLaneOf`. + +**Final (CI, on every PR touching core):** + +7. All four `build` shards succeed and each uploads `core--test-evidence` containing a + non-empty `.json` manifest, JUnit XML, HTML report, and JaCoCo `.exec`. +8. `core-test-contract` passes: `reconcile` reports `database_identities_equal: true` and + `unit_database_disjoint: true`, i.e. the H2, MySQL, and PostgreSQL manifests hold identical + normalized identity multisets and the unit manifest shares none of them. +9. No `@Tag("gravitino-core-*-test")` string literal exists in `core/src/test` (all lane + membership goes through `@CoreBackend.*`). + +--- + +## Appendix A: API Changes + +### A.1 `CoreBackend` annotations + +Location: `core/src/test/java/org/apache/gravitino/storage/relational/CoreBackend.java`. +Test-source only; not part of any published artifact. + +```java +public final class CoreBackend { + public static final String H2_TAG = "gravitino-core-h2-test"; + public static final String MYSQL_TAG = "gravitino-core-mysql-test"; + public static final String POSTGRESQL_TAG = "gravitino-core-postgresql-test"; + + private CoreBackend() {} + + @Documented @Inherited @Retention(RUNTIME) @Target(TYPE) + @Tag(H2_TAG) + public @interface H2 {} + + @Documented @Inherited @Retention(RUNTIME) @Target(TYPE) + @Tag(MYSQL_TAG) + public @interface MySQL {} + + @Documented @Inherited @Retention(RUNTIME) @Target(TYPE) + @Tag(POSTGRESQL_TAG) + public @interface PostgreSQL {} + + @Documented @Inherited @Retention(RUNTIME) @Target(TYPE) + @Tag(H2_TAG) @Tag(MYSQL_TAG) @Tag(POSTGRESQL_TAG) + public @interface All {} +} +``` + +Semantics: + +| Declaration | Tags carried | Lanes | +|---|---|---| +| (none) | none | `coreUnitTest` | +| `@CoreBackend.H2` | `h2` | `coreH2Test` | +| `@CoreBackend.H2 @CoreBackend.MySQL` | `h2`, `mysql` | `coreH2Test`, `coreMySQLTest` | +| `@CoreBackend.All` | `h2`, `mysql`, `postgresql` | all three backend lanes | +| `@Tag("gravitino-docker-test")` only | `docker` | **none** (orphan) | + +`@Target(TYPE)` restricts the annotations to classes. `@Inherited` means an abstract base class +can carry the annotation and every concrete subclass (including Jupiter `@Nested` classes and +`TestJDBCBackend` subclasses) inherits lane membership. The three `*_TAG` constants are the +Java-side mirror of `coreBackendTestTags` in `core/build.gradle.kts` and must be kept equal. + +Usage patterns as migrated on the PR: + +```java +// Multi-backend via a parameter provider; the lane's system property narrows storageProvider(). +@CoreBackend.All +abstract class AbstractEntityStorageTest { + static Object[][] storageProvider() { /* filtered by BackendTestSelector.isSelected */ } +} + +// Multi-backend via @TestTemplate; BackendTestExtension emits one invocation for the lane's backend. +@CoreBackend.All +@ExtendWith({BackendTestExtension.class, ...}) +public abstract class TestJDBCBackend { ... } + +// One @Nested class per single backend; each nested class is its own lane member. +@Tag("gravitino-docker-test") +public class TestJdbcPartitionStatisticStorageIT { + @Nested @CoreBackend.MySQL @Tag("gravitino-docker-test") static class MySQLTest extends Base {} + @Nested @CoreBackend.PostgreSQL @Tag("gravitino-docker-test") static class PostgreSQLTest extends Base {} + @Nested @CoreBackend.H2 static class H2Test extends Base {} +} +``` + +### A.2 Gradle tasks + +Location: `core/build.gradle.kts`. + +```kotlin +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(taskName) { ... } + +registerCoreTestTask("coreUnitTest") +registerCoreTestTask("coreH2Test", "h2") +registerCoreTestTask("coreMySQLTest", "mysql") +registerCoreTestTask("corePostgreSQLTest", "postgresql") + +tasks.register("coreTestLaneOf") { ... } +``` + +Per-task configuration set by `registerCoreTestTask`: + +| Property | Unit lane (`backend == null`) | Backend lane | +|---|---|---| +| `useJUnitPlatform` filter | `excludeTags(h2, mysql, postgresql, "gravitino-docker-test")` | `includeTags(coreBackendTestTags[backend])` | +| `systemProperty(gravitino.core.test.backend)` | not set | `backend` | +| `extraProperties["includeDockerTaggedTests"]` | not set (root build applies its default) | `true` (root build does not add `excludeTags("gravitino-docker-test")`) | +| `maxParallelForks` / `junit.jupiter.execution.parallel.enabled` | default | `1` / `false` | +| Docker precondition (`doFirst`) | none | for `mysql`/`postgresql`: fail unless `rootProject.extra["dockerTest"] == true` | +| JUnit XML | `core/build/test-results//` | same | +| HTML report | `build/reports/tests/core//` | same | +| JaCoCo exec | `core/build/jacoco/.exec` | same | +| Up-to-date inputs | `coreTestSuite=unit`, `coreTestBackend=none`, `coreTestIncludesDockerTaggedTests=false` | `coreTestSuite=`, `coreTestBackend=`, `coreTestIncludesDockerTaggedTests=true` | + +Adding a backend is one map entry plus one `registerCoreTestTask(...)` call plus one nested +annotation in `CoreBackend`; the filter code needs no change. + +The lane JaCoCo files feed `validateCoreSuiteCoverage` and `jacocoTestReport` when +`-PcoreSuiteCoverage=true`, which is how the CI `coverage` job merges the four lanes. + +### A.3 `coreTestLaneOf` task and `CoreTestLaneOf` tool + +Gradle side: + +```kotlin +tasks.register("coreTestLaneOf") { + group = "verification" + dependsOn(tasks.named("testClasses")) + classpath = sourceSets["test"].runtimeClasspath + mainClass.set("org.apache.gravitino.storage.relational.CoreTestLaneOf") + doFirst { + val className = project.findProperty("className") as? String + ?: throw GradleException("Usage: ./gradlew :core:coreTestLaneOf -PclassName=") + args(className) + } +} +``` + +Java side, `core/src/test/java/org/apache/gravitino/storage/relational/CoreTestLaneOf.java`: + +```java +public final class CoreTestLaneOf { + public static void main(String[] args) // exactly one arg: a fully qualified class name +} +``` + +Behaviour: + +- `args.length != 1` or class not on the test classpath: message to stderr, exit code 1. +- Otherwise prints ` tags: [...]` followed by exactly one of: + - ` runs in: coreH2Test, coreMySQLTest, corePostgreSQLTest` (subset, in that order), + - ` carries gravitino-docker-test but no backend tag - it will NOT run in ANY lane. Add ...`, + - ` carries no backend tag - runs in coreUnitTest.` +- Exit code 0 in all three printed cases; the orphan case is a warning, not a failure, so it can + be used interactively without special-casing. + +It runs only `testClasses` (compilation), never a `Test` task. + +### A.4 `dev/ci/core_test_identity.py` + +Two subcommands remain (the unwired `compare-legacy` subcommand was removed on this PR): + +``` +core_test_identity.py manifest --lane {unit,h2,mysql,postgresql} --results

--output +core_test_identity.py reconcile --manifests --output +``` + +Unit tests: `dev/ci/tests/test_core_test_identity.py`, run by the `core-test-contract` job +before reconcile. + +--- + +## Appendix B: Design Sketch + +### B.1 Discovery-time versus execution-time filtering + +JUnit Platform runs a test task in two phases. **Discovery** builds a `TestPlan`: the Jupiter +engine scans the class directories, creates a `ClassTestDescriptor` per test class, and attaches +each descriptor's tags. `PostDiscoveryFilter`s then prune descriptors from that plan. **Execution** +walks the surviving plan, evaluates `ExecutionCondition`s, and runs (or skips) each node. + +Gradle's `useJUnitPlatform { includeTags(...) / excludeTags(...) }` is compiled into a +`TagFilter`, which is a `PostDiscoveryFilter`. A descriptor excluded by it is removed from the plan +before execution starts. Gradle's XML reporter only sees the executed plan, so an excluded class +contributes no `` and no `` elements at all. + +An `ExecutionCondition` (what the reverted `BackendLaneCondition` was) runs at execution time. A +disabled class is still in the plan; Jupiter reports it as skipped, and Gradle writes a +`` for each of its methods. That is a *trace*, and the +reconcile invariant in B.3 tolerates no trace. + +The shipped design therefore uses tags only. The `@CoreBackend.*` annotations exist solely so that +contributors do not type tag strings; at the JUnit level they are indistinguishable from writing +`@Tag("gravitino-core-h2-test")` directly. + +### B.2 How JUnit resolves the tags + +Jupiter collects a class's tags with +`AnnotationSupport.findRepeatableAnnotations(clazz, Tag.class)`. That lookup: + +1. Reads the class's directly present annotations. +2. Follows `@Inherited` annotations up the superclass chain. +3. For each annotation found, recursively inspects the annotation *type's* own meta-annotations, + so a `@CoreBackend.All` on the class yields the three `@Tag` meta-annotations declared on + `CoreBackend.All`. +4. Unwraps the `@Tags` container so repeated `@Tag`s are returned individually. + +Two consequences shape the API: + +- Tags come from an annotation type's fixed meta-annotations, never from an attribute value on + the usage site. This is why a parameterized `backends = {...}` attribute cannot participate in + standard tag filtering and would need a custom filter. +- Only annotations reachable through this reflective walk count. Any annotation whose meaning + depends on code running (an `@ExtendWith` extension, a condition) is invisible to the tag + filter. `TestCoreDatabaseLaneAnnotations.testNoneOfThemCarryAnExecutionTimeHook` enforces that + the four annotation types declare nothing outside `{Documented, Inherited, Retention, Target, + Tag, Tags}`, so the annotations cannot acquire execution-time behaviour without breaking the + test. + +`CoreTestLaneOf` calls exactly this `findRepeatableAnnotations` method and then applies the same +membership rule the Gradle filter applies (`tags ∩ {h2, mysql, postgresql}`), which is why its +answer is authoritative without running a lane. `TestCoreDatabaseLaneAnnotations.tagsOf` uses the +same call, so the regression guard, the local tool, and the engine share one lookup. + +### B.3 How `core_test_identity.py` validates lane membership + +**`manifest --lane L --results DIR`** parses every `TEST-*.xml` under `DIR` and, for each +``: + +1. Extracts backend markers from `C` (via `STATS_BACKEND_CLASS_RE`, matching + `TestJdbcPartitionStatisticStorageIT$Test`) and from `N` (via + `TEST_TEMPLATE_BACKEND_RE`, matching `[ Backend]` as emitted by + `BackendTestExtension`, and `BACKEND_TOKEN_RE`, matching a standalone `h2`/`mysql`/`postgresql` + token as emitted by parameterized `storageProvider()` names). +2. Fails closed (`ManifestError`, exit 1) if `L == unit` and any marker is present, or if `L` is + a database lane and any marker other than `L` is present ("foreign backend marker"). +3. Normalizes the identity: nested stats class names collapse to `...$BackendTest`, backend + tokens collapse to `BACKEND`, `[BACKEND Backend]`, and trailing `[n]` invocation indices + collapse to `[INDEX]`. +4. Counts the normalized `(classname, name)` pair in a multiset, counts status + (`passed`/`skipped`/`failures`/`errors`), and sums `time`. + +It also fails closed on missing XML, zero testcases, any failure or error, and malformed +durations. The output JSON carries the identity list, counts, and a SHA-256 `identity_digest`. + +Note step 2 does not look at status: a `` testcase is still a testcase. This is the +precise reason execution-time skipping is incompatible with the design. Under the reverted +`ExecutionCondition` scheme, `TestJdbcPartitionStatisticStorageIT$MySQLTest` appeared in the H2 +lane's XML as skipped, step 2 saw a `mysql` marker in the `h2` lane, and the manifest step +failed. Even a class with no recognizable markers would have broken reconcile, since a skipped +entry present in one lane but absent from another changes the multiset. + +**`reconcile --manifests unit h2 mysql postgresql`** loads exactly four manifests (one per lane, +no duplicates, no missing), re-validates each (schema, counts, digest, no failures), then asserts: + +- `counters[mysql] == counters[h2]` and `counters[postgresql] == counters[h2]` as multisets, + reporting up to five missing/extra identities on mismatch. +- `counters[unit] & counters[h2]` is empty (unit and database identities are disjoint). + +It writes `summary.json` with `database_identities_equal`, `unit_database_disjoint`, per-lane +summaries, and combined counts. Any violation exits 1 and fails the `core-test-contract` job. + +### B.4 End-to-end flow in CI + +``` +changes ──► build (matrix: core-unit | core-h2 | core-mysql | core-postgresql | ...) + │ each core-* shard: + │ ./gradlew :core: ... (tag filter prunes at discovery) + │ core_test_identity.py manifest --lane L (fails on foreign markers) + │ validate evidence files exist and are non-empty + │ upload core--test-evidence + ▼ + core-test-contract (needs: [changes, build]) + │ unittest dev/ci/tests/test_core_test_identity.py + │ download the four evidence artifacts + │ core_test_identity.py reconcile --manifests unit h2 mysql postgresql + │ upload summary.json as core-test-contract + ▼ + coverage (needs core-test-contract; merges the four JaCoCo .exec files) +``` + +`dev/ci/test-shards.sh` maps `build/core-` to `:core:` and emits `-x :core:test` +for the `others` shard so the unsplit task never runs in CI. + +### B.5 Invariants, in one place + +1. A class is in lane `L` iff its `findRepeatableAnnotations(Tag)` set contains `L`'s tag + (backend lanes) or contains none of the three backend tags and not `gravitino-docker-test` + (unit lane). +2. Every `@CoreBackend.*` type declares only `Tag`/`Tags` plus the four standard + meta-annotations. (Guarded by `TestCoreDatabaseLaneAnnotations`.) +3. A lane's JUnit XML contains testcases only for classes in that lane. (Follows from 1 and 2 + via discovery-time pruning; checked by `manifest`'s foreign-marker rule.) +4. The h2, mysql, and postgresql lanes yield identical normalized identity multisets, and the + unit lane is disjoint from them. (Checked by `reconcile`.) +5. `CoreBackend.*_TAG == coreBackendTestTags[*]`. (Not machine-checked; documented on both + sides.) + +--- + +## Appendix C: How to run core tests locally + +This is the contributor-facing walkthrough; `docs/how-to-test.md` covers the repo-wide +`./gradlew test` task and does not mention `core` specifically, so it was left unchanged and this +appendix is the source of truth for the lanes instead. + +### Running a lane + +```bash +./gradlew :core:coreUnitTest # no database, no Docker - the default +./gradlew :core:coreH2Test # H2-backed tests, no Docker +./gradlew :core:coreMySQLTest -PskipDockerTests=false # MySQL-backed, needs Docker +./gradlew :core:corePostgreSQLTest -PskipDockerTests=false # PostgreSQL-backed, needs Docker +``` + +`coreMySQLTest`/`corePostgreSQLTest` fail fast with a clear `GradleException` if Docker isn't +running and `-PskipDockerTests=false` wasn't passed - they won't silently no-op. `coreH2Test` needs +neither Docker nor that flag. Each lane runs its `Test` task sequentially +(`maxParallelForks = 1`) because database tests mutate process-wide state. + +Do **not** run `./gradlew :core:test` - it is deprecated in place (Q4/A.2): it still works, but +warns and does not correspond to any of the four lanes above or any CI shard. + +### Tagging a new test class + +Pick the annotation that matches where the class needs to run, from +`org.apache.gravitino.storage.relational.CoreBackend`: + +```java +@CoreBackend.All // against all three backends - the default choice +public abstract class MyMultiBackendTest { ... } + +@CoreBackend.H2 // only against H2 - a genuinely H2-only test +public class MyH2OnlyTest { ... } +``` + +No annotation at all means the class is a plain unit test and runs only in `coreUnitTest`. + +**Use `@CoreBackend.All` unless the class is genuinely single-backend.** Stacking a subset (e.g. +`@CoreBackend.H2 @CoreBackend.MySQL`) compiles and each lane it names runs the class, but CI's +`reconcile` step then requires a *normalized sibling* in every backend lane it omits (Appendix +A.1/A.4, `CoreBackend`'s Javadoc) - today the only shape that satisfies that is one `@Nested` +class per backend under a shared outer class, following +`TestJdbcPartitionStatisticStorageIT`'s `H2Test`/`MySQLTest`/`PostgreSQLTest` pattern exactly +(`core_test_identity.py`'s normalization is hard-coded to that one outer class name - see B.3). A +standalone partial-backend class without that sibling structure passes locally in the lanes it +runs in and then fails `reconcile` in CI with "Database identity mismatch". If in doubt, use +`@CoreBackend.All`. See A.1's usage-pattern table for the three concrete shapes already in the +codebase (parameter-provider, `@TestTemplate`, and one `@Nested` class per backend). + +### Checking where a class lands, before running anything + +```bash +./gradlew :core:coreTestLaneOf -PclassName=org.apache.gravitino.storage.relational.TestJDBCBackend +``` + +Compiles test sources (nothing else) and prints the class's tags and the lane(s) it runs in, or an +explicit warning if it carries `gravitino-docker-test` with no `@CoreBackend.*` annotation - the +one case that silently drops a class out of every lane (Q6). Run this after adding or changing an +annotation on a database-touching test class, before pushing. diff --git a/dev/ci/core_test_identity.py b/dev/ci/core_test_identity.py new file mode 100644 index 00000000000..c11c22cfe16 --- /dev/null +++ b/dev/ci/core_test_identity.py @@ -0,0 +1,509 @@ +#!/usr/bin/env python3 +# 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. + +"""Build and reconcile normalized core-test identity manifests. + +Gradle writes one JUnit XML directory per Test task. This tool turns those +reports into stable, backend-neutral identity multisets so the H2, MySQL, and +PostgreSQL lanes can prove that they exercised the same test contract. It also +records status counts and elapsed test time for CI artifacts. +""" + +import argparse +from collections import Counter +from decimal import Decimal, InvalidOperation +import hashlib +import json +import math +from pathlib import Path +import re +import sys +import xml.etree.ElementTree as ET + + +SCHEMA_VERSION = 1 +LANES = ("unit", "h2", "mysql", "postgresql") +DATABASE_LANES = LANES[1:] +MANIFEST_LANES = LANES +STATUS_KEYS = ("passed", "skipped", "failures", "errors") + +BACKEND_NAME_PATTERN = r"h2|mysql|postgresql" +TEST_TEMPLATE_BACKEND_RE = re.compile( + rf"\[(?P{BACKEND_NAME_PATTERN})\s+Backend\]", re.IGNORECASE +) +STATS_BACKEND_CLASS_RE = re.compile( + rf"(?PTestJdbcPartitionStatisticStorageIT)\$" + rf"(?P{BACKEND_NAME_PATTERN})Test(?=$|\$)", + re.IGNORECASE, +) +BACKEND_TOKEN_RE = re.compile( + rf"(?{BACKEND_NAME_PATTERN})(?![A-Za-z0-9])", + re.IGNORECASE, +) +TRAILING_INVOCATION_INDEX_RE = re.compile( + r"\[(?:#)?\d+\](?=(?:\s*\[BACKEND Backend\])?\s*$)", re.IGNORECASE +) + + +class ManifestError(ValueError): + """Raised when test results cannot form a trustworthy manifest.""" + + +def _local_name(tag): + """Return an XML element name without its optional namespace.""" + return tag.rsplit("}", 1)[-1] + + +def _canonical_backend(value): + """Return the canonical spelling of a recognized backend.""" + return value.lower() + + +def _classname_backend_markers(value): + """Find backend markers in the backend-specific nested stats classes.""" + return { + _canonical_backend(match.group("backend")) + for match in STATS_BACKEND_CLASS_RE.finditer(value) + } + + +def _test_name_backend_markers(value): + """Find structured backend markers in a testcase name.""" + markers = { + _canonical_backend(match.group("backend")) + for match in TEST_TEMPLATE_BACKEND_RE.finditer(value) + } + markers.update( + _canonical_backend(match.group("backend")) + for match in BACKEND_TOKEN_RE.finditer(value) + ) + return markers + + +def _normalize_classname(value): + """Normalize backend-specific nested stats class names.""" + return STATS_BACKEND_CLASS_RE.sub( + lambda match: f"{match.group('prefix')}$BackendTest", value + ) + + +def _normalize_test_name(value): + """Normalize backend markers and trailing parameterized indices.""" + normalized = TEST_TEMPLATE_BACKEND_RE.sub("[BACKEND Backend]", value) + normalized = BACKEND_TOKEN_RE.sub("BACKEND", normalized) + return TRAILING_INVOCATION_INDEX_RE.sub("[INDEX]", normalized) + + +def normalize_identity(lane, classname, test_name): + """Validate lane markers and return a normalized test identity pair.""" + classname = (classname or "").strip() + test_name = (test_name or "").strip() + if not classname or not test_name: + raise ManifestError("Every must have non-empty classname and name attributes") + + markers = _classname_backend_markers(classname) | _test_name_backend_markers(test_name) + if lane == "unit" and markers: + raise ManifestError( + "Unit test result contains an explicit backend marker " + f"{sorted(markers)}: {classname}::{test_name}" + ) + if lane in DATABASE_LANES: + foreign_markers = markers - {lane} + if foreign_markers: + raise ManifestError( + f"{lane} test result contains foreign backend marker(s) " + f"{sorted(foreign_markers)}: {classname}::{test_name}" + ) + + return ( + _normalize_classname(classname), + _normalize_test_name(test_name), + ) + + +def _testcase_status(testcase): + """Classify one JUnit testcase element.""" + child_tags = {_local_name(child.tag) for child in testcase} + if "failure" in child_tags: + return "failures" + if "error" in child_tags: + return "errors" + if "skipped" in child_tags: + return "skipped" + return "passed" + + +def _testcase_duration(testcase, source_file): + """Parse one JUnit testcase duration as a non-negative Decimal.""" + value = testcase.get("time", "0") + try: + duration = Decimal(value) + except InvalidOperation as error: + raise ManifestError( + f"Invalid testcase duration {value!r} in {source_file}" + ) from error + if not duration.is_finite() or duration < 0: + raise ManifestError(f"Invalid testcase duration {value!r} in {source_file}") + return duration + + +def _identities_as_json(identities): + """Convert an identity Counter to deterministic JSON records.""" + return [ + {"classname": classname, "name": name, "count": count} + for (classname, name), count in sorted(identities.items()) + ] + + +def _identities_from_json(manifest, source_file): + """Validate and restore an identity Counter from a manifest.""" + records = manifest.get("identities") + if not isinstance(records, list): + raise ManifestError(f"Manifest {source_file} has no identities list") + + identities = Counter() + for record in records: + if not isinstance(record, dict): + raise ManifestError(f"Manifest {source_file} has an invalid identity record") + classname = record.get("classname") + name = record.get("name") + count = record.get("count") + if not isinstance(classname, str) or not classname: + raise ManifestError(f"Manifest {source_file} has an invalid classname") + if not isinstance(name, str) or not name: + raise ManifestError(f"Manifest {source_file} has an invalid testcase name") + if not isinstance(count, int) or isinstance(count, bool) or count <= 0: + raise ManifestError(f"Manifest {source_file} has an invalid identity count") + identity = (classname, name) + if identity in identities: + raise ManifestError(f"Manifest {source_file} repeats identity {identity}") + identities[identity] = count + return identities + + +def _identity_digest(identities): + """Return a stable digest of an identity multiset.""" + digest = hashlib.sha256() + for (classname, name), count in sorted(identities.items()): + digest.update(classname.encode("utf-8")) + digest.update(b"\0") + digest.update(name.encode("utf-8")) + digest.update(b"\0") + digest.update(str(count).encode("ascii")) + digest.update(b"\n") + return digest.hexdigest() + + +def build_manifest(lane, results_directory): + """Parse Gradle JUnit XML reports and return a normalized manifest.""" + if lane not in MANIFEST_LANES: + raise ManifestError( + f"Unknown lane {lane!r}; expected one of {', '.join(MANIFEST_LANES)}" + ) + + results_directory = Path(results_directory) + if not results_directory.is_dir(): + raise ManifestError(f"Results directory does not exist: {results_directory}") + + xml_files = sorted(results_directory.rglob("TEST-*.xml")) + if not xml_files: + raise ManifestError(f"No TEST-*.xml files found under {results_directory}") + + identities = Counter() + statuses = Counter({key: 0 for key in STATUS_KEYS}) + duration = Decimal("0") + source_files = [] + + for xml_file in xml_files: + relative_source = xml_file.relative_to(results_directory).as_posix() + source_files.append(relative_source) + try: + root = ET.parse(xml_file).getroot() + except (ET.ParseError, OSError) as error: + raise ManifestError(f"Could not parse {xml_file}: {error}") from error + + testcases = ( + element + for element in root.iter() + if _local_name(element.tag) == "testcase" + ) + for testcase in testcases: + identity = normalize_identity( + lane, testcase.get("classname"), testcase.get("name") + ) + identities[identity] += 1 + statuses[_testcase_status(testcase)] += 1 + duration += _testcase_duration(testcase, relative_source) + + test_count = sum(identities.values()) + if test_count == 0: + raise ManifestError(f"No entries found under {results_directory}") + if statuses["failures"] or statuses["errors"]: + raise ManifestError( + f"Lane {lane} contains {statuses['failures']} failure(s) and " + f"{statuses['errors']} error(s)" + ) + + return { + "schema_version": SCHEMA_VERSION, + "lane": lane, + "successful": True, + "test_count": test_count, + "unique_identity_count": len(identities), + "duration_seconds": float(duration), + "status_counts": {key: statuses[key] for key in STATUS_KEYS}, + "source_files": source_files, + "identity_digest": _identity_digest(identities), + "identities": _identities_as_json(identities), + } + + +def write_json(document, output_file): + """Write one deterministic JSON document.""" + output_file = Path(output_file) + output_file.parent.mkdir(parents=True, exist_ok=True) + with output_file.open("w", encoding="utf-8") as output: + json.dump(document, output, indent=2, sort_keys=True) + output.write("\n") + + +def _load_manifest(manifest_file): + """Load and validate the common fields of one manifest.""" + manifest_file = Path(manifest_file) + try: + with manifest_file.open(encoding="utf-8") as source: + manifest = json.load(source) + except (OSError, json.JSONDecodeError) as error: + raise ManifestError(f"Could not read manifest {manifest_file}: {error}") from error + + if not isinstance(manifest, dict): + raise ManifestError(f"Manifest {manifest_file} must contain a JSON object") + if manifest.get("schema_version") != SCHEMA_VERSION: + raise ManifestError(f"Manifest {manifest_file} has an unsupported schema version") + lane = manifest.get("lane") + if lane not in MANIFEST_LANES: + raise ManifestError(f"Manifest {manifest_file} has invalid lane {lane!r}") + if manifest.get("successful") is not True: + raise ManifestError(f"Manifest {manifest_file} is not successful") + + identities = _identities_from_json(manifest, manifest_file) + test_count = manifest.get("test_count") + if not isinstance(test_count, int) or isinstance(test_count, bool) or test_count <= 0: + raise ManifestError(f"Manifest {manifest_file} has invalid test_count") + if sum(identities.values()) != test_count: + raise ManifestError(f"Manifest {manifest_file} identity counts do not match test_count") + + unique_identity_count = manifest.get("unique_identity_count") + if ( + not isinstance(unique_identity_count, int) + or isinstance(unique_identity_count, bool) + or unique_identity_count <= 0 + or unique_identity_count != len(identities) + ): + raise ManifestError( + f"Manifest {manifest_file} has invalid unique_identity_count" + ) + + statuses = manifest.get("status_counts") + if not isinstance(statuses, dict): + raise ManifestError(f"Manifest {manifest_file} has invalid status_counts") + for key in STATUS_KEYS: + value = statuses.get(key) + if not isinstance(value, int) or isinstance(value, bool) or value < 0: + raise ManifestError(f"Manifest {manifest_file} has invalid status {key}") + if sum(statuses[key] for key in STATUS_KEYS) != test_count: + raise ManifestError(f"Manifest {manifest_file} statuses do not match test_count") + if statuses["failures"] or statuses["errors"]: + raise ManifestError(f"Manifest {manifest_file} contains failed tests") + + duration = manifest.get("duration_seconds") + if ( + not isinstance(duration, (int, float)) + or isinstance(duration, bool) + or not math.isfinite(duration) + or duration < 0 + ): + raise ManifestError(f"Manifest {manifest_file} has invalid duration_seconds") + + source_files = manifest.get("source_files") + if ( + not isinstance(source_files, list) + or not source_files + or any(not isinstance(source, str) or not source for source in source_files) + or len(set(source_files)) != len(source_files) + ): + raise ManifestError(f"Manifest {manifest_file} has invalid source_files") + + identity_digest = manifest.get("identity_digest") + if identity_digest != _identity_digest(identities): + raise ManifestError(f"Manifest {manifest_file} has invalid identity_digest") + + return lane, manifest, identities + + +def _format_identity_difference(reference, actual): + """Format a bounded explanation of a Counter mismatch.""" + differences = [] + for label, values in (("missing", reference - actual), ("extra", actual - reference)): + for (classname, name), count in sorted(values.items())[:5]: + differences.append(f"{label} {count} x {classname}::{name}") + return "; ".join(differences) + + +def _format_identities(identities): + """Format a bounded identity Counter for an error message.""" + return "; ".join( + f"{count} x {classname}::{name}" + for (classname, name), count in sorted(identities.items())[:5] + ) + + +def _load_split_manifests(manifest_files): + """Load exactly one trustworthy manifest for each split lane.""" + manifest_files = [Path(path) for path in manifest_files] + if len(manifest_files) != len(LANES): + raise ManifestError(f"Expected exactly {len(LANES)} manifests, got {len(manifest_files)}") + + by_lane = {} + counters = {} + for manifest_file in manifest_files: + lane, manifest, identities = _load_manifest(manifest_file) + if lane not in LANES: + raise ManifestError( + f"Expected a split-lane manifest, got lane {lane} from {manifest_file}" + ) + if lane in by_lane: + raise ManifestError(f"Received more than one manifest for lane {lane}") + by_lane[lane] = manifest + counters[lane] = identities + + missing_lanes = set(LANES) - set(by_lane) + if missing_lanes: + raise ManifestError(f"Missing manifest lane(s): {', '.join(sorted(missing_lanes))}") + return by_lane, counters + + +def _lane_summary(manifest, identities): + """Return the evidence retained for one successfully loaded lane.""" + return { + "test_count": manifest["test_count"], + "unique_identity_count": manifest["unique_identity_count"], + "duration_seconds": manifest["duration_seconds"], + "status_counts": manifest["status_counts"], + "source_file_count": len(manifest["source_files"]), + "source_files": manifest["source_files"], + "identity_digest": _identity_digest(identities), + } + + +def reconcile_manifests(manifest_files): + """Require four lanes and reconcile the three database identity multisets.""" + by_lane, counters = _load_split_manifests(manifest_files) + + reference = counters["h2"] + for lane in DATABASE_LANES[1:]: + if counters[lane] != reference: + difference = _format_identity_difference(reference, counters[lane]) + raise ManifestError( + f"Database identity mismatch between h2 and {lane}: {difference}" + ) + + unit_database_overlap = counters["unit"] & reference + if unit_database_overlap: + raise ManifestError( + "Unit/database identity overlap: " + f"{_format_identities(unit_database_overlap)}" + ) + + combined_statuses = { + key: sum(by_lane[lane]["status_counts"][key] for lane in LANES) + for key in STATUS_KEYS + } + lane_summaries = { + lane: _lane_summary(by_lane[lane], counters[lane]) for lane in LANES + } + + return { + "schema_version": SCHEMA_VERSION, + "successful": True, + "database_identities_equal": True, + "unit_database_disjoint": True, + "database_test_count_per_lane": sum(reference.values()), + "database_unique_identity_count": len(reference), + "database_identity_digest": _identity_digest(reference), + "combined_test_count": sum(by_lane[lane]["test_count"] for lane in LANES), + "combined_duration_seconds": float( + sum( + ( + Decimal(str(by_lane[lane]["duration_seconds"])) + for lane in LANES + ), + Decimal("0"), + ) + ), + "combined_status_counts": combined_statuses, + "lanes": lane_summaries, + } + + +def _create_argument_parser(): + """Create the command-line parser.""" + parser = argparse.ArgumentParser(description=__doc__) + subparsers = parser.add_subparsers(dest="command", required=True) + + manifest_parser = subparsers.add_parser( + "manifest", help="Create one normalized manifest from Gradle JUnit XML" + ) + manifest_parser.add_argument("--lane", required=True, choices=MANIFEST_LANES) + manifest_parser.add_argument( + "--results", required=True, type=Path, help="Directory containing TEST-*.xml" + ) + manifest_parser.add_argument( + "--output", required=True, type=Path, help="JSON manifest to write" + ) + + reconcile_parser = subparsers.add_parser( + "reconcile", help="Reconcile unit and database lane manifests" + ) + reconcile_parser.add_argument( + "--manifests", required=True, nargs="+", type=Path, help="The four lane manifests" + ) + reconcile_parser.add_argument( + "--output", required=True, type=Path, help="Combined JSON summary to write" + ) + return parser + + +def main(argv=None): + """Run the command-line interface.""" + parser = _create_argument_parser() + args = parser.parse_args(argv) + try: + if args.command == "manifest": + document = build_manifest(args.lane, args.results) + else: + document = reconcile_manifests(args.manifests) + write_json(document, args.output) + except ManifestError as error: + print(f"error: {error}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/dev/ci/test-shards.sh b/dev/ci/test-shards.sh new file mode 100755 index 00000000000..0c868c75a69 --- /dev/null +++ b/dev/ci/test-shards.sh @@ -0,0 +1,177 @@ +#!/usr/bin/env bash +# +# 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. +# +# Single source of truth for how CI test suites are split into parallel shards. +# +# Usage: +# dev/ci/test-shards.sh --list Print the suite's shard names as a JSON array. +# dev/ci/test-shards.sh Print the Gradle task arguments of a shard, one per line. +# +# Suites: +# build Unit tests run by .github/workflows/build.yml. +# backend-it Integration tests run by .github/workflows/backend-integration-test.yml. +# +# Every suite ends with a catch-all `others` shard that excludes the test tasks owned by its named +# shards, so a new module is always tested by `others` until it is moved to a named shard. Build's +# core lanes map directly to dedicated tasks; project-based shards remain in the lists below. +# To rebalance, update the task mappings or project lists below; the workflows need no change. + +set -euo pipefail + +# ---- build suite ------------------------------------------------------------------------------- +# Core separates its unit and database contracts into explicit tasks. Database lanes remain +# sequential internally; CI gives each lane its own shard so their results and coverage inputs are +# independently inspectable. +BUILD_CORE_SHARDS=( + core-unit + core-h2 + core-mysql + core-postgresql +) + +# Projects with `gravitino-docker-test` tests. Gradle runs them one by one under the shared test +# environment lock, so they are kept away from the parallel unit tests in `others`. +BUILD_DOCKER=( + :authorizations:authorization-chain + :authorizations:authorization-ranger + :catalogs:catalog-fileset + :catalogs:catalog-glue + :catalogs:catalog-hive + :catalogs:catalog-jdbc-doris + :catalogs:catalog-jdbc-mysql + :catalogs:catalog-jdbc-postgresql + :catalogs:catalog-jdbc-starrocks + :catalogs:catalog-kafka + :catalogs:catalog-lakehouse-hudi + :catalogs:catalog-lakehouse-iceberg + :catalogs:catalog-lakehouse-paimon + :catalogs:hive-metastore-common + :clients:client-java + :clients:filesystem-hadoop3 + :flink-connector:flink-common + :iceberg:iceberg-rest-server + :maintenance:jobs + :maintenance:optimizer + :plugins:idp-basic + :spark-connector:spark-3.5 +) + +# ---- backend-it suite -------------------------------------------------------------------------- +# client-java, catalog-fileset and filesystem-hadoop3 stay in `others`: together with the remaining +# modules they still finish before the slowest shard, and one less shard saves a job per backend. +BACKEND_IT_HIVE=( + :catalogs:catalog-hive + :catalogs:catalog-glue + :catalogs:catalog-lakehouse-hudi +) + +BACKEND_IT_LAKEHOUSE=( + :iceberg:iceberg-rest-server + :catalogs:catalog-lakehouse-iceberg + :catalogs:catalog-lakehouse-paimon + :lance:lance-rest-server +) + +usage() { + sed -n '/^# Usage:/,/^# To rebalance/p' "$0" | sed 's/^# \{0,1\}//' >&2 + exit 1 +} + +# Prints the shard names of a suite, in matrix order. +shards_of() { + case "$1" in + build) echo "${BUILD_CORE_SHARDS[*]} docker others" ;; + backend-it) echo "hive lakehouse others" ;; + *) echo "Unknown suite: $1" >&2; usage ;; + esac +} + +# Prints the variable name holding the projects of a named shard. +projects_var() { + case "$1/$2" in + build/docker) echo BUILD_DOCKER ;; + backend-it/hive) echo BACKEND_IT_HIVE ;; + backend-it/lakehouse) echo BACKEND_IT_LAKEHOUSE ;; + *) echo "Unknown shard '$2' for suite '$1'" >&2; usage ;; + esac +} + +# Prints `:test` for every project in the array named by $1. +print_test_tasks() { + local project + eval 'for project in "${'"$1"'[@]}"; do echo "${project}:test"; done' +} + +# `others` runs the suite's root task with every named shard's test task excluded. +print_others() { + local suite="$1" root_task="$2" shard task + echo "${root_task}" + + if [ "${suite}" = "build" ]; then + # The explicit core shards replace the legacy task. Keep Docker-tagged projects in their own + # shard as before so `others` cannot execute either group a second time through root `build`. + printf -- '-x\n:core:test\n' + for task in $(print_test_tasks BUILD_DOCKER); do + printf -- '-x\n%s\n' "${task}" + done + return + fi + + for shard in $(shards_of "${suite}"); do + [ "${shard}" = "others" ] && continue + for task in $(print_test_tasks "$(projects_var "${suite}" "${shard}")"); do + printf -- '-x\n%s\n' "${task}" + done + done +} + +[ $# -eq 2 ] || usage +suite="$1" +shard="$2" +shard_names="$(shards_of "${suite}")" + +if [ "${shard}" = "--list" ]; then + printf '[' + sep="" + for name in ${shard_names}; do + printf '%s"%s"' "${sep}" "${name}" + sep="," + done + printf ']\n' + exit 0 +fi + +if [ "${shard}" = "others" ]; then + case "${suite}" in + build) print_others build build ;; + backend-it) print_others backend-it test ;; + *) echo "Unknown suite: ${suite}" >&2; usage ;; + esac +else + case "${suite}/${shard}" in + build/core-unit) echo :core:coreUnitTest ;; + build/core-h2) echo :core:coreH2Test ;; + build/core-mysql) echo :core:coreMySQLTest ;; + build/core-postgresql) echo :core:corePostgreSQLTest ;; + *) + projects="$(projects_var "${suite}" "${shard}")" + print_test_tasks "${projects}" + ;; + esac +fi diff --git a/dev/ci/tests/fixtures/core_test_identity/h2/TEST-backend.xml b/dev/ci/tests/fixtures/core_test_identity/h2/TEST-backend.xml new file mode 100644 index 00000000000..7b6bfdd4bbc --- /dev/null +++ b/dev/ci/tests/fixtures/core_test_identity/h2/TEST-backend.xml @@ -0,0 +1,27 @@ + + + + + + + + + + diff --git a/dev/ci/tests/fixtures/core_test_identity/mysql/TEST-backend.xml b/dev/ci/tests/fixtures/core_test_identity/mysql/TEST-backend.xml new file mode 100644 index 00000000000..47a84c52d1e --- /dev/null +++ b/dev/ci/tests/fixtures/core_test_identity/mysql/TEST-backend.xml @@ -0,0 +1,27 @@ + + + + + + + + + + diff --git a/dev/ci/tests/fixtures/core_test_identity/postgresql/TEST-backend.xml b/dev/ci/tests/fixtures/core_test_identity/postgresql/TEST-backend.xml new file mode 100644 index 00000000000..dd1aad76ca3 --- /dev/null +++ b/dev/ci/tests/fixtures/core_test_identity/postgresql/TEST-backend.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + diff --git a/dev/ci/tests/fixtures/core_test_identity/unit/TEST-unit.xml b/dev/ci/tests/fixtures/core_test_identity/unit/TEST-unit.xml new file mode 100644 index 00000000000..7c169da3478 --- /dev/null +++ b/dev/ci/tests/fixtures/core_test_identity/unit/TEST-unit.xml @@ -0,0 +1,26 @@ + + + + + + + + + diff --git a/dev/ci/tests/test_core_test_identity.py b/dev/ci/tests/test_core_test_identity.py new file mode 100644 index 00000000000..23959b4bda5 --- /dev/null +++ b/dev/ci/tests/test_core_test_identity.py @@ -0,0 +1,331 @@ +# 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. + +import copy +from collections import Counter +import importlib.util +import json +from pathlib import Path +import tempfile +import unittest + + +SCRIPT_PATH = Path(__file__).parents[1] / "core_test_identity.py" +SPEC = importlib.util.spec_from_file_location("core_test_identity", SCRIPT_PATH) +core_test_identity = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(core_test_identity) + +FIXTURES = Path(__file__).parent / "fixtures" / "core_test_identity" + + +def identity_counter(manifest): + """Return the manifest identities in their natural Counter form.""" + return Counter( + { + (record["classname"], record["name"]): record["count"] + for record in manifest["identities"] + } + ) + + +def write_report(directory, testcases): + """Write a minimal Gradle-compatible JUnit XML report.""" + directory.mkdir(parents=True, exist_ok=True) + (directory / "TEST-fixture.xml").write_text( + "\n" + f"{testcases}\n", + encoding="utf-8", + ) + + +class TestCoreTestIdentity(unittest.TestCase): + def test_manifest_normalizes_database_identities_as_multisets(self): + manifests = { + lane: core_test_identity.build_manifest(lane, FIXTURES / lane) + for lane in core_test_identity.LANES + } + + database_counters = [ + identity_counter(manifests[lane]) + for lane in core_test_identity.DATABASE_LANES + ] + self.assertEqual(database_counters[0], database_counters[1]) + self.assertEqual(database_counters[0], database_counters[2]) + self.assertEqual( + database_counters[0][ + ( + "org.apache.gravitino.stats.storage." + "TestJdbcPartitionStatisticStorageIT$BackendTest", + "writesPartitionStats()[INDEX]", + ) + ], + 2, + ) + self.assertIn( + ( + "org.apache.gravitino.TestCatalogMetaService", + "testCreateCatalog()[BACKEND Backend]", + ), + database_counters[0], + ) + self.assertIn( + ( + "org.apache.gravitino.TestCatalogMetaService", + "testDropCatalog()[BACKEND Backend]", + ), + database_counters[0], + ) + self.assertIn( + ("org.apache.gravitino.BackendTokenTest", "roundTrip[BACKEND]"), + database_counters[0], + ) + self.assertIn( + ( + "org.apache.gravitino.UnmarkedBackendTest", + "unmarkedSharedCase()", + ), + database_counters[0], + ) + + unit = manifests["unit"] + self.assertEqual(unit["test_count"], 3) + self.assertEqual(unit["duration_seconds"], 0.7) + self.assertEqual(unit["status_counts"]["passed"], 2) + self.assertEqual(unit["status_counts"]["skipped"], 1) + self.assertEqual(unit["source_files"], ["TEST-unit.xml"]) + self.assertIn( + ( + "org.apache.gravitino.TestH2ExceptionConverter", + "testH2Converter()", + ), + identity_counter(unit), + ) + self.assertIn( + ( + "org.apache.gravitino.storage.relational.mapper.provider.postgresql." + "TestCatalogMetaPostgreSQLProvider", + "testInsertSql()", + ), + identity_counter(unit), + ) + + def test_lane_validation_rejects_explicit_wrong_backend_markers(self): + cases = ( + ( + "unit", + '', + "Unit test result contains an explicit backend marker", + ), + ( + "h2", + '', + "foreign backend marker", + ), + ( + "postgresql", + '', + "foreign backend marker", + ), + ) + for lane, testcase, message in cases: + with self.subTest(lane=lane), tempfile.TemporaryDirectory() as temp_dir: + results = Path(temp_dir) + write_report(results, testcase) + with self.assertRaisesRegex(core_test_identity.ManifestError, message): + core_test_identity.build_manifest(lane, results) + + def test_manifest_fails_closed_on_missing_or_untrustworthy_results(self): + with tempfile.TemporaryDirectory() as temp_dir: + with self.assertRaisesRegex(core_test_identity.ManifestError, "No TEST-"): + core_test_identity.build_manifest("unit", temp_dir) + + invalid_cases = ( + ("", "No "), + ("", "Could not parse"), + ( + '', + "contains 1 failure", + ), + ( + '', + "and 1 error", + ), + ) + for xml, message in invalid_cases: + with self.subTest(message=message), tempfile.TemporaryDirectory() as temp_dir: + results = Path(temp_dir) + (results / "TEST-invalid.xml").write_text(xml, encoding="utf-8") + with self.assertRaisesRegex(core_test_identity.ManifestError, message): + core_test_identity.build_manifest("unit", results) + + def test_reconcile_emits_combined_timing_and_identity_summary(self): + with tempfile.TemporaryDirectory() as temp_dir: + output_directory = Path(temp_dir) + manifest_files = [] + for lane in core_test_identity.LANES: + manifest = core_test_identity.build_manifest(lane, FIXTURES / lane) + manifest_file = output_directory / f"{lane}.json" + core_test_identity.write_json(manifest, manifest_file) + manifest_files.append(manifest_file) + + summary = core_test_identity.reconcile_manifests(manifest_files) + + self.assertTrue(summary["successful"]) + self.assertTrue(summary["database_identities_equal"]) + self.assertTrue(summary["unit_database_disjoint"]) + self.assertEqual(summary["database_test_count_per_lane"], 6) + self.assertEqual(summary["database_unique_identity_count"], 5) + self.assertEqual(summary["combined_test_count"], 21) + self.assertEqual(summary["combined_duration_seconds"], 7.3) + self.assertEqual(summary["combined_status_counts"]["skipped"], 2) + self.assertEqual(set(summary["lanes"]), set(core_test_identity.LANES)) + self.assertEqual( + summary["lanes"]["h2"]["source_files"], ["TEST-backend.xml"] + ) + + def test_reconcile_requires_exactly_four_matching_lanes(self): + with tempfile.TemporaryDirectory() as temp_dir: + output_directory = Path(temp_dir) + manifests = {} + for lane in core_test_identity.LANES: + manifest = core_test_identity.build_manifest(lane, FIXTURES / lane) + manifest_file = output_directory / f"{lane}.json" + core_test_identity.write_json(manifest, manifest_file) + manifests[lane] = manifest_file + + with self.assertRaisesRegex(core_test_identity.ManifestError, "exactly 4"): + core_test_identity.reconcile_manifests(list(manifests.values())[:3]) + + mismatched = copy.deepcopy( + core_test_identity.build_manifest("mysql", FIXTURES / "mysql") + ) + mismatched["identities"][0]["name"] += "-different" + mismatched["identity_digest"] = core_test_identity._identity_digest( + identity_counter(mismatched) + ) + mismatched_file = output_directory / "mysql-mismatched.json" + core_test_identity.write_json(mismatched, mismatched_file) + with self.assertRaisesRegex( + core_test_identity.ManifestError, "Database identity mismatch" + ): + core_test_identity.reconcile_manifests( + [ + manifests["unit"], + manifests["h2"], + mismatched_file, + manifests["postgresql"], + ] + ) + + with self.assertRaisesRegex( + core_test_identity.ManifestError, "more than one manifest for lane h2" + ): + core_test_identity.reconcile_manifests( + [ + manifests["unit"], + manifests["h2"], + manifests["h2"], + manifests["postgresql"], + ] + ) + + def test_reconcile_rejects_unit_database_overlap(self): + with tempfile.TemporaryDirectory() as temp_dir: + output_directory = Path(temp_dir) + overlapping_results = output_directory / "overlapping-unit-results" + write_report( + overlapping_results, + '', + ) + + manifest_files = [] + for lane in core_test_identity.LANES: + results = ( + overlapping_results if lane == "unit" else FIXTURES / lane + ) + manifest = core_test_identity.build_manifest(lane, results) + manifest_file = output_directory / f"{lane}.json" + core_test_identity.write_json(manifest, manifest_file) + manifest_files.append(manifest_file) + + with self.assertRaisesRegex( + core_test_identity.ManifestError, "Unit/database identity overlap" + ): + core_test_identity.reconcile_manifests(manifest_files) + + def test_reconcile_rejects_tampered_identity_evidence(self): + with tempfile.TemporaryDirectory() as temp_dir: + output_directory = Path(temp_dir) + manifest_files = [] + for lane in core_test_identity.LANES: + manifest = core_test_identity.build_manifest(lane, FIXTURES / lane) + if lane == "h2": + manifest["identity_digest"] = "0" * 64 + manifest_file = output_directory / f"{lane}.json" + core_test_identity.write_json(manifest, manifest_file) + manifest_files.append(manifest_file) + + with self.assertRaisesRegex( + core_test_identity.ManifestError, "invalid identity_digest" + ): + core_test_identity.reconcile_manifests(manifest_files) + + def test_cli_writes_manifest_and_reconciliation_output(self): + with tempfile.TemporaryDirectory() as temp_dir: + output_directory = Path(temp_dir) + manifest_files = [] + for lane in core_test_identity.LANES: + manifest_file = output_directory / f"{lane}.json" + return_code = core_test_identity.main( + [ + "manifest", + "--lane", + lane, + "--results", + str(FIXTURES / lane), + "--output", + str(manifest_file), + ] + ) + self.assertEqual(return_code, 0) + self.assertTrue(manifest_file.is_file()) + manifest_files.append(manifest_file) + + summary_file = output_directory / "summary.json" + return_code = core_test_identity.main( + [ + "reconcile", + "--manifests", + *(str(path) for path in manifest_files), + "--output", + str(summary_file), + ] + ) + self.assertEqual(return_code, 0) + with summary_file.open(encoding="utf-8") as source: + summary = json.load(source) + self.assertTrue(summary["database_identities_equal"]) + + +if __name__ == "__main__": + unittest.main()