[duckdb] Moved integ test out of venice-test-common (#1455)

Added a new CI task dedicated to Duck Vinci. Dependency changes: - Re-introduced the snapshot dependency for duckdb. - Removed the dependency on venice-duckdb from venice-test-common, so that this artifact is not tainted by a snapshot dependency. - Changed venice-duckdb deps from implementation to api.
linkedin · Jan 17, 2025 · 216474f · 216474f
1 parent 3c7b161
commit 216474f
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 117 deletions.
diff --git a/.github/workflows/VeniceCI-CompatibilityTests.yml b/.github/workflows/VeniceCI-CompatibilityTests.yml
@@ -50,6 +50,50 @@ jobs:
           path: ${{ github.job }}-jdk${{ matrix.jdk }}-logs.tar.gz
           retention-days: 30
 
+  DuckVinciIntegrationTests:
+    strategy:
+      fail-fast: false
+      matrix:
+        jdk: [17]
+    runs-on: ubuntu-latest
+    timeout-minutes: 60
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - name: Set up JDK
+        uses: actions/setup-java@v4
+        with:
+          java-version: ${{ matrix.jdk }}
+          distribution: 'temurin'
+          cache: 'gradle'
+      - shell: bash
+        run: |
+          git remote set-head origin --auto
+          git remote add upstream https://github.com/linkedin/venice
+          git fetch upstream
+      - name: Setup Gradle
+        uses: gradle/actions/setup-gradle@v4
+        with:
+          add-job-summary: never
+      - name: Run DuckDB Integration Tests
+        run: ./gradlew -DforkEvery=1 -DmaxParallelForks=1 :integrations:venice-duckdb:integrationTest --continue
+      - name: Package Build Artifacts
+        if: success() || failure()
+        shell: bash
+        run: |
+          mkdir ${{ github.job }}-artifacts
+          find . -path "**/build/reports/*" -or -path "**/build/test-results/*" > artifacts.list
+          rsync -R --files-from=artifacts.list . ${{ github.job }}-artifacts
+          tar -zcvf ${{ github.job }}-jdk${{ matrix.jdk }}-logs.tar.gz ${{ github.job }}-artifacts
+      - name: Upload Build Artifacts
+        if: success() || failure()
+        uses: actions/upload-artifact@v4
+        with:
+          name: ${{ github.job }}
+          path: ${{ github.job }}-jdk${{ matrix.jdk }}-logs.tar.gz
+          retention-days: 30
+
   PulsarVeniceIntegrationTests:
     strategy:
       fail-fast: false
@@ -114,12 +158,11 @@ jobs:
           path: ${{ github.job }}-artifacts.tar.gz
           retention-days: 30
 
-
   CompatibilityTestsCompletionCheck:
     strategy:
       fail-fast: false
     runs-on: ubuntu-latest
-    needs: [AvroCompatibilityTests, PulsarVeniceIntegrationTests]
+    needs: [AvroCompatibilityTests, DuckVinciIntegrationTests, PulsarVeniceIntegrationTests]
     timeout-minutes: 120
     steps:
       - name: AllIsWell

diff --git a/.github/workflows/VeniceCI-E2ETests.yml b/.github/workflows/VeniceCI-E2ETests.yml
@@ -108,101 +108,6 @@ jobs:
           key: ${{ secrets.BUILDPULSE_ACCESS_KEY_ID }}
           secret: ${{ secrets.BUILDPULSE_SECRET_ACCESS_KEY }}
 
-  IntegrationTests_1001:
-    name: IntegrationTests_1001
-    strategy:
-      fail-fast: false
-      matrix:
-        jdk: [17]
-    runs-on: ubuntu-latest
-    permissions:
-     id-token: write
-     contents: read
-     checks: write
-     pull-requests: write
-     issues: write
-    timeout-minutes: 120
-    concurrency:
-     group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}-jdk${{ matrix.jdk }}-IntegrationTests_1001
-     cancel-in-progress: ${{ github.event_name == 'pull_request' }}
-    steps:
-      - uses: actions/checkout@v4
-        with:
-          fetch-depth: 0
-      - name: Set up JDK
-        uses: actions/setup-java@v4
-        with:
-          java-version: ${{ matrix.jdk }}
-          distribution: 'temurin'
-          cache: 'gradle'
-      - shell: bash
-        run: |
-          git remote set-head origin --auto
-          git remote add upstream https://github.com/linkedin/venice
-          git fetch upstream
-      - name: Setup Gradle
-        uses: gradle/actions/setup-gradle@v4
-        with:
-          add-job-summary: never
-      - name: Run Integration Tests
-        run: ./gradlew --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTests_1001
-      - name: Package Build Artifacts
-        if: success() || failure()
-        shell: bash
-        run: |
-          mkdir ${{ github.job }}-artifacts
-          echo "Repository owner: ${{ github.repository_owner }}"
-          echo "Repository name: ${{ github.repository }}"
-          echo "event name: ${{ github.event_name }}"
-          find . -path "**/build/reports/*" -or -path "**/build/test-results/*" > artifacts.list
-          rsync -R --files-from=artifacts.list . ${{ github.job }}-artifacts
-          tar -zcvf ${{ github.job }}-jdk${{ matrix.jdk }}-logs.tar.gz ${{ github.job }}-artifacts
-      - name: Generate Fork Repo Test Reports
-        if: ${{ (github.repository_owner != 'linkedin') && (success() || failure()) }}
-        uses: dorny/test-reporter@v1.9.1
-        env:
-         NODE_OPTIONS: --max-old-space-size=9182
-        with:
-          token: ${{ secrets.GITHUB_TOKEN }}
-          name: ${{ github.job }} Test Reports       # Name where it report the test results
-          path: '**/TEST-*.xml'
-          fail-on-error: 'false'
-          max-annotations: '10'
-          list-tests: 'all'
-          list-suites: 'all'
-          reporter: java-junit
-      - name: Publish Test Report
-        continue-on-error: true
-        env:
-          NODE_OPTIONS: "--max_old_space_size=8192"
-        uses: mikepenz/action-junit-report@v5
-        if: always()
-        with:
-          check_name: ${{ github.job }}-jdk${{ matrix.jdk }} Report
-          comment: false
-          annotate_only: true
-          flaky_summary: true
-          commit: ${{github.event.workflow_run.head_sha}}
-          detailed_summary: true
-          report_paths: '**/build/test-results/test/TEST-*.xml'
-      - name: Upload Build Artifacts
-        if: success() || failure()
-        uses: actions/upload-artifact@v4
-        with:
-          name: ${{ github.job }}
-          path: ${{ github.job }}-jdk${{ matrix.jdk }}-logs.tar.gz
-          retention-days: 30
-      - name: Upload test results to BuildPulse for flaky test detection
-        if: github.event_name == 'push' && github.ref == 'refs/heads/main' && !cancelled()
-        uses: buildpulse/buildpulse-action@main
-        with:
-          account: 100582612927
-          repository: 100441445875
-          path: |
-            **/TEST-*.xml
-          key: ${{ secrets.BUILDPULSE_ACCESS_KEY_ID }}
-          secret: ${{ secrets.BUILDPULSE_SECRET_ACCESS_KEY }}
-
   IntegrationTests_1010:
     name: IntegrationTests_1010
     strategy:
@@ -3059,7 +2964,7 @@ jobs:
       matrix:
         jdk: [17]
     runs-on: ubuntu-latest
-    needs: [IntegrationTests_1000, IntegrationTests_1001, IntegrationTests_1010, IntegrationTests_1020, IntegrationTests_1030, IntegrationTests_1040, IntegrationTests_1050, IntegrationTests_1060, IntegrationTests_1070, IntegrationTests_1080, IntegrationTests_1090, IntegrationTests_1100, IntegrationTests_1110, IntegrationTests_1120, IntegrationTests_1130, IntegrationTests_1200, IntegrationTests_1210, IntegrationTests_1220, IntegrationTests_1230, IntegrationTests_1240, IntegrationTests_1250, IntegrationTests_1260, IntegrationTests_1270, IntegrationTests_1280, IntegrationTests_1400, IntegrationTests_1410, IntegrationTests_1420, IntegrationTests_1430, IntegrationTests_1440, IntegrationTests_1500, IntegrationTests_1550, IntegrationTests_9999]
+    needs: [IntegrationTests_1000, IntegrationTests_1010, IntegrationTests_1020, IntegrationTests_1030, IntegrationTests_1040, IntegrationTests_1050, IntegrationTests_1060, IntegrationTests_1070, IntegrationTests_1080, IntegrationTests_1090, IntegrationTests_1100, IntegrationTests_1110, IntegrationTests_1120, IntegrationTests_1130, IntegrationTests_1200, IntegrationTests_1210, IntegrationTests_1220, IntegrationTests_1230, IntegrationTests_1240, IntegrationTests_1250, IntegrationTests_1260, IntegrationTests_1270, IntegrationTests_1280, IntegrationTests_1400, IntegrationTests_1410, IntegrationTests_1420, IntegrationTests_1430, IntegrationTests_1440, IntegrationTests_1500, IntegrationTests_1550, IntegrationTests_9999]
     timeout-minutes: 20
     steps:
     - name: NoOp

diff --git a/build.gradle b/build.gradle
@@ -82,7 +82,7 @@ ext.libraries = [
     commonsLang: 'commons-lang:commons-lang:2.6',
     conscrypt: 'org.conscrypt:conscrypt-openjdk-uber:2.5.2',
     d2: "com.linkedin.pegasus:d2:${pegasusVersion}",
-    duckdbJdbc: "org.duckdb:duckdb_jdbc:1.1.3",
+    duckdbJdbc: "org.duckdb:duckdb_jdbc:1.2.0-20250117.012118-121", // TODO: Remove SNAPSHOT when the real release is published!
     failsafe: 'net.jodah:failsafe:2.4.0',
     fastUtil: 'it.unimi.dsi:fastutil:8.3.0',
     grpcNettyShaded: "io.grpc:grpc-netty-shaded:${grpcVersion}",

diff --git a/integrations/venice-duckdb/build.gradle b/integrations/venice-duckdb/build.gradle
@@ -1,13 +1,60 @@
+configurations {
+  integrationTestImplementation.extendsFrom testImplementation
+}
+
 dependencies {
-  implementation libraries.avro
-  implementation libraries.avroUtilCompatHelper
-  implementation libraries.duckdbJdbc
+  api libraries.avro
+  api libraries.avroUtilCompatHelper
+  api libraries.duckdbJdbc
   api libraries.log4j2api
 
-  implementation project(':clients:da-vinci-client')
-  implementation project(':internal:venice-client-common')
+  api project(':clients:da-vinci-client')
+  api project(':internal:venice-client-common')
+
+  api project(':internal:venice-common')
+
+  testImplementation project(':internal:venice-common')
+
+  integrationTestImplementation project(path: ':internal:venice-test-common', configuration: 'integrationTestUtils')
+  integrationTestImplementation project(':clients:venice-push-job')
+  integrationTestImplementation project(':clients:venice-thin-client')
+  integrationTestImplementation project(':integrations:venice-duckdb')
+}
+
+
+sourceSets {
+  integrationTest {
+    // 'src/integrationTest/java' is in srcDir by default. Just add the proto directory
+    resources
+  }
+}
+
+def integrationTestConfigs = {
+  mustRunAfter test
+  classpath = sourceSets.integrationTest.runtimeClasspath
+  testClassesDirs = sourceSets.integrationTest.output.classesDirs
+  forkEvery = Integer.parseInt(project.properties.get('integrationTest.forkEvery', "$forkEvery"))
+  maxParallelForks = Integer.parseInt(project.properties.get('integrationTest.maxParallelForks', "$maxParallelForks"))
+}
+
+task integrationTest(type: Test) {
+  configure integrationTestConfigs
+}
+check.dependsOn(integrationTest)
+
+idea {
+  module {
+    testSourceDirs += project.sourceSets.integrationTest.java.srcDirs
+  }
+}
+
+task integrationTestJar(type: Jar) {
+  classifier 'integrationTest'
+  from sourceSets.integrationTest.output
+}
 
-  implementation project(':internal:venice-common')
+artifacts {
+  integrationTestJar
 }
 
 checkerFramework {

diff --git a/...inciRecordTransformerIntegrationTest.java → ...inciRecordTransformerIntegrationTest.java b/...inciRecordTransformerIntegrationTest.java → ...inciRecordTransformerIntegrationTest.java
@@ -70,8 +70,16 @@
 import org.testng.annotations.Test;
 
 
+/**
+ * On DuckDB 1.1.3, this test works on mac but fails on the CI, likely due to: https://github.com/duckdb/duckdb-java/issues/14
+ *
+ * With a more recent snapshot release, where the above issue is fixed and merged, it works in both environments.
+ *
+ * Once there is an official release containing this fix, we could consider moving the integration tests back to
+ * venice-test-common.
+ */
 public class DuckDBDaVinciRecordTransformerIntegrationTest {
-  private static final Logger LOGGER = LogManager.getLogger(DaVinciClientRecordTransformerTest.class);
+  private static final Logger LOGGER = LogManager.getLogger(DuckDBDaVinciRecordTransformerIntegrationTest.class);
   private static final int TEST_TIMEOUT = 3 * Time.MS_PER_MINUTE;
   private VeniceClusterWrapper cluster;
   private D2Client d2Client;
@@ -109,14 +117,7 @@ public void deleteClassHash() {
     }
   }
 
-  /**
-   * This test works on mac but fails on the CI, likely due to: https://github.com/duckdb/duckdb-java/issues/14
-   *
-   * There is a fix merged for this, but it has not been released yet.
-   *
-   * TODO: Re-enable once we can depend on a clean release.
-   */
-  @Test(timeOut = TEST_TIMEOUT, enabled = false)
+  @Test(timeOut = TEST_TIMEOUT)
   public void testRecordTransformer() throws Exception {
     DaVinciConfig clientConfig = new DaVinciConfig();
 

diff --git a/internal/venice-test-common/build.gradle b/internal/venice-test-common/build.gradle
@@ -75,7 +75,6 @@ dependencies {
   implementation project(':internal:alpini:netty4:alpini-netty4-base')
   implementation project(':internal:alpini:router:alpini-router-api')
   implementation project(':internal:alpini:router:alpini-router-base')
-  implementation project(':integrations:venice-duckdb')
 
   implementation('org.apache.helix:helix-core:1.4.1:jdk8') {
     exclude group: 'org.apache.helix'
@@ -140,8 +139,6 @@ def integrationTestBuckets = [
       "com.linkedin.venice.endToEnd.DaVinciClientDiskFullTest",
       "com.linkedin.venice.endToEnd.DaVinciClientMemoryLimitTest",
       "com.linkedin.venice.endToEnd.DaVinciClientRecordTransformerTest"],
-  "1001": [
-      "com.linkedin.venice.endToEnd.DuckDBDaVinciRecordTransformerIntegrationTest"],
   "1010": [
        "com.linkedin.venice.endToEnd.DaVinciClientTest"],
   "1020": [