[#3940] improvement(gradle): skip Docker dependent tests by default #3974

mchades · 2024-06-26T06:33:55Z

What changes were proposed in this pull request?

skip Docker dependent tests by default when run build and test task
don't skip Docker dependent tests in CI

Why are the changes needed?

Fix: #3940

Does this PR introduce any user-facing change?

no

How was this patch tested?

verified locally

.github/workflows/backend-integration-test.yml

yuqi1129 · 2024-06-26T08:01:14Z

docs/how-to-build.md

-+ Make sure you have installed Docker in your environment if you plan to run integration texts. Without it, some Docker-related tests may not run.
+ Gravitino excludes all Docker-related tests by default. To run integration tests, make sure you have installed
+  Docker in your environment and set `skipDockerDependentTests=false` in the `gradle.properties` file(or
+  use `-PskipDockerDependentTests=false` in the command). Otherwise, some Docker-related tests may not execute.


What do we do with the configuration skipITs? Do we need to remove it?

I don't think we need to do anything with skipITs. Because both UT and IT may use Docker, parameter skipITs is used to control IT, while the new configuration skipDockerDependentTests is used to control Docker.

I understand what you mean. Do other people have any suggestions about it?

I prefer to combine skipITs and skipDockerDependentTests, because it's too complicated. maybe we could use skipTestsWithDocker and skipTestsWithoutDocker

I prefer to combine skipITs and skipDockerDependentTests, because it's too complicated. maybe we could use skipTestsWithDocker and skipTestsWithoutDocker

This is the same with removing skipITs , right?

gradle.properties

jerryshao · 2024-06-26T11:04:56Z

build.gradle.kts

@@ -676,7 +674,9 @@ fun printDockerCheckInfo() {
  val macDockerConnector = project.extra["macDockerConnector"] as? Boolean ?: false
  val isOrbStack = project.extra["isOrbStack"] as? Boolean ?: false

-  if (OperatingSystem.current().isMacOsX() &&
+  if (extra["skipDockerDependentTests"].toString().toBoolean()) {
+    project.extra["docker_it_test"] = false


can you rename the "docker_it_test" to "docker_test"?

Also for the tag name "gravitino-docker-it", can you please change to "gravitino-docker-test"?

yuqi1129 · 2024-06-26T12:17:41Z

I'm okay with the current changes.

yuqi1129 · 2024-06-26T12:44:34Z

docs/how-to-build.md

@@ -26,7 +26,9 @@ This software is licensed under the Apache License version 2."
  You don't have to preinstall the specified JDK environment, as Gradle detects the JDK version needed and downloads it automatically.
 + Gravitino uses the Gradle Java Toolchain to detect and manage JDK versions, it checks the
  installed JDK by running the `./gradlew javaToolchains` command. See [Gradle Java Toolchain](https://docs.gradle.org/current/userguide/toolchains.html#sec:java_toolchain).
-+ Make sure you have installed Docker in your environment if you plan to run integration texts. Without it, some Docker-related tests may not run.
+ Gravitino excludes all Docker-related tests by default. To run integration tests, make sure you have installed


To run integration tests and docker related unit test, or you can use 'to run docker related test'

zhoukangcn · 2024-06-26T14:09:58Z

.github/workflows/build.yml

@@ -66,7 +66,7 @@ jobs:
          cache: 'gradle'

      - name: Build with Gradle
-        run: ./gradlew build -x test -PjdkVersion=8
+        run: ./gradlew build -x test -PjdkVersion=8 -PskipDockerTests=false


I'm a little confused, -x test is used here to skip tests, so why do we need to set skipDockerTests=false?

yeah, it's redundant, I will remove it

jerryshao · 2024-06-27T09:20:18Z

.github/workflows/backend-integration-test.yml

@@ -83,7 +83,7 @@ jobs:
      - name: Package Gravitino
        if : ${{ matrix.test-mode == 'deploy' }}
        run: |
-          ./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }}
+          ./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false


Also here, it is meaningless to add -PskipDockerTests=false here. Please recheck all the changes, don't blindly add the property.

jerryshao · 2024-06-27T09:27:04Z

docs/how-to-build.md

@@ -26,7 +26,9 @@ This software is licensed under the Apache License version 2."
  You don't have to preinstall the specified JDK environment, as Gradle detects the JDK version needed and downloads it automatically.
 + Gravitino uses the Gradle Java Toolchain to detect and manage JDK versions, it checks the
  installed JDK by running the `./gradlew javaToolchains` command. See [Gradle Java Toolchain](https://docs.gradle.org/current/userguide/toolchains.html#sec:java_toolchain).
-+ Make sure you have installed Docker in your environment if you plan to run integration texts. Without it, some Docker-related tests may not run.
+ Gravitino excludes all Docker-related tests by default. To run Docker related tests, make sure you have installed
+  Docker in your environment and set `skipDockerTests=false` in the `gradle.properties` file(or


Add the whitespace after file.

jerryshao · 2024-06-27T09:29:05Z

docs/how-to-build.md

-+ Make sure you have installed Docker in your environment if you plan to run integration texts. Without it, some Docker-related tests may not run.
+ Gravitino excludes all Docker-related tests by default. To run Docker related tests, make sure you have installed
+  Docker in your environment and set `skipDockerTests=false` in the `gradle.properties` file(or
+  use `-PskipDockerTests=false` in the command). Otherwise, some Docker-related tests may not execute.


"Otherwise, all Docker required tests will skip."

yuqi1129 · 2024-06-28T02:48:51Z

.github/workflows/build.yml

@@ -96,7 +96,7 @@ jobs:
          dev/ci/util_free_space.sh

      - name: Build with Gradle
-        run: ./gradlew build -PskipITs -PjdkVersion=${{ matrix.java-version }} -x :clients:client-python:build
+        run: ./gradlew build -PskipITs -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false -x :clients:client-python:build


Why do we enable docker tests for build task?

build task needs to run UTs, some UTs include docker tests(such as Doris catalog)

skip Docker dependent tests by default

0f78369

mchades requested review from jerryshao, xunliu, shaofengshi and yuqi1129 June 26, 2024 06:33

mchades self-assigned this Jun 26, 2024

yuqi1129 reviewed Jun 26, 2024

View reviewed changes

.github/workflows/backend-integration-test.yml Outdated Show resolved Hide resolved

yuqi1129 reviewed Jun 26, 2024

View reviewed changes

jerryshao reviewed Jun 26, 2024

View reviewed changes

gradle.properties Outdated Show resolved Hide resolved

jerryshao reviewed Jun 26, 2024

View reviewed changes

rename to skipDockerTests

4a69c43

mchades requested review from jerryshao and yuqi1129 June 26, 2024 12:09

rename tag gravitino-docker-it to gravitino-docker-test

12ed8fe

mchades force-pushed the ignore-docker branch from 610a771 to 12ed8fe Compare June 26, 2024 12:15

yuqi1129 reviewed Jun 26, 2024

View reviewed changes

fix doc

62831ac

zhoukangcn reviewed Jun 26, 2024

View reviewed changes

remove redundant

5ff4fb3

mchades requested review from FANNG1, yuqi1129 and zhoukangcn June 27, 2024 02:58

jerryshao reviewed Jun 27, 2024

View reviewed changes

mchades added 2 commits June 28, 2024 10:35

fix doc

6a61134

fix

ee0c665

yuqi1129 reviewed Jun 28, 2024

View reviewed changes

jerryshao approved these changes Jun 28, 2024

View reviewed changes

jerryshao merged commit 1565e19 into apache:main Jun 28, 2024
33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#3940] improvement(gradle): skip Docker dependent tests by default #3974

[#3940] improvement(gradle): skip Docker dependent tests by default #3974

mchades commented Jun 26, 2024

yuqi1129 Jun 26, 2024

mchades Jun 26, 2024

yuqi1129 Jun 26, 2024

FANNG1 Jun 26, 2024 •

edited

Loading

mchades Jun 26, 2024

jerryshao Jun 26, 2024

jerryshao Jun 26, 2024

mchades Jun 26, 2024

yuqi1129 commented Jun 26, 2024

yuqi1129 Jun 26, 2024

mchades Jun 26, 2024

zhoukangcn Jun 26, 2024

mchades Jun 27, 2024

jerryshao Jun 27, 2024 •

edited

Loading

jerryshao Jun 27, 2024

jerryshao Jun 27, 2024

yuqi1129 Jun 28, 2024

mchades Jun 28, 2024

[#3940] improvement(gradle): skip Docker dependent tests by default #3974

[#3940] improvement(gradle): skip Docker dependent tests by default #3974

Conversation

mchades commented Jun 26, 2024

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FANNG1 Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuqi1129 commented Jun 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerryshao Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FANNG1 Jun 26, 2024 •

edited

Loading

jerryshao Jun 27, 2024 •

edited

Loading