From 0610bcfbe334fdb84879f622f9ac6d521591eab5 Mon Sep 17 00:00:00 2001 From: Xun Liu Date: Thu, 18 Apr 2024 20:44:45 +0800 Subject: [PATCH] [#2292] improvement(PyClient): Fix unstable Python client integration test (#2994) ### What changes were proposed in this pull request? 1. Add `skipPyClientITs` param in Gradle build script. Let's Python client integration test not running in the backend ITs 2. Combined Gradle build command of Python client test and integration test 3. improvement FilesetCatalog integration test codes. 4. Test mode + Test Principle: Every Python ITs class base on the `IntegrationTestEnv`, `IntegrationTestEnv` will automatic start and stop Gravitino server to support ITs, But when you run multiple ITs class at same time, The first test class that finishes running will shut down the Gravitino server, which will cause other test classes to fail if they can't connect to the Gravitino server. + Run test in the IDE: Through `IntegrationTestEnv` class automatic start and stop Gravitino server to support ITs. + Run test in the Github Action: Manual start and stop Gravitino server, and set `EXTERNAL_START_GRAVITINO` environment variable + Run test in the Gradle command `:client:client-python:test`: Gradle automatic start and stop Gravitino server, and set `EXTERNAL_START_GRAVITINO` environment variable. ### Why are the changes needed? Fix: #2292 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Through CI check. --- .../workflows/backend-integration-test.yml | 2 +- .github/workflows/python-integration-test.yml | 18 ++++----- clients/client-python/build.gradle.kts | 39 +++++++++---------- .../tests/integration/integration_test_env.py | 24 +++++++----- .../tests/integration/test_fileset_catalog.py | 11 +++--- 5 files changed, 49 insertions(+), 45 deletions(-) diff --git a/.github/workflows/backend-integration-test.yml b/.github/workflows/backend-integration-test.yml index 3cd93760e91..68664e85d2b 100644 --- a/.github/workflows/backend-integration-test.yml +++ b/.github/workflows/backend-integration-test.yml @@ -95,7 +95,7 @@ jobs: - name: Backend Integration Test id: integrationTest run: | - ./gradlew test --rerun-tasks -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} -PskipWebITs -P${{ matrix.backend }} + ./gradlew test --rerun-tasks -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} -PskipWebITs -P${{ matrix.backend }} -PskipPyClientITs - name: Upload integrate tests reports uses: actions/upload-artifact@v3 diff --git a/.github/workflows/python-integration-test.yml b/.github/workflows/python-integration-test.yml index 95e18493fa6..2d9a6f9dd49 100644 --- a/.github/workflows/python-integration-test.yml +++ b/.github/workflows/python-integration-test.yml @@ -57,18 +57,18 @@ jobs: - name: Set up QEMU uses: docker/setup-qemu-action@v2 - - name: Free up disk space - run: | - dev/ci/util_free_space.sh - - name: Python Client Integration Test id: integrationTest run: | - #./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }} - #for pythonVersion in "3.8" "3.9" "3.10" "3.11" - #do - # ./gradlew -PjdkVersion=${{ matrix.java-version }} -PpythonVersion=${pythonVersion} :client:client-python:integrationTest - #done + ./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }} + + for pythonVersion in "3.8" "3.9" "3.10" "3.11" + do + echo "Use Python version ${pythonVersion} to test the Python client." + ./gradlew -PjdkVersion=${{ matrix.java-version }} -PpythonVersion=${pythonVersion} :client:client-python:test + # Clean Gravitino database to clean test data + rm -rf ./distribution/package/data + done - name: Upload integrate tests reports uses: actions/upload-artifact@v3 diff --git a/clients/client-python/build.gradle.kts b/clients/client-python/build.gradle.kts index 21b89367a1f..03bf90f687a 100644 --- a/clients/client-python/build.gradle.kts +++ b/clients/client-python/build.gradle.kts @@ -26,9 +26,9 @@ fun gravitinoServer(operation: String) { val exitCode = process.waitFor() if (exitCode == 0) { val currentContext = process.inputStream.bufferedReader().readText() - println("Current docker context is: $currentContext") + println("Gravitino server status: $currentContext") } else { - println("checkOrbStackStatus Command execution failed with exit code $exitCode") + println("Gravitino server execution failed with exit code $exitCode") } } @@ -39,26 +39,25 @@ tasks { } val test by registering(VenvTask::class) { - dependsOn(pipInstall) - venvExec = "python" - args = listOf("-m", "unittest") - workingDir = projectDir.resolve(".") - } - - val integrationTest by registering(VenvTask::class) { - doFirst() { - gravitinoServer("start") - } + val skipPyClientITs = project.hasProperty("skipPyClientITs") + if (!skipPyClientITs) { + doFirst { + gravitinoServer("start") + } - dependsOn(pipInstall) - venvExec = "python" - args = listOf("-m", "unittest") - workingDir = projectDir.resolve(".") - environment = mapOf("PROJECT_VERSION" to project.version, - "GRADLE_START_GRAVITINO" to "True") + dependsOn(pipInstall) + venvExec = "python" + args = listOf("-m", "unittest") + workingDir = projectDir.resolve(".") + environment = mapOf( + "PROJECT_VERSION" to project.version, + "GRAVITINO_HOME" to project.rootDir.path + "/distribution/package", + "START_EXTERNAL_GRAVITINO" to "true" + ) - doLast { - gravitinoServer("stop") + doLast { + gravitinoServer("stop") + } } } diff --git a/clients/client-python/tests/integration/integration_test_env.py b/clients/client-python/tests/integration/integration_test_env.py index 73cbaed5e01..e02206bf07c 100644 --- a/clients/client-python/tests/integration/integration_test_env.py +++ b/clients/client-python/tests/integration/integration_test_env.py @@ -20,11 +20,11 @@ def get_gravitino_server_version(): response.close() return True except requests.exceptions.RequestException as e: - logger.warning("Failed to access the server: {}", e) + logger.warning("Failed to access the Gravitino server") return False -def check_gravitino_server_status(): +def check_gravitino_server_status() -> bool: gravitino_server_running = False for i in range(5): logger.info("Monitoring Gravitino server status. Attempt %s", i + 1) @@ -53,15 +53,21 @@ class IntegrationTestEnv(unittest.TestCase): def setUpClass(cls): _init_logging() - if os.environ.get('GRADLE_START_GRAVITINO') is not None: - logger.info('Manual start gravitino server [%s].', check_gravitino_server_status()) + if os.environ.get('START_EXTERNAL_GRAVITINO') is not None: + """Maybe Gravitino server already startup by Gradle test command or developer manual startup.""" + if not check_gravitino_server_status(): + logger.error("ERROR: Can't find online Gravitino server!") return - current_path = os.getcwd() - cls.gravitino_startup_script = os.path.join(current_path, '../../../distribution/package/bin/gravitino.sh') + GravitinoHome = os.environ.get('GRAVITINO_HOME') + if GravitinoHome is None: + logger.error('Gravitino Python client integration test must configure `GRAVITINO_HOME`') + quit(0) + + cls.gravitino_startup_script = os.path.join(GravitinoHome, 'bin/gravitino.sh') if not os.path.exists(cls.gravitino_startup_script): logger.error("Can't find Gravitino startup script: %s, " - "Please execute `./gradlew compileDistribution -x test` in the gravitino project root " + "Please execute `./gradlew compileDistribution -x test` in the Gravitino project root " "directory.", cls.gravitino_startup_script) quit(0) @@ -78,11 +84,9 @@ def setUpClass(cls): logger.error("ERROR: Can't start Gravitino server!") quit(0) - cls.clean_test_date() - @classmethod def tearDownClass(cls): - if os.environ.get('GRADLE_START_GRAVITINO') is not None: + if os.environ.get('START_EXTERNAL_GRAVITINO') is not None: return logger.info("Stop integration test environment...") diff --git a/clients/client-python/tests/integration/test_fileset_catalog.py b/clients/client-python/tests/integration/test_fileset_catalog.py index 20d20970392..240547928f5 100644 --- a/clients/client-python/tests/integration/test_fileset_catalog.py +++ b/clients/client-python/tests/integration/test_fileset_catalog.py @@ -3,6 +3,7 @@ This software is licensed under the Apache License version 2. """ import logging +from random import random, randint from gravitino.api.catalog import Catalog from gravitino.api.fileset import Fileset @@ -20,11 +21,11 @@ class TestFilesetCatalog(IntegrationTestEnv): catalog: Catalog = None metalake: GravitinoMetalake = None - metalake_name: str = "testMetalake" - catalog_name: str = "testCatalog" - schema_name: str = "testSchema" - fileset_name: str = "testFileset1" - fileset_alter_name: str = "testFilesetAlter" + metalake_name: str = "testMetalake" + str(randint(1, 100)) + catalog_name: str = "testCatalog" + str(randint(1, 100)) + schema_name: str = "testSchema" + str(randint(1, 100)) + fileset_name: str = "testFileset1" + str(randint(1, 100)) + fileset_alter_name: str = "testFilesetAlter" + str(randint(1, 100)) provider: str = "hadoop" metalake_ident: NameIdentifier = NameIdentifier.of(metalake_name)