From 3f0bcb51143d273fcd19533eb2a21ff18d4a8181 Mon Sep 17 00:00:00 2001 From: Sourav Maji Date: Tue, 17 Dec 2024 08:55:59 -0800 Subject: [PATCH] Further split services unit tests into Routers/Servers/Controllers (#1399) The client module contains da-vinci-client which contains most of the server code. This PR splits the unit tests into more categories based on the venice components like router/server/controller etc which should improve total UT CI runtime further from 28 mins to 12mins. Also fixed the some flakiness in SIT tests for jdk8. --- .github/workflows/UnitTests-core.yml | 6 ++- .../VeniceCI-StaticAnalysisAndUnitTests.yml | 40 +++++++++++++------ .../consumer/StoreIngestionTaskTest.java | 1 + 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/.github/workflows/UnitTests-core.yml b/.github/workflows/UnitTests-core.yml index 40a8f3da72b..4778fc999fa 100644 --- a/.github/workflows/UnitTests-core.yml +++ b/.github/workflows/UnitTests-core.yml @@ -50,12 +50,14 @@ jobs: rsync -R --files-from=artifacts.list . ${{ inputs.artifact_suffix }}-artifacts tar -zcvf ${{ inputs.artifact_suffix }}-jdk${{ matrix.jdk }}-logs.tar.gz ${{ inputs.artifact_suffix }}-artifacts - name: Publish Test Report + continue-on-error: true env: NODE_OPTIONS: "--max_old_space_size=8192" uses: mikepenz/action-junit-report@v3 - if: always() # always run even if the previous step fails + if: always() with: - report_paths: '**/build/test-results/test/TEST-*.xml' + check_name: ${{ inputs.artifact_suffix }}-jdk${{ matrix.jdk }} Report + report_paths: '**/build/test-results/test/TEST-*.xml' - name: Upload Build Artifacts if: success() || failure() uses: actions/upload-artifact@v4 diff --git a/.github/workflows/VeniceCI-StaticAnalysisAndUnitTests.yml b/.github/workflows/VeniceCI-StaticAnalysisAndUnitTests.yml index 07c41228aae..0c33b4b587b 100644 --- a/.github/workflows/VeniceCI-StaticAnalysisAndUnitTests.yml +++ b/.github/workflows/VeniceCI-StaticAnalysisAndUnitTests.yml @@ -65,8 +65,7 @@ jobs: uses: ./.github/workflows/UnitTests-core.yml with: artifact_suffix: clients - arg: :clients:da-vinci-client:jacocoTestCoverageVerification :clients:da-vinci-client:diffCoverage - :clients:venice-admin-tool:jacocoTestCoverageVerification :clients:venice-admin-tool:diffCoverage + arg: :clients:venice-admin-tool:jacocoTestCoverageVerification :clients:venice-admin-tool:diffCoverage :clients:venice-producer:jacocoTestCoverageVerification :clients:venice-producer:diffCoverage :integrations:venice-pulsar:jacocoTestCoverageVerification :integrations:venice-pulsar:diffCoverage :clients:venice-client:jacocoTestCoverageVerification :clients:venice-client:diffCoverage @@ -84,19 +83,28 @@ jobs: :internal:venice-test-common:jacocoTestCoverageVerification :internal:venice-test-common:diffCoverage --continue - Services: - uses: ./.github/workflows/UnitTests-core.yml - with: - artifact_suffix: services - arg: :services:venice-controller:jacocoTestCoverageVerification :services:venice-controller:diffCoverage - :services:venice-router:jacocoTestCoverageVerification :services:venice-router:diffCoverage - :services:venice-server:jacocoTestCoverageVerification :services:venice-server:diffCoverage --continue + Controller: + uses: ./.github/workflows/UnitTests-core.yml + with: + artifact_suffix: controller + arg: :services:venice-controller:jacocoTestCoverageVerification :services:venice-controller:diffCoverage --continue + Server: + uses: ./.github/workflows/UnitTests-core.yml + with: + artifact_suffix: server + arg: :clients:da-vinci-client:jacocoTestCoverageVerification :clients:da-vinci-client:diffCoverage + :services:venice-server:jacocoTestCoverageVerification :services:venice-server:diffCoverage --continue + Router: + uses: ./.github/workflows/UnitTests-core.yml + with: + artifact_suffix: router + arg: :services:venice-router:jacocoTestCoverageVerification :services:venice-router:diffCoverage --continue StaticAnalysisAndUnitTestsCompletionCheck: strategy: fail-fast: false runs-on: ubuntu-latest - needs: [ValidateGradleWrapper, StaticAnalysis, Clients, Internal, Services] + needs: [ValidateGradleWrapper, StaticAnalysis, Clients, Internal, Controller, Server, Router] timeout-minutes: 120 if: success() || failure() # Always run this job, regardless of previous job status steps: @@ -118,8 +126,16 @@ jobs: echo "Internal module unit tests failed." exit 1 fi - if [ "${{ needs.Services.result }}" != "success" ]; then - echo "Services module unit tests failed." + if [ "${{ needs.Controller.result }}" != "success" ]; then + echo "Controller module unit tests failed." + exit 1 + fi + if [ "${{ needs.Server.result }}" != "success" ]; then + echo "Server module unit tests failed." + exit 1 + fi + if [ "${{ needs.Router.result }}" != "success" ]; then + echo "Router module unit tests failed." exit 1 fi # If all previous jobs were successful, proceed diff --git a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/kafka/consumer/StoreIngestionTaskTest.java b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/kafka/consumer/StoreIngestionTaskTest.java index c174a8f584e..1d2e117d42c 100644 --- a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/kafka/consumer/StoreIngestionTaskTest.java +++ b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/kafka/consumer/StoreIngestionTaskTest.java @@ -2323,6 +2323,7 @@ public void testDataValidationCheckPointing(SortedInput sortedInput, AAConfig aa }); StoreIngestionTaskTestConfig config = new StoreIngestionTaskTestConfig(relevantPartitions, () -> { + Utils.sleep(1000); // Verify that all partitions reported success. maxOffsetPerPartition.entrySet() .stream()