Split unit tests by time (hyperledger#7079)

* Split unit tests by time Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> * Parallelize compile and unit tests since there is not shared cache Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> * fix Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> * Reduce ATs runnes to 10 Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> * Apply suggestions from code review Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> --------- Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
fab-10 · May 21, 2024 · 4016447 · 4016447
1 parent 0cca642
commit 4016447
Show file tree

Hide file tree

Showing 5 changed files with 113 additions and 116 deletions.
diff --git a/.github/workflows/acceptance-tests.yml b/.github/workflows/acceptance-tests.yml
@@ -12,7 +12,7 @@ concurrency:
 
 env:
   GRADLE_OPTS: "-Xmx6g"
-  total-runners: 16
+  total-runners: 10
 
 jobs:
   acceptanceTestEthereum:
@@ -24,7 +24,7 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        runner_index: [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]
+        runner_index: [0,1,2,3,4,5,6,7,8,9]
     steps:
       - name: Checkout Repo
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
@@ -37,24 +37,27 @@ jobs:
           java-version: 17
       - name: Install required packages
         run:  sudo apt-get install -y xmlstarlet
-      - name: get acceptance test report
+      - name: setup gradle
+        uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1
+        with:
+          cache-disabled: true
+      - name: List unit tests
+        run: ./gradlew acceptanceTestNotPrivacy --test-dry-run -Dorg.gradle.parallel=true -Dorg.gradle.caching=true
+      - name: Extract current test list
+        run: mkdir tmp; find . -type f -name TEST-*.xml | xargs -I{} bash -c "xmlstarlet sel -t -v '/testsuite/@name' '{}'; echo ' acceptanceTestNotPrivacy'" | tee tmp/currentTests.list
+      - name: get acceptance test reports
         uses: dawidd6/action-download-artifact@e7466d1a7587ed14867642c2ca74b5bcc1e19a2d
         with:
           branch: main
           name_is_regexp: true
           name: 'acceptance-node-\d*\d-test-results'
           path: tmp/junit-xml-reports-downloaded
           if_no_artifact_found: true
-      - name: setup gradle
-        uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1
-        with:
-          cache-disabled: true
-      - name: Compile acceptance tests
-        run: ./gradlew :acceptance-tests:tests:testClasses
-      - name: List acceptance tests
-        run: ./gradlew :acceptance-tests:tests:listAcceptanceTestNotPrivacy -Dorg.gradle.caching=true | fgrep org.hyperledger.besu.tests.acceptance > tmp/currentTests.list
       - name: Split tests
-        run: .github/workflows/splitTestsByTime.sh tmp/junit-xml-reports-downloaded ${{env.total-runners}} ${{ matrix.runner_index }} > testList.txt
+        run: .github/workflows/splitTestsByTime.sh tmp/junit-xml-reports-downloaded "tmp/junit-xml-reports-downloaded/acceptance-node-.*-test-results" "TEST-" ${{env.total-runners}} ${{ matrix.runner_index }} > testList.txt
+      - name: format gradle args
+        # we do not need the module task here
+        run:  cat testList.txt | cut -f 2- -d ' ' | tee gradleArgs.txt
       - name: Upload Timing
         uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3
         if: matrix.runner_index == 0
@@ -67,13 +70,13 @@ jobs:
         with:
           name: acceptance-tests-lists
           path: 'tmp/*.list'
-      - name: format gradle args
-        # insert --tests option between each.
-        run:  cat testList.txt | sed -e 's/^\| / --tests /g' | tee gradleArgs.txt
+      - name: Upload gradle test tasks
+        uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3
+        with:
+          name: test-args-${{ matrix.runner_index }}.txt
+          path: '*.txt'
       - name: run acceptance tests
         run: ./gradlew acceptanceTestNotPrivacy `cat gradleArgs.txt` -Dorg.gradle.parallel=true -Dorg.gradle.caching=true
-      - name: cleanup tempfiles
-        run: rm testList.txt gradleArgs.txt
       - name: Upload Acceptance Test Results
         uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3
         with:

diff --git a/.github/workflows/pre-review.yml b/.github/workflows/pre-review.yml
@@ -11,7 +11,8 @@ concurrency:
   cancel-in-progress: true
 
 env:
-  GRADLE_OPTS: "-Xmx6g -Dorg.gradle.daemon=false -Dorg.gradle.parallel=true"
+  GRADLE_OPTS: "-Xmx6g -Dorg.gradle.parallel=true"
+  total-runners: 8
 
 jobs:
   repolint:
@@ -75,37 +76,14 @@ jobs:
     env:
         GRADLEW_UNIT_TEST_ARGS: ${{matrix.gradle_args}}
     runs-on: ubuntu-22.04
-    needs: [ compile ]
+    needs: [spotless, gradle-wrapper, repolint]
     permissions:
       checks: write
       statuses: write
     strategy:
       fail-fast: true
       matrix:
-        gradle_args:
-          - "test -x besu:test -x consensus:test -x crypto:test -x ethereum:eth:test -x ethereum:api:test -x ethereum:core:test"
-          - "besu:test consensus:test crypto:test"
-          - "ethereum:api:testBonsai"
-          - "ethereum:api:testForest"
-          - "ethereum:api:testRemainder"
-          - "ethereum:eth:test"
-          - "ethereum:core:test"
-        #includes will need exact strings from gradle args above
-        include:
-          - gradle_args: "test -x besu:test -x consensus:test -x crypto:test -x ethereum:eth:test -x ethereum:api:test -x ethereum:core:test"
-            filename: "everythingElse"
-          - gradle_args: "besu:test consensus:test crypto:test"
-            filename: "consensusCrypto"
-          - gradle_args: "ethereum:api:testBonsai"
-            filename: "apiBonsai"
-          - gradle_args: "ethereum:api:testRemainder"
-            filename: "apiForest"
-          - gradle_args: "ethereum:api:testRemainder"
-            filename: "apiRemainder"
-          - gradle_args: "ethereum:eth:test"
-            filename: "eth"
-          - gradle_args: "ethereum:core:test"
-            filename: "core"
+        runner_index: [0,1,2,3,4,5,6,7]
     steps:
       - name: Checkout Repo
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
@@ -116,22 +94,54 @@ jobs:
         with:
           distribution: temurin
           java-version: 17
+      - name: Install required packages
+        run:  sudo apt-get install -y xmlstarlet
       - name: Setup Gradle
         uses: gradle/actions/setup-gradle@9e899d11ad247ec76be7a60bc1cf9d3abbb9e7f1
         with:
           cache-disabled: true
+      - name: List unit tests
+        run: ./gradlew test --test-dry-run -Dorg.gradle.parallel=true -Dorg.gradle.caching=true
+      - name: Extract current test list
+        run: mkdir tmp; find . -type f -name TEST-*.xml | xargs -I{} bash -c "xmlstarlet sel -t -v '/testsuite/@name' '{}'; echo '{}' | sed 's#\./\(.*\)/build/test-results/.*# \1#'" | tee tmp/currentTests.list
+      - name: get unit test reports
+        uses: dawidd6/action-download-artifact@e7466d1a7587ed14867642c2ca74b5bcc1e19a2d
+        with:
+          branch: main
+          name_is_regexp: true
+          name: 'unit-.*-test-results'
+          path: tmp/junit-xml-reports-downloaded
+          if_no_artifact_found: true
+      - name: Split tests
+        run: .github/workflows/splitTestsByTime.sh tmp/junit-xml-reports-downloaded "tmp/junit-xml-reports-downloaded/unit-.*-test-results" "build/test-results" ${{env.total-runners}} ${{ matrix.runner_index }} > testList.txt
+      - name: Upload Timing
+        uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3
+        if: matrix.runner_index == 0
+        with:
+          name: unit-tests-timing
+          path: 'tmp/timing.tsv'
+      - name: Upload Lists
+        uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3
+        if: matrix.runner_index == 0
+        with:
+          name: unit-tests-lists
+          path: 'tmp/*.list'
+      - name: Upload gradle test tasks
+        uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3
+        with:
+          name: testList-${{ matrix.runner_index }}.txt
+          path: testList.txt
       - name: run unit tests
-        id: unitTest
-        run: ./gradlew $GRADLEW_UNIT_TEST_ARGS
+        run: cat testList.txt | xargs -P 1 ./gradlew -Dorg.gradle.parallel=true -Dorg.gradle.caching=true
       - name: Upload Unit Test Results
         uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3
         with:
-          name: unit-${{matrix.filename}}-test-results
+          name: unit-${{matrix.runner_index}}-test-results
           path: '**/test-results/**/TEST-*.xml'
   unittests-passed:
     name: "unittests-passed"
     runs-on: ubuntu-22.04
-    needs: [unitTests]
+    needs: [compile, unitTests]
     permissions:
       checks: write
       statuses: write

diff --git a/.github/workflows/splitTestsByTime.sh b/.github/workflows/splitTestsByTime.sh
@@ -15,11 +15,13 @@
 ##
 
 REPORTS_DIR="$1"
-SPLIT_COUNT=$2
-SPLIT_INDEX=$3
+REPORT_STRIP_PREFIX="$2"
+REPORT_STRIP_SUFFIX="$3"
+SPLIT_COUNT=$4
+SPLIT_INDEX=$5
 
 # extract tests time from Junit XML reports
-find "$REPORTS_DIR" -type f -name TEST-*.xml | xargs -I{} bash -c "xmlstarlet sel -t -v 'sum(//testcase/@time)' '{}'; echo '{}' | sed 's/.*TEST\-\(.*\)\.xml/ \1/'" > tmp/timing.tsv
+find "$REPORTS_DIR" -type f -name TEST-*.xml | xargs -I{} bash -c "xmlstarlet sel -t -v 'concat(sum(//testcase/@time), \" \", //testsuite/@name)' '{}'; echo '{}' | sed \"s#${REPORT_STRIP_PREFIX}/\(.*\)/${REPORT_STRIP_SUFFIX}.*# \1#\"" > tmp/timing.tsv
 
 # Sort times in descending order
 IFS=$'\n' sorted=($(sort -nr tmp/timing.tsv))
@@ -34,15 +36,18 @@ do
 	sums[$i]=0
 done
 
+echo -n '' > tmp/processedTests.list
+
 # add tests to groups trying to balance the sum of execution time of each group
 for line in "${sorted[@]}"; do
 	line_parts=( $line )
-	test_time=${line_parts[0]//./} # convert to millis
-	test_time=${test_time##0} # remove leading zeros
+	test_time=$( echo "${line_parts[0]} * 1000 / 1" | bc )  # convert to millis without decimals
 	test_name=${line_parts[1]}
+	module_dir=${line_parts[2]}
+	test_with_module="$test_name $module_dir"
 
   # Does the test still exists?
-  if grep -F -q --line-regexp "$test_name" tmp/currentTests.list
+  if grep -F -q --line-regexp "$test_with_module" tmp/currentTests.list
   then
     # Find index of min sum
     idx_min_sum=0
@@ -58,26 +63,60 @@ for line in "${sorted[@]}"; do
 
     # Add the test to the min sum list
     min_sum_tests=${tests[$idx_min_sum]}
-    tests[$idx_min_sum]="${min_sum_tests}${test_name},"
+    tests[$idx_min_sum]="${min_sum_tests}${test_with_module},"
 
     # Update the sums
     ((sums[idx_min_sum]+=test_time))
 
-    echo "$test_name" >> tmp/processedTests.list
+    echo "$test_with_module" >> tmp/processedTests.list
   fi
 done
 
 # Any new test?
 grep -F --line-regexp -v -f tmp/processedTests.list tmp/currentTests.list > tmp/newTests.list
 idx_new_test=0
-while read -r new_test_name
+while read -r new_test_with_module
 do
-  idx_group=$(( idx_new_test % SPLIT_COUNT ))
-  group=${tests[$idx_group]}
-  tests[$idx_group]="${group}${new_test_name},"
-  idx_new_test=$(( idx_new_test + 1 ))
+	idx_group=$(( idx_new_test % SPLIT_COUNT ))
+	group=${tests[$idx_group]}
+	tests[$idx_group]="${group}${new_test_with_module},"
+	idx_new_test=$(( idx_new_test + 1 ))
 done < tmp/newTests.list
 
+# remove last comma
+for ((i=0; i<SPLIT_COUNT; i++))
+do
+  test_list=${tests[$i]%,}
+  tests[$i]="$test_list"
+done
+
+
+# group tests by module
+module_list=( $( echo "${tests[$SPLIT_INDEX]}" | tr "," "\n" | awk '{print $2}' | sort -u ) )
+
+declare -A group_by_module
+for module_dir in "${module_list[@]}"
+do
+	group_by_module[$module_dir]=""
+done
+
+IFS="," test_list=( ${tests[$SPLIT_INDEX]} )
+unset IFS
+
+for line in "${test_list[@]}"
+do
+	line_parts=( $line )
+	test_name=${line_parts[0]}
+	module_dir=${line_parts[1]}
+
+	module_group=${group_by_module[$module_dir]}
+	group_by_module[$module_dir]="$module_group$test_name "
+done
 
 # return the requests index, without quotes to drop the last trailing space
-echo ${tests[$SPLIT_INDEX]//,/ }
+for module_dir in "${module_list[@]}"
+do
+	module_test_task=":${module_dir//\//:}:test"
+	module_tests=$( echo "${group_by_module[$module_dir]% }" | sed -e 's/^\| / --tests /g' )
+	echo "$module_test_task $module_tests"
+done
diff --git a/acceptance-tests/tests/build.gradle b/acceptance-tests/tests/build.gradle
@@ -258,32 +258,3 @@ task acceptanceTestPermissioning(type: Test) {
 
   doFirst { mkdir "${buildDir}/jvmErrorLogs" }
 }
-
-// temporary solution to get a list of tests
-// Gradle >8.3 has a supported test dry-run option to achieve the same result
-task listAcceptanceTestNotPrivacy {
-  doLast {
-    def testExecutionSpec = tasks.getByName("acceptanceTestNotPrivacy") as Test
-
-    def processor = new org.gradle.api.internal.tasks.testing.TestClassProcessor() {
-        void startProcessing(org.gradle.api.internal.tasks.testing.TestResultProcessor processor) {}
-        void stop() {}
-        void stopNow() {}
-
-        void processTestClass(org.gradle.api.internal.tasks.testing.TestClassRunInfo info) {
-          def splitName = info.getTestClassName().split("\\.");
-          def testClassName = splitName[splitName.length-1];
-          if(testClassName.endsWith("Test") && !testClassName.startsWith("Abstract")) {
-            println(info.getTestClassName())
-          }
-        }
-      }
-
-    def detector = new org.gradle.api.internal.tasks.testing.detection.DefaultTestClassScanner(testExecutionSpec.getCandidateClassFiles(), testExecutionSpec.getTestFramework().getDetector()?.tap {
-      setTestClasses(testExecutionSpec.getTestClassesDirs().getFiles())
-      setTestClasspath(Collections.unmodifiableSet(testExecutionSpec.getClasspath().getFiles()))
-    }, processor)
-
-    detector.run()
-  }
-}
diff --git a/ethereum/api/build.gradle b/ethereum/api/build.gradle
@@ -168,29 +168,3 @@ tasks.register('generateTestBlockchain') {
   }
 }
 test.dependsOn(generateTestBlockchain)
-/*
- Utility tasks used to separate out long running suites of tests so they can be parallelized in CI
- */
-tasks.register("testBonsai", Test) {
-  useJUnitPlatform()
-  filter {
-    includeTestsMatching("org.hyperledger.besu.ethereum.api.jsonrpc.bonsai.*")
-  }
-  dependsOn(generateTestBlockchain)
-}
-
-tasks.register("testForest", Test) {
-  useJUnitPlatform()
-  filter {
-    includeTestsMatching("org.hyperledger.besu.ethereum.api.jsonrpc.forest.*")
-  }
-  dependsOn(generateTestBlockchain)
-}
-
-tasks.register("testRemainder", Test) {
-  useJUnitPlatform()
-  filter {
-    excludeTestsMatching("org.hyperledger.besu.ethereum.api.jsonrpc.bonsai.*")
-    excludeTestsMatching("org.hyperledger.besu.ethereum.api.jsonrpc.forest.*")
-  }
-}