From 86c28af622cb1785af478445f0919bca06950bce Mon Sep 17 00:00:00 2001 From: yuqi Date: Wed, 23 Sep 2026 11:16:52 +0800 Subject: [PATCH 1/5] [#13459] improvement(ci): Split build and Backend IT into sharded sub-jobs --- .../backend-integration-test-action.yml | 17 +- .../workflows/backend-integration-test.yml | 51 +++--- .github/workflows/build.yml | 96 ++++++++--- dev/ci/test-shards.sh | 158 ++++++++++++++++++ 4 files changed, 271 insertions(+), 51 deletions(-) create mode 100755 dev/ci/test-shards.sh 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 85a700cb170..1508cd3761a 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,20 @@ jobs: java-version: ${{ matrix.java-version }} backend: ${{ matrix.backend }} test-mode: ${{ matrix.test-mode }} + shard: ${{ matrix.shard }} + + # Single status check that summarizes every backend integration test shard, so branch protection + # does not need to track individual shard names. + BackendIT-result: + runs-on: ubuntu-22.04 + needs: [ changes, BackendIT ] + if: always() + steps: + - name: Check backend integration test results + run: | + results='${{ toJSON(needs.*.result) }}' + echo "${results}" + if echo "${results}" | grep -Eq '"(failure|cancelled)"'; then + echo "At least one backend integration test shard failed or was cancelled." + exit 1 + fi diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1ad49fbac51..d7865afad1e 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 ] - timeout-minutes: 120 + # Shards are defined in dev/ci/test-shards.sh. + shard: ${{ fromJSON(needs.changes.outputs.build_shards) }} + timeout-minutes: 90 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,8 +200,11 @@ jobs: exit 0 fi + shard_args_text="$(dev/ci/test-shards.sh build "${{ matrix.shard }}")" + mapfile -t shard_args <<< "${shard_args_text}" + gradle_args=( - build + "${shard_args[@]}" --max-workers=2 -PskipWeb=true -PskipITs @@ -217,13 +230,50 @@ jobs: ./gradlew "${gradle_args[@]}" - - name: Fetch base branch for coverage diff + - name: Upload coverage data if: github.event_name == 'pull_request' + 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 + catalogs-contrib/**/*.log + catalogs-contrib/**/*.tar + catalogs/**/*.log + catalogs/**/*.tar + + coverage: + runs-on: ubuntu-latest + timeout-minutes: 15 + needs: build + if: github.event_name == 'pull_request' && needs.build.result == 'success' + steps: + - uses: actions/checkout@v4 + + - name: Fetch base branch for coverage diff 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: Generate Coverage Report id: coverage - if: github.event_name == 'pull_request' run: | python3 dev/ci/jacoco_report.py \ --base-ref "${{ github.base_ref }}" \ @@ -234,11 +284,11 @@ 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 @@ -247,19 +297,23 @@ jobs: pr-number.txt - 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 + # Single status check that summarizes every build shard, so branch protection does not need to + # track individual shard names. + build-result: + runs-on: ubuntu-latest + needs: [ changes, compile-check, spark-connector-build, build, coverage ] + if: always() + steps: + - name: Check build results + run: | + results='${{ toJSON(needs.*.result) }}' + echo "${results}" + if echo "${results}" | grep -Eq '"(failure|cancelled)"'; then + echo "At least one build job failed or was cancelled." + exit 1 + fi diff --git a/dev/ci/test-shards.sh b/dev/ci/test-shards.sh new file mode 100755 index 00000000000..a9188a79534 --- /dev/null +++ b/dev/ci/test-shards.sh @@ -0,0 +1,158 @@ +#!/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 projects of all named +# shards, so a new module is always tested by `others` until it is moved to a named shard. +# To rebalance, move a project between the lists below; the workflows need no change. + +set -euo pipefail + +# ---- build suite ------------------------------------------------------------------------------- +# `core` holds the shared test environment lock for its whole run, so it gets its own shard. +BUILD_CORE=( + :core +) + +# 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 -------------------------------------------------------------------------- +BACKEND_IT_HIVE=( + :catalogs:catalog-hive + :catalogs:catalog-glue + :catalogs:catalog-lakehouse-hudi +) + +BACKEND_IT_CLIENT=( + :clients:client-java + :catalogs:catalog-fileset + :clients:filesystem-hadoop3 +) + +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 "core docker others" ;; + backend-it) echo "hive client 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/core) echo BUILD_CORE ;; + build/docker) echo BUILD_DOCKER ;; + backend-it/hive) echo BACKEND_IT_HIVE ;; + backend-it/client) echo BACKEND_IT_CLIENT ;; + 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}" + 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 + projects="$(projects_var "${suite}" "${shard}")" + print_test_tasks "${projects}" +fi From 87b7431d477faef312ecc1214ecf4aae88868acf Mon Sep 17 00:00:00 2001 From: yuqi Date: Thu, 24 Sep 2026 20:19:32 +0800 Subject: [PATCH 2/5] [#13459] improvement(ci): Use three Backend IT shards and drop the aggregate result jobs - Fold the `client` Backend IT shard into `others`. Both are short, and the merged shard still finishes before the slowest build shard, so PR feedback time is unchanged while each backend runs one job less. - Remove `build-result` and `BackendIT-result`. No branch protection rule requires them, so they only added checks. --- .github/workflows/backend-integration-test.yml | 16 ---------------- .github/workflows/build.yml | 16 ---------------- dev/ci/test-shards.sh | 11 +++-------- 3 files changed, 3 insertions(+), 40 deletions(-) diff --git a/.github/workflows/backend-integration-test.yml b/.github/workflows/backend-integration-test.yml index 1508cd3761a..74d72df1eef 100644 --- a/.github/workflows/backend-integration-test.yml +++ b/.github/workflows/backend-integration-test.yml @@ -83,19 +83,3 @@ jobs: backend: ${{ matrix.backend }} test-mode: ${{ matrix.test-mode }} shard: ${{ matrix.shard }} - - # Single status check that summarizes every backend integration test shard, so branch protection - # does not need to track individual shard names. - BackendIT-result: - runs-on: ubuntu-22.04 - needs: [ changes, BackendIT ] - if: always() - steps: - - name: Check backend integration test results - run: | - results='${{ toJSON(needs.*.result) }}' - echo "${results}" - if echo "${results}" | grep -Eq '"(failure|cancelled)"'; then - echo "At least one backend integration test shard failed or was cancelled." - exit 1 - fi diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d7865afad1e..19f01c99364 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -301,19 +301,3 @@ jobs: run: | echo "Total coverage ${{ steps.coverage.outputs.coverage-overall }}" echo "Changed Files coverage ${{ steps.coverage.outputs.coverage-changed-files }}" - - # Single status check that summarizes every build shard, so branch protection does not need to - # track individual shard names. - build-result: - runs-on: ubuntu-latest - needs: [ changes, compile-check, spark-connector-build, build, coverage ] - if: always() - steps: - - name: Check build results - run: | - results='${{ toJSON(needs.*.result) }}' - echo "${results}" - if echo "${results}" | grep -Eq '"(failure|cancelled)"'; then - echo "At least one build job failed or was cancelled." - exit 1 - fi diff --git a/dev/ci/test-shards.sh b/dev/ci/test-shards.sh index a9188a79534..61eece991d7 100755 --- a/dev/ci/test-shards.sh +++ b/dev/ci/test-shards.sh @@ -67,18 +67,14 @@ BUILD_DOCKER=( ) # ---- 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_CLIENT=( - :clients:client-java - :catalogs:catalog-fileset - :clients:filesystem-hadoop3 -) - BACKEND_IT_LAKEHOUSE=( :iceberg:iceberg-rest-server :catalogs:catalog-lakehouse-iceberg @@ -95,7 +91,7 @@ usage() { shards_of() { case "$1" in build) echo "core docker others" ;; - backend-it) echo "hive client lakehouse others" ;; + backend-it) echo "hive lakehouse others" ;; *) echo "Unknown suite: $1" >&2; usage ;; esac } @@ -106,7 +102,6 @@ projects_var() { build/core) echo BUILD_CORE ;; build/docker) echo BUILD_DOCKER ;; backend-it/hive) echo BACKEND_IT_HIVE ;; - backend-it/client) echo BACKEND_IT_CLIENT ;; backend-it/lakehouse) echo BACKEND_IT_LAKEHOUSE ;; *) echo "Unknown shard '$2' for suite '$1'" >&2; usage ;; esac From 95948e20a72f98e797d1c10fc85534a55b57a81c Mon Sep 17 00:00:00 2001 From: Nevin Zheng Date: Sat, 26 Sep 2026 22:27:17 -0700 Subject: [PATCH 3/5] test(core): define explicit database test lanes --- build.gradle.kts | 17 +- core/build.gradle.kts | 104 ++++ .../TestJdbcPartitionStatisticStorageIT.java | 5 + .../storage/AbstractEntityStorageTest.java | 24 +- .../storage/TestBackendTestSelector.java | 138 ++++ .../relational/BackendTestExtension.java | 18 +- .../relational/BackendTestSelector.java | 71 +++ .../storage/relational/TestJDBCBackend.java | 2 + .../service/TestJobMetaService.java | 48 -- .../service/TestJobMetaServiceValidation.java | 78 +++ dev/ci/core_test_identity.py | 587 ++++++++++++++++++ .../core_test_identity/h2/TEST-backend.xml | 27 + .../core_test_identity/legacy/TEST-legacy.xml | 49 ++ .../core_test_identity/mysql/TEST-backend.xml | 27 + .../postgresql/TEST-backend.xml | 29 + .../core_test_identity/unit/TEST-unit.xml | 26 + dev/ci/tests/test_core_test_identity.py | 407 ++++++++++++ 17 files changed, 1596 insertions(+), 61 deletions(-) create mode 100644 core/src/test/java/org/apache/gravitino/storage/TestBackendTestSelector.java create mode 100644 core/src/test/java/org/apache/gravitino/storage/relational/BackendTestSelector.java create mode 100644 core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaServiceValidation.java create mode 100644 dev/ci/core_test_identity.py create mode 100644 dev/ci/tests/fixtures/core_test_identity/h2/TEST-backend.xml create mode 100644 dev/ci/tests/fixtures/core_test_identity/legacy/TEST-legacy.xml create mode 100644 dev/ci/tests/fixtures/core_test_identity/mysql/TEST-backend.xml create mode 100644 dev/ci/tests/fixtures/core_test_identity/postgresql/TEST-backend.xml create mode 100644 dev/ci/tests/fixtures/core_test_identity/unit/TEST-unit.xml create mode 100644 dev/ci/tests/test_core_test_identity.py 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..16d8aa7638e 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,6 +106,107 @@ artifacts { add("testArtifacts", testJar) } +val coreDatabaseTestTag = "gravitino-core-database-test" +val coreH2TestTag = "gravitino-core-h2-test" +val coreMySQLTestTag = "gravitino-core-mysql-test" +val corePostgreSQLTestTag = "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") + inputs.property("includeDockerTaggedTests", 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) { + excludeTags(coreDatabaseTestTag, "gravitino-docker-test") + } else { + includeTags(coreDatabaseTestTag) + when (backend) { + "h2" -> excludeTags(coreMySQLTestTag, corePostgreSQLTestTag) + "mysql" -> excludeTags(coreH2TestTag, corePostgreSQLTestTag) + "postgresql" -> excludeTags(coreH2TestTag, coreMySQLTestTag) + else -> throw GradleException("Unsupported core test backend: $backend") + } + } + } + + 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") + +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) + } +} + tasks.test { val testMode = project.properties["testMode"] as? String ?: "embedded" if (testMode == "embedded") { 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..42b5938596a 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 @@ -85,6 +85,7 @@ *
  • Database-specific SQL syntax (MySQL ON DUPLICATE KEY vs PostgreSQL ON CONFLICT) * */ +@Tag("gravitino-core-database-test") @Tag("gravitino-docker-test") public class TestJdbcPartitionStatisticStorageIT { @@ -95,6 +96,7 @@ public class TestJdbcPartitionStatisticStorageIT { * Abstract base class containing all test logic. Each database-specific test class extends this * and implements the database setup. */ + @Tag("gravitino-core-database-test") @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 + @Tag("gravitino-core-mysql-test") @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 + @Tag("gravitino-core-postgresql-test") @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 + @Tag("gravitino-core-h2-test") 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..ab1c8f76811 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,7 @@ 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.RelationalBackend; import org.apache.gravitino.storage.relational.RelationalEntityStore; import org.apache.gravitino.storage.relational.RelationalGarbageCollector; @@ -103,10 +105,12 @@ import org.apache.ibatis.session.SqlSession; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@Tag("gravitino-core-database-test") 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..68e57488392 --- /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..4f32ec11e63 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,22 @@ 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()); + 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/TestJDBCBackend.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java index 1e4fe8c884f..ac7cc18fec1 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 @@ -81,9 +81,11 @@ import org.apache.gravitino.utils.NamespaceUtil; import org.apache.ibatis.session.SqlSession; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; +@Tag("gravitino-core-database-test") @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/dev/ci/core_test_identity.py b/dev/ci/core_test_identity.py new file mode 100644 index 00000000000..84b888126be --- /dev/null +++ b/dev/ci/core_test_identity.py @@ -0,0 +1,587 @@ +#!/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. A separate +legacy comparison proves that the four split lanes preserve the exact test +multiset previously exercised by ``:core:test``. +""" + +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 + ("legacy",) +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 _combine_counters(counters): + """Combine identity Counters without changing their inputs.""" + combined = Counter() + for counter in counters: + combined.update(counter) + return combined + + +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 compare_legacy_partition(legacy_manifest_file, split_manifest_files): + """Prove that legacy core:test equals the exact sum of the split lanes.""" + legacy_lane, legacy_manifest, legacy_identities = _load_manifest( + legacy_manifest_file + ) + if legacy_lane != "legacy": + raise ManifestError( + f"Expected a legacy manifest, got lane {legacy_lane} from {legacy_manifest_file}" + ) + + reconciliation = reconcile_manifests(split_manifest_files) + split_manifests, split_counters = _load_split_manifests(split_manifest_files) + split_identities = _combine_counters(split_counters[lane] for lane in LANES) + if legacy_identities != split_identities: + difference = _format_identity_difference(legacy_identities, split_identities) + raise ManifestError(f"Legacy/split identity mismatch: {difference}") + + split_statuses = { + key: sum(split_manifests[lane]["status_counts"][key] for lane in LANES) + for key in STATUS_KEYS + } + return { + "schema_version": SCHEMA_VERSION, + "successful": True, + "partition_equal": True, + "legacy": _lane_summary(legacy_manifest, legacy_identities), + "split": { + "test_count": sum(split_identities.values()), + "unique_identity_count": len(split_identities), + "duration_seconds": reconciliation["combined_duration_seconds"], + "status_counts": split_statuses, + "source_file_count": sum( + len(split_manifests[lane]["source_files"]) for lane in LANES + ), + "source_files": { + lane: split_manifests[lane]["source_files"] for lane in LANES + }, + "identity_digest": _identity_digest(split_identities), + }, + "lanes": reconciliation["lanes"], + } + + +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" + ) + + comparison_parser = subparsers.add_parser( + "compare-legacy", + help="Compare a legacy core:test manifest with the four split lanes", + ) + comparison_parser.add_argument( + "--legacy-manifest", + required=True, + type=Path, + help="Manifest generated from the legacy :core:test results", + ) + comparison_parser.add_argument( + "--split-manifests", + required=True, + nargs="+", + type=Path, + help="The unit, H2, MySQL, and PostgreSQL manifests", + ) + comparison_parser.add_argument( + "--output", required=True, type=Path, help="Partition comparison JSON 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) + elif args.command == "reconcile": + document = reconcile_manifests(args.manifests) + else: + document = compare_legacy_partition( + args.legacy_manifest, args.split_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/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/legacy/TEST-legacy.xml b/dev/ci/tests/fixtures/core_test_identity/legacy/TEST-legacy.xml new file mode 100644 index 00000000000..e1b8809b57e --- /dev/null +++ b/dev/ci/tests/fixtures/core_test_identity/legacy/TEST-legacy.xml @@ -0,0 +1,49 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 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..045f60034e6 --- /dev/null +++ b/dev/ci/tests/test_core_test_identity.py @@ -0,0 +1,407 @@ +# 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_compare_legacy_partition_requires_exact_split_multiset(self): + with tempfile.TemporaryDirectory() as temp_dir: + output_directory = Path(temp_dir) + split_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) + split_manifest_files.append(manifest_file) + + legacy = core_test_identity.build_manifest( + "legacy", FIXTURES / "legacy" + ) + legacy_file = output_directory / "legacy.json" + core_test_identity.write_json(legacy, legacy_file) + comparison = core_test_identity.compare_legacy_partition( + legacy_file, split_manifest_files + ) + + self.assertTrue(comparison["successful"]) + self.assertTrue(comparison["partition_equal"]) + self.assertEqual(comparison["legacy"]["test_count"], 21) + self.assertEqual(comparison["split"]["test_count"], 21) + self.assertEqual( + comparison["legacy"]["identity_digest"], + comparison["split"]["identity_digest"], + ) + self.assertEqual( + comparison["legacy"]["source_files"], ["TEST-legacy.xml"] + ) + + mismatched = copy.deepcopy(legacy) + mismatched["identities"][0]["name"] += "-different" + mismatched["identity_digest"] = core_test_identity._identity_digest( + identity_counter(mismatched) + ) + mismatched_file = output_directory / "legacy-mismatched.json" + core_test_identity.write_json(mismatched, mismatched_file) + with self.assertRaisesRegex( + core_test_identity.ManifestError, "Legacy/split identity mismatch" + ): + core_test_identity.compare_legacy_partition( + mismatched_file, split_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"]) + + legacy_manifest_file = output_directory / "legacy.json" + return_code = core_test_identity.main( + [ + "manifest", + "--lane", + "legacy", + "--results", + str(FIXTURES / "legacy"), + "--output", + str(legacy_manifest_file), + ] + ) + self.assertEqual(return_code, 0) + + comparison_file = output_directory / "legacy-comparison.json" + return_code = core_test_identity.main( + [ + "compare-legacy", + "--legacy-manifest", + str(legacy_manifest_file), + "--split-manifests", + *(str(path) for path in manifest_files), + "--output", + str(comparison_file), + ] + ) + self.assertEqual(return_code, 0) + with comparison_file.open(encoding="utf-8") as source: + comparison = json.load(source) + self.assertTrue(comparison["partition_equal"]) + + +if __name__ == "__main__": + unittest.main() From 6412c3e02f580669eda8fadca5bdd3e192cdf63f Mon Sep 17 00:00:00 2001 From: Nevin Zheng Date: Sat, 26 Sep 2026 22:27:34 -0700 Subject: [PATCH 4/5] ci: run core database tests in independent shards --- .github/workflows/build.yml | 204 +++++++++++++++++++++++++++++++++++- dev/ci/test-shards.sh | 44 ++++++-- 2 files changed, 233 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 19f01c99364..ea7caca5c8d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -203,12 +203,17 @@ jobs: 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=( "${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 @@ -230,8 +235,74 @@ 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' + if: >- + github.event_name == 'pull_request' && + !startsWith(matrix.shard, 'core-') uses: actions/upload-artifact@v7 with: name: jacoco-${{ matrix.shard }} @@ -250,19 +321,91 @@ jobs: 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: 15 - needs: build - if: github.event_name == 'pull_request' && needs.build.result == 'success' + 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 run: git fetch origin ${{ github.base_ref }} --depth=1 @@ -272,6 +415,56 @@ jobs: 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 run: | @@ -295,6 +488,7 @@ jobs: path: | coverage-report.md pr-number.txt + core/build/reports/jacoco/test/jacocoTestReport.xml - name: Output Coverage Info if: steps.coverage.outputs.has_reports == 'true' diff --git a/dev/ci/test-shards.sh b/dev/ci/test-shards.sh index 61eece991d7..0c868c75a69 100755 --- a/dev/ci/test-shards.sh +++ b/dev/ci/test-shards.sh @@ -27,16 +27,22 @@ # 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 projects of all named -# shards, so a new module is always tested by `others` until it is moved to a named shard. -# To rebalance, move a project between the lists below; the workflows need no change. +# 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` holds the shared test environment lock for its whole run, so it gets its own shard. -BUILD_CORE=( - :core +# 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 @@ -90,7 +96,7 @@ usage() { # Prints the shard names of a suite, in matrix order. shards_of() { case "$1" in - build) echo "core docker others" ;; + build) echo "${BUILD_CORE_SHARDS[*]} docker others" ;; backend-it) echo "hive lakehouse others" ;; *) echo "Unknown suite: $1" >&2; usage ;; esac @@ -99,7 +105,6 @@ shards_of() { # Prints the variable name holding the projects of a named shard. projects_var() { case "$1/$2" in - build/core) echo BUILD_CORE ;; build/docker) echo BUILD_DOCKER ;; backend-it/hive) echo BACKEND_IT_HIVE ;; backend-it/lakehouse) echo BACKEND_IT_LAKEHOUSE ;; @@ -117,6 +122,17 @@ print_test_tasks() { 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 @@ -148,6 +164,14 @@ if [ "${shard}" = "others" ]; then *) echo "Unknown suite: ${suite}" >&2; usage ;; esac else - projects="$(projects_var "${suite}" "${shard}")" - print_test_tasks "${projects}" + 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 From 095f7d7430f4a8c441ba7c095210fed72045b7c2 Mon Sep 17 00:00:00 2001 From: Nevin Zheng Date: Sun, 27 Sep 2026 01:43:41 -0700 Subject: [PATCH 5/5] test(core): reuse database fixtures by class --- .../job/TestJobManagerMultiNode.java | 3 + .../storage/AbstractEntityStorageTest.java | 12 +- .../gravitino/storage/TestSQLScripts.java | 3 + .../relational/BackendTestExtension.java | 369 ++++++++++++++--- .../storage/relational/DatabaseFixture.java | 42 ++ .../storage/relational/DatabaseIsolation.java | 32 ++ .../relational/TestBackendTestExtension.java | 390 ++++++++++++++++++ .../test/container/ContainerSuite.java | 17 +- .../test/container/TestContainerSuite.java | 48 +++ .../test/util/CloseContainerExtension.java | 23 +- 10 files changed, 846 insertions(+), 93 deletions(-) create mode 100644 core/src/test/java/org/apache/gravitino/storage/relational/DatabaseFixture.java create mode 100644 core/src/test/java/org/apache/gravitino/storage/relational/DatabaseIsolation.java create mode 100644 core/src/test/java/org/apache/gravitino/storage/relational/TestBackendTestExtension.java create mode 100644 integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestContainerSuite.java diff --git a/core/src/test/java/org/apache/gravitino/job/TestJobManagerMultiNode.java b/core/src/test/java/org/apache/gravitino/job/TestJobManagerMultiNode.java index 487645a10d9..8336560691f 100644 --- a/core/src/test/java/org/apache/gravitino/job/TestJobManagerMultiNode.java +++ b/core/src/test/java/org/apache/gravitino/job/TestJobManagerMultiNode.java @@ -51,6 +51,8 @@ import org.apache.gravitino.meta.JobEntity; import org.apache.gravitino.meta.JobTemplateEntity; import org.apache.gravitino.storage.RandomIdGenerator; +import org.apache.gravitino.storage.relational.DatabaseFixture; +import org.apache.gravitino.storage.relational.DatabaseIsolation; import org.apache.gravitino.storage.relational.RelationalEntityStore; import org.apache.gravitino.storage.relational.TestJDBCBackend; import org.apache.gravitino.utils.NameIdentifierUtil; @@ -66,6 +68,7 @@ * JobManager} and {@link LocalJobExecutor}, share the same relational metadata store. The status * pull and the cleanup are triggered manually, so that the test controls which node runs them when. */ +@DatabaseFixture(DatabaseIsolation.FRESH_NAMESPACE) public class TestJobManagerMultiNode extends TestJDBCBackend { private static final String METALAKE = "metalake_job_multi_node"; 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 ab1c8f76811..1e303bef1a2 100644 --- a/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java +++ b/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java @@ -76,8 +76,8 @@ import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; import org.apache.gravitino.file.Fileset; -import org.apache.gravitino.integration.test.container.ContainerSuite; import org.apache.gravitino.integration.test.util.BaseIT; +import org.apache.gravitino.integration.test.util.CloseContainerExtension; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.meta.CatalogEntity; @@ -103,14 +103,15 @@ import org.apache.gravitino.storage.relational.converters.SQLExceptionConverterFactory; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; import org.apache.ibatis.session.SqlSession; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @Tag("gravitino-core-database-test") +@ExtendWith(CloseContainerExtension.class) abstract class AbstractEntityStorageTest { protected static final Logger LOG = LoggerFactory.getLogger(AbstractEntityStorageTest.class); @@ -134,13 +135,6 @@ static Object[][] storageProvider() { .toArray(Object[][]::new); } - @AfterEach - void closeSuit() throws IOException { - // todo: refactor TestEntityStorage to extend TestJDBCBackend, otherwise, each test will start - // and stop the container suite. - ContainerSuite.getInstance().close(); - } - protected void init(String type, Config config) throws IllegalAccessException { Preconditions.checkArgument(StringUtils.isNotBlank(type)); File dir = new File(DB_DIR); diff --git a/core/src/test/java/org/apache/gravitino/storage/TestSQLScripts.java b/core/src/test/java/org/apache/gravitino/storage/TestSQLScripts.java index c7586147786..1a222d78e4e 100644 --- a/core/src/test/java/org/apache/gravitino/storage/TestSQLScripts.java +++ b/core/src/test/java/org/apache/gravitino/storage/TestSQLScripts.java @@ -34,12 +34,15 @@ import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.gravitino.storage.relational.DatabaseFixture; +import org.apache.gravitino.storage.relational.DatabaseIsolation; import org.apache.gravitino.storage.relational.TestJDBCBackend; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; import org.apache.ibatis.session.SqlSession; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.TestTemplate; +@DatabaseFixture(DatabaseIsolation.FRESH_NAMESPACE) public class TestSQLScripts extends TestJDBCBackend { @TestTemplate 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 4f32ec11e63..12d2ce26bbe 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 @@ -50,12 +50,15 @@ import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.service.EntityIdService; import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.Extension; import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.LifecycleMethodExecutionExceptionHandler; import org.junit.jupiter.api.extension.TestTemplateInvocationContext; import org.junit.jupiter.api.extension.TestTemplateInvocationContextProvider; +import org.junit.jupiter.api.extension.TestWatcher; import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -70,34 +73,44 @@ public class BackendTestExtension ExtensionContext.Namespace.create(BackendTestExtension.class); private static final String STORE_KEY = "BACKEND_MAP"; + private final BackendFactory backendFactory; + + /** Creates the extension with the production database-backend factory. */ + public BackendTestExtension() { + this(new DefaultBackendFactory()); + } + + BackendTestExtension(BackendFactory backendFactory) { + this.backendFactory = backendFactory; + } + @Override public void beforeAll(ExtensionContext context) { - // Initialize a Map and store it at the Class level. - // Key: backendType ("h2", "mysql"...), Value: RelationalBackend instance - context.getStore(NAMESPACE).put(STORE_KEY, new ConcurrentHashMap()); + context.getStore(NAMESPACE).put(STORE_KEY, new ConcurrentHashMap()); } @Override @SuppressWarnings("unchecked") - public void afterAll(ExtensionContext context) { - // Test class ended, close all started Backend - ConcurrentHashMap map = - (ConcurrentHashMap) context.getStore(NAMESPACE).get(STORE_KEY); + public void afterAll(ExtensionContext context) throws Exception { + ConcurrentHashMap map = + (ConcurrentHashMap) context.getStore(NAMESPACE).remove(STORE_KEY); if (map != null) { - map.forEach( - (type, backend) -> { - try { - LOG.info("Tearing down backend: {}", type); - backend.close(); - // H2 special cleaning logic - if ("h2".equals(type) && backend instanceof H2BackendWrapper) { - ((H2BackendWrapper) backend).cleanFile(); - } - } catch (Exception e) { - LOG.error("Failed to close backend {}", type, e); - } - }); + Exception failure = null; + for (BackendResource resource : map.values()) { + try { + resource.close(); + } catch (Exception e) { + if (failure == null) { + failure = e; + } else { + failure.addSuppressed(e); + } + } + } + if (failure != null) { + throw failure; + } } } @@ -113,8 +126,8 @@ public Stream provideTestTemplateInvocationContex 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)); + return createInvocationContexts( + testMethodName, Collections.singletonList(selectedBackend.get()), true); } List backendsToTest = new ArrayList<>(); @@ -130,17 +143,33 @@ public Stream provideTestTemplateInvocationContex "Running tests with H2 backend only. Set env var 'dockerTest=true' to include all backends."); } - return backendsToTest.stream() - .map(backendType -> new BackendInvocationContext(testMethodName, backendType)); + return createInvocationContexts(testMethodName, backendsToTest, false); + } + + Stream createInvocationContexts( + String testMethodName, List backends, boolean reuseBackend) { + return backends.stream() + .map( + backendType -> + new BackendInvocationContext( + testMethodName, backendType, reuseBackend, backendFactory)); } private static class BackendInvocationContext implements TestTemplateInvocationContext { private final String testMethodName; private final String backendType; - - public BackendInvocationContext(String testMethodName, String backendType) { + private final boolean reuseBackend; + private final BackendFactory backendFactory; + + private BackendInvocationContext( + String testMethodName, + String backendType, + boolean reuseBackend, + BackendFactory backendFactory) { this.testMethodName = testMethodName; this.backendType = backendType; + this.reuseBackend = reuseBackend; + this.backendFactory = backendFactory; } @Override @@ -150,35 +179,26 @@ public String getDisplayName(int invocationIndex) { @Override public List getAdditionalExtensions() { - return Collections.singletonList(new BackendSetupCallback(backendType)); + return Collections.singletonList( + new BackendSetupCallback(backendType, reuseBackend, backendFactory)); } } - private static class BackendSetupCallback implements BeforeEachCallback { - private final String backendType; - private final BaseIT baseIT = new BaseIT(); + @FunctionalInterface + interface BackendFactory { + BackendResource create(String backendType) throws Exception; + } - public BackendSetupCallback(String backendType) { - this.backendType = backendType; - } + @FunctionalInterface + interface BackendActivator { + void activate() throws Exception; + } + private static class DefaultBackendFactory implements BackendFactory { @Override - public void beforeEach(ExtensionContext context) throws Exception { - - // Lazy loading: If the backend of this type has not started, start it - RelationalBackend backend = startBackend(backendType); - - // Inject into the test instance - Object testInstance = context.getRequiredTestInstance(); - if (testInstance instanceof TestJDBCBackend) { - LOG.info("Injecting {} backend into test instance", backendType); - ((TestJDBCBackend) testInstance).setBackend(backend); - ((TestJDBCBackend) testInstance).setBackendType(backendType); - } - } - - private RelationalBackend startBackend(String type) throws Exception { - LOG.info("Initializing backend resource: {}", type); + public BackendResource create(String backendType) throws Exception { + BaseIT baseIT = new BaseIT(); + LOG.info("Initializing backend resource: {}", backendType); Config config = Mockito.mock(Config.class); Mockito.when(config.get(Configs.ENTITY_STORE)).thenReturn(Configs.RELATIONAL_ENTITY_STORE); Mockito.when(config.get(Configs.ENTITY_RELATIONAL_STORE)) @@ -189,20 +209,15 @@ private RelationalBackend startBackend(String type) throws Exception { .thenReturn(DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLISECONDS); Mockito.when(config.get(CACHE_ENABLED)).thenReturn(true); - - FieldUtils.writeField(GravitinoEnv.getInstance(), "config", config, true); - FieldUtils.writeField( - GravitinoEnv.getInstance(), "idGenerator", RandomIdGenerator.INSTANCE, true); - RelationalBackend backend = new JDBCBackend(); - if ("mysql".equals(type)) { + if ("mysql".equals(backendType)) { String url = baseIT.startAndInitMySQLBackend(); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_URL)).thenReturn(url); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)).thenReturn("root"); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD)).thenReturn("root"); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER)) .thenReturn("com.mysql.cj.jdbc.Driver"); - } else if ("postgresql".equals(type)) { + } else if ("postgresql".equals(backendType)) { String url = baseIT.startAndInitPGBackend(); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_URL)).thenReturn(url); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)).thenReturn("root"); @@ -223,15 +238,240 @@ private RelationalBackend startBackend(String type) throws Exception { Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER)).thenReturn("org.h2.Driver"); // Wrap it with a Wrapper so that files can be deleted during the clean process - backend = new H2BackendWrapper(config, jdbcPath); + backend = new H2BackendWrapper(jdbcPath); + } + + BackendResource resource = + new BackendResource( + backendType, + backend, + () -> { + FieldUtils.writeField(GravitinoEnv.getInstance(), "config", config, true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "idGenerator", RandomIdGenerator.INSTANCE, true); + EntityIdService.initialize(new RelationalEntityStoreIdResolver()); + }); + try { + resource.activate(); + // Close the backend before initializing to make sure the singleton sqlSession is cleared. + backend.close(); + backend.initialize(config); + return resource; + } catch (Exception failure) { + try { + resource.close(); + } catch (Exception closeFailure) { + failure.addSuppressed(closeFailure); + } + throw failure; + } + } + } + + private static class BackendSetupCallback + implements BeforeEachCallback, + AfterEachCallback, + LifecycleMethodExecutionExceptionHandler, + TestWatcher { + private final String backendType; + private final boolean reuseBackend; + private final BackendFactory backendFactory; + private BackendResource backendResource; + private boolean closeAfterEach; + + private BackendSetupCallback( + String backendType, boolean reuseBackend, BackendFactory backendFactory) { + this.backendType = backendType; + this.reuseBackend = reuseBackend; + this.backendFactory = backendFactory; + } + + @Override + public void beforeEach(ExtensionContext context) throws Exception { + DatabaseIsolation isolation = isolation(context); + if (isolation == DatabaseIsolation.DEDICATED_SERVER) { + throw new UnsupportedOperationException( + "DEDICATED_SERVER isolation is not implemented for core database tests"); } - // close the backend before initializing to make sure the singleton sqlSession - // has been cleared. - backend.close(); - backend.initialize(config); - EntityIdService.initialize(new RelationalEntityStoreIdResolver()); - return backend; + closeAfterEach = !reuseBackend || isolation == DatabaseIsolation.FRESH_NAMESPACE; + try { + if (reuseBackend && isolation == DatabaseIsolation.FRESH_NAMESPACE) { + closeClassBackend(context); + } + backendResource = closeAfterEach ? newBackendResource() : getOrCreateClassBackend(context); + backendResource.activate(); + injectBackend(context, backendResource.backend); + } catch (Exception e) { + cleanupAfterSetupFailure(e); + throw e; + } + } + + @Override + public void afterEach(ExtensionContext context) throws Exception { + if (closeAfterEach && backendResource != null) { + backendResource.close(); + backendResource = null; + } + } + + @Override + public void testAborted(ExtensionContext context, Throwable cause) { + poisonSharedFixture(); + } + + @Override + public void testFailed(ExtensionContext context, Throwable cause) { + poisonSharedFixture(); + } + + @Override + public void handleBeforeEachMethodExecutionException( + ExtensionContext context, Throwable throwable) throws Throwable { + poisonSharedFixture(); + throw throwable; + } + + @Override + public void handleAfterEachMethodExecutionException( + ExtensionContext context, Throwable throwable) throws Throwable { + poisonSharedFixture(); + throw throwable; + } + + private void injectBackend(ExtensionContext context, RelationalBackend backend) { + Object testInstance = context.getRequiredTestInstance(); + if (testInstance instanceof TestJDBCBackend) { + LOG.info("Injecting {} backend into test instance", backendType); + ((TestJDBCBackend) testInstance).setBackend(backend); + ((TestJDBCBackend) testInstance).setBackendType(backendType); + } + } + + private BackendResource getOrCreateClassBackend(ExtensionContext context) throws Exception { + ConcurrentHashMap backendMap = backendMap(context); + synchronized (backendMap) { + BackendResource resource = backendMap.get(backendType); + if (resource != null && resource.poisoned) { + backendMap.remove(backendType); + resource.close(); + resource = null; + } + if (resource == null) { + resource = newBackendResource(); + backendMap.put(backendType, resource); + } + return resource; + } + } + + private void closeClassBackend(ExtensionContext context) throws Exception { + ConcurrentHashMap backendMap = backendMap(context); + synchronized (backendMap) { + BackendResource resource = backendMap.remove(backendType); + if (resource != null) { + resource.close(); + } + } + } + + @SuppressWarnings("unchecked") + private ConcurrentHashMap backendMap(ExtensionContext context) { + ExtensionContext classContext = context; + while (classContext.getTestMethod().isPresent()) { + classContext = + classContext + .getParent() + .orElseThrow(() -> new IllegalStateException("Test class context is unavailable")); + } + + ConcurrentHashMap backendMap = + (ConcurrentHashMap) + classContext.getStore(NAMESPACE).get(STORE_KEY); + if (backendMap == null) { + throw new IllegalStateException("Backend fixture store is unavailable"); + } + return backendMap; + } + + private BackendResource newBackendResource() throws Exception { + return backendFactory.create(backendType); + } + + private void cleanupAfterSetupFailure(Exception failure) { + if (backendResource == null) { + return; + } + if (!closeAfterEach) { + backendResource.poisoned = true; + return; + } + try { + backendResource.close(); + } catch (Exception closeFailure) { + failure.addSuppressed(closeFailure); + } finally { + backendResource = null; + } + } + + private void poisonSharedFixture() { + if (!closeAfterEach && backendResource != null) { + backendResource.poisoned = true; + } + } + + private DatabaseIsolation isolation(ExtensionContext context) { + DatabaseFixture methodFixture = + context.getRequiredTestMethod().getAnnotation(DatabaseFixture.class); + if (methodFixture != null) { + return methodFixture.value(); + } + DatabaseFixture classFixture = + context.getRequiredTestClass().getAnnotation(DatabaseFixture.class); + return classFixture == null ? DatabaseIsolation.RESETTABLE_NAMESPACE : classFixture.value(); + } + } + + static class BackendResource { + private final String backendType; + private final RelationalBackend backend; + private final BackendActivator activator; + private boolean poisoned; + + BackendResource(String backendType, RelationalBackend backend, BackendActivator activator) { + this.backendType = backendType; + this.backend = backend; + this.activator = activator; + } + + private void activate() throws Exception { + activator.activate(); + } + + private void close() throws Exception { + LOG.info("Tearing down backend: {}", backendType); + Exception failure = null; + try { + backend.close(); + } catch (Exception e) { + failure = e; + } + try { + if (backend instanceof H2BackendWrapper) { + ((H2BackendWrapper) backend).cleanFile(); + } + } catch (Exception e) { + if (failure == null) { + failure = e; + } else { + failure.addSuppressed(e); + } + } + if (failure != null) { + throw failure; + } } } @@ -239,9 +479,8 @@ private RelationalBackend startBackend(String type) throws Exception { public static class H2BackendWrapper extends JDBCBackend { private final String path; - public H2BackendWrapper(Config config, String path) { + public H2BackendWrapper(String path) { this.path = path; - super.initialize(config); } public void cleanFile() throws IOException { diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseFixture.java b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseFixture.java new file mode 100644 index 00000000000..6c966af4b0c --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseFixture.java @@ -0,0 +1,42 @@ +/* + * 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; + +/** Declares the database-fixture isolation required by a test class or method. */ +@Documented +@Inherited +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE, ElementType.METHOD}) +public @interface DatabaseFixture { + + /** + * Returns the required database isolation. + * + * @return the isolation policy + */ + DatabaseIsolation value() default DatabaseIsolation.RESETTABLE_NAMESPACE; +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseIsolation.java b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseIsolation.java new file mode 100644 index 00000000000..c87d27ca6ca --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseIsolation.java @@ -0,0 +1,32 @@ +/* + * 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; + +/** Isolation policies for database-backed tests. */ +public enum DatabaseIsolation { + /** Reuse a class-scoped fixture and reset its tables before each test. */ + RESETTABLE_NAMESPACE, + + /** Create and close a fresh database backend for each test. */ + FRESH_NAMESPACE, + + /** Create a dedicated database server for each test. */ + DEDICATED_SERVER +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestBackendTestExtension.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestBackendTestExtension.java new file mode 100644 index 00000000000..57dcef8aeee --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestBackendTestExtension.java @@ -0,0 +1,390 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.AfterEachCallback; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.Extension; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.LifecycleMethodExecutionExceptionHandler; +import org.junit.jupiter.api.extension.TestTemplateInvocationContext; +import org.junit.jupiter.api.extension.TestWatcher; + +class TestBackendTestExtension { + + @Test + void testSelectedBackendReusesClassFixture() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, true, contexts.methodContext("first")); + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(1, factory.backends.size()); + assertEquals(2, factory.activationCount); + verify(factory.backends.get(0), never()).close(); + + extension.afterAll(contexts.classContext); + verify(factory.backends.get(0), times(1)).close(); + } + + @Test + void testLegacyH2DoesNotReuseFixture() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, false, contexts.methodContext("first")); + runInvocation(extension, false, contexts.methodContext("second")); + + assertEquals(2, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), times(1)).close(); + + extension.afterAll(contexts.classContext); + } + + @Test + void testFreshClassCreatesAndClosesEachInvocation() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(FreshIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, true, contexts.methodContext("first")); + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(2, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), times(1)).close(); + + extension.afterAll(contexts.classContext); + } + + @Test + void testFreshMethodEvictsSharedFixture() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(MixedIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, true, contexts.methodContext("resettable")); + runInvocation(extension, true, contexts.methodContext("fresh")); + runInvocation(extension, true, contexts.methodContext("resettable")); + + assertEquals(3, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), times(1)).close(); + verify(factory.backends.get(2), never()).close(); + + extension.afterAll(contexts.classContext); + verify(factory.backends.get(2), times(1)).close(); + } + + @Test + void testFailedSharedFixtureIsClosedAndRebuilt() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + ExtensionContext firstContext = contexts.methodContext("first"); + Extension firstCallback = startInvocation(extension, true, firstContext); + ((AfterEachCallback) firstCallback).afterEach(firstContext); + ((TestWatcher) firstCallback).testFailed(firstContext, new AssertionError("expected")); + + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(2, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), never()).close(); + + extension.afterAll(contexts.classContext); + verify(factory.backends.get(1), times(1)).close(); + } + + @Test + void testLifecycleFailurePoisonsSharedFixture() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + ExtensionContext firstContext = contexts.methodContext("first"); + Extension firstCallback = startInvocation(extension, true, firstContext); + assertThrows( + IllegalStateException.class, + () -> + ((LifecycleMethodExecutionExceptionHandler) firstCallback) + .handleBeforeEachMethodExecutionException( + firstContext, new IllegalStateException("expected"))); + + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(2, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + + extension.afterAll(contexts.classContext); + verify(factory.backends.get(1), times(1)).close(); + } + + @Test + void testActivationFailurePoisonsSharedFixture() throws Exception { + List backends = new ArrayList<>(); + AtomicInteger activations = new AtomicInteger(); + BackendTestExtension.BackendFactory factory = + backendType -> { + RelationalBackend backend = mock(RelationalBackend.class); + backends.add(backend); + return new BackendTestExtension.BackendResource( + backendType, + backend, + () -> { + if (activations.incrementAndGet() == 2) { + throw new IllegalStateException("expected"); + } + }); + }; + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, true, contexts.methodContext("first")); + assertThrows( + IllegalStateException.class, + () -> startInvocation(extension, true, contexts.methodContext("second"))); + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(2, backends.size()); + verify(backends.get(0), times(1)).close(); + + extension.afterAll(contexts.classContext); + verify(backends.get(1), times(1)).close(); + } + + @Test + void testDedicatedServerIsolationFailsFast() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DedicatedServerTests.class); + extension.beforeAll(contexts.classContext); + + assertThrows( + UnsupportedOperationException.class, + () -> startInvocation(extension, true, contexts.methodContext("first"))); + assertEquals(0, factory.backends.size()); + + extension.afterAll(contexts.classContext); + } + + @Test + void testAfterAllClosesEveryFixtureAndAggregatesFailures() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, true, "h2", contexts.methodContext("first")); + runInvocation(extension, true, "mysql", contexts.methodContext("second")); + doThrow(new IOException("h2 close")).when(factory.backends.get(0)).close(); + doThrow(new IOException("mysql close")).when(factory.backends.get(1)).close(); + + Exception failure = + assertThrows(Exception.class, () -> extension.afterAll(contexts.classContext)); + assertEquals(1, failure.getSuppressed().length); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), times(1)).close(); + } + + @Test + void testH2CleanupRunsAfterUncheckedBackendCloseFailure() throws Exception { + BackendTestExtension.H2BackendWrapper backend = + mock(BackendTestExtension.H2BackendWrapper.class); + BackendTestExtension extension = + new BackendTestExtension( + backendType -> + new BackendTestExtension.BackendResource(backendType, backend, () -> {})); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + runInvocation(extension, true, contexts.methodContext("first")); + + doThrow(new IllegalStateException("close")).when(backend).close(); + doThrow(new IOException("clean")).when(backend).cleanFile(); + + Exception failure = + assertThrows(Exception.class, () -> extension.afterAll(contexts.classContext)); + assertEquals("close", failure.getMessage()); + assertEquals(1, failure.getSuppressed().length); + assertEquals("clean", failure.getSuppressed()[0].getMessage()); + verify(backend, times(1)).cleanFile(); + } + + @Test + void testDisplayNamePreservesMethodAndBackend() { + BackendTestExtension extension = + new BackendTestExtension( + backendType -> + new BackendTestExtension.BackendResource( + backendType, mock(RelationalBackend.class), () -> {})); + + TestTemplateInvocationContext context = + extension + .createInvocationContexts("testMethod", List.of("h2"), true) + .findFirst() + .orElseThrow(); + + assertEquals("testMethod()[H2 Backend]", context.getDisplayName(1)); + } + + private static void runInvocation( + BackendTestExtension extension, boolean reuseBackend, ExtensionContext context) + throws Exception { + runInvocation(extension, reuseBackend, "h2", context); + } + + private static void runInvocation( + BackendTestExtension extension, + boolean reuseBackend, + String backendType, + ExtensionContext context) + throws Exception { + Extension callback = startInvocation(extension, reuseBackend, backendType, context); + ((AfterEachCallback) callback).afterEach(context); + } + + private static Extension startInvocation( + BackendTestExtension extension, boolean reuseBackend, ExtensionContext context) + throws Exception { + return startInvocation(extension, reuseBackend, "h2", context); + } + + private static Extension startInvocation( + BackendTestExtension extension, + boolean reuseBackend, + String backendType, + ExtensionContext context) + throws Exception { + TestTemplateInvocationContext invocation = + extension + .createInvocationContexts("testMethod", List.of(backendType), reuseBackend) + .findFirst() + .orElseThrow(); + Extension callback = invocation.getAdditionalExtensions().get(0); + ((BeforeEachCallback) callback).beforeEach(context); + return callback; + } + + private static class CountingBackendFactory implements BackendTestExtension.BackendFactory { + private final List backends = new ArrayList<>(); + private int activationCount; + + @Override + public BackendTestExtension.BackendResource create(String backendType) { + RelationalBackend backend = mock(RelationalBackend.class); + backends.add(backend); + return new BackendTestExtension.BackendResource( + backendType, backend, () -> activationCount++); + } + } + + private static class TestContexts { + private final Class testClass; + private final ExtensionContext classContext; + + private TestContexts(Class testClass) { + this.testClass = testClass; + classContext = mock(ExtensionContext.class); + ExtensionContext.Store store = mock(ExtensionContext.Store.class); + Map entries = new HashMap<>(); + + doAnswer( + invocation -> { + entries.put(invocation.getArgument(0), invocation.getArgument(1)); + return null; + }) + .when(store) + .put(any(), any()); + when(store.get(any())).thenAnswer(invocation -> entries.get(invocation.getArgument(0))); + when(store.remove(any())).thenAnswer(invocation -> entries.remove(invocation.getArgument(0))); + when(classContext.getStore(any(ExtensionContext.Namespace.class))).thenReturn(store); + when(classContext.getTestMethod()).thenReturn(Optional.empty()); + when(classContext.getRequiredTestClass()).thenAnswer(invocation -> testClass); + } + + private ExtensionContext methodContext(String methodName) throws NoSuchMethodException { + Method method = testClass.getDeclaredMethod(methodName); + ExtensionContext methodContext = mock(ExtensionContext.class); + when(methodContext.getTestMethod()).thenReturn(Optional.of(method)); + when(methodContext.getRequiredTestMethod()).thenReturn(method); + when(methodContext.getRequiredTestClass()).thenAnswer(invocation -> testClass); + when(methodContext.getRequiredTestInstance()).thenReturn(this); + when(methodContext.getParent()).thenReturn(Optional.of(classContext)); + return methodContext; + } + } + + private static class DefaultIsolationTests { + void first() {} + + void second() {} + } + + @DatabaseFixture(DatabaseIsolation.FRESH_NAMESPACE) + private static class FreshIsolationTests { + void first() {} + + void second() {} + } + + private static class MixedIsolationTests { + void resettable() {} + + @DatabaseFixture(DatabaseIsolation.FRESH_NAMESPACE) + void fresh() {} + } + + @DatabaseFixture(DatabaseIsolation.DEDICATED_SERVER) + private static class DedicatedServerTests { + void first() {} + } +} diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java index 9adff2730da..14e82b762e5 100644 --- a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java @@ -382,10 +382,10 @@ public void startDorisContainer(DorisImageName imageName) { } public void startMySQLContainer(TestDatabaseName testDatabaseName) { - ITUtils.cleanDisk(); if (mySQLContainer == null) { synchronized (ContainerSuite.class) { if (mySQLContainer == null) { + ITUtils.cleanDisk(); initIfNecessary(); // Start MySQL container MySQLContainer.Builder mysqlBuilder = @@ -439,10 +439,10 @@ public void startMySQLVersion5Container(TestDatabaseName testDatabaseName) { } public void startPostgreSQLContainer(TestDatabaseName testDatabaseName, PGImageName pgImageName) { - ITUtils.cleanDisk(); if (!pgContainerMap.containsKey(pgImageName)) { synchronized (ContainerSuite.class) { if (!pgContainerMap.containsKey(pgImageName)) { + ITUtils.cleanDisk(); initIfNecessary(); // Start PostgreSQL container PostgreSQLContainer.Builder pgBuilder = @@ -964,7 +964,8 @@ private static long[] cidrToRange(String cidr) throws Exception { @Override public void close() throws IOException { try { - closer.close(); + closeResources(closer); + } finally { mySQLContainer = null; mySQLVersion5Container = null; hiveContainer = null; @@ -978,8 +979,16 @@ public void close() throws IOException { kerberosHiveContainer = null; sqlBaseHiveContainer = null; pgContainerMap.clear(); + } + } + + static void closeResources(CloseableGroup resources) throws IOException { + try { + resources.close(); + } catch (IOException e) { + throw e; } catch (Exception e) { - LOG.error("Failed to close ContainerEnvironment", e); + throw new IOException("Failed to close ContainerEnvironment", e); } } } diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestContainerSuite.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestContainerSuite.java new file mode 100644 index 00000000000..4c7a21b934e --- /dev/null +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestContainerSuite.java @@ -0,0 +1,48 @@ +/* + * 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.integration.test.container; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import org.apache.gravitino.integration.test.util.CloseableGroup; +import org.junit.jupiter.api.Test; + +class TestContainerSuite { + + @Test + void testCloseResourcesPropagatesFailureAfterClosingEveryResource() { + CloseableGroup resources = CloseableGroup.create(); + int[] closeCount = new int[1]; + resources.register(() -> closeCount[0]++); + resources.register( + () -> { + closeCount[0]++; + throw new IOException("expected"); + }); + + IOException failure = + assertThrows(IOException.class, () -> ContainerSuite.closeResources(resources)); + + assertEquals("expected", failure.getMessage()); + assertEquals(2, closeCount[0]); + } +} diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/CloseContainerExtension.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/CloseContainerExtension.java index a37a9647009..5e26bf52488 100644 --- a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/CloseContainerExtension.java +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/CloseContainerExtension.java @@ -32,14 +32,11 @@ public class CloseContainerExtension implements BeforeAllCallback { @Override public void beforeAll(ExtensionContext extensionContext) { - // Ensure that the container suite is initialized before closing it - if (ContainerSuite.initialized()) { - synchronized (CloseContainerExtension.class) { - extensionContext - .getRoot() - .getStore(ExtensionContext.Namespace.GLOBAL) - .getOrComputeIfAbsent(CloseableContainer.class); - } + synchronized (CloseContainerExtension.class) { + extensionContext + .getRoot() + .getStore(ExtensionContext.Namespace.GLOBAL) + .getOrComputeIfAbsent(CloseableContainer.class); } } @@ -48,13 +45,9 @@ private static class CloseableContainer implements ExtensionContext.Store.Closea private static final ContainerSuite CONTAINER_SUITE = ContainerSuite.getInstance(); @Override - public void close() { - try { - CONTAINER_SUITE.close(); - LOGGER.info("Containers were closed successfully"); - } catch (Exception e) { - LOGGER.warn("Containers were not closed as expected", e); - } + public void close() throws Exception { + CONTAINER_SUITE.close(); + LOGGER.info("Containers were closed successfully"); } } }