From 380192c249e9c6bebaf73c65441d84e5a48dd6ab Mon Sep 17 00:00:00 2001 From: David Arthur Date: Fri, 6 Sep 2024 17:04:53 -0400 Subject: [PATCH 01/13] Fail the whole pipeline if junit step times out. --- .github/scripts/junit.py | 16 ++++++++++++++-- .github/workflows/build.yml | 6 +++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.github/scripts/junit.py b/.github/scripts/junit.py index 1850c0c2e8a17..bb6dc33cbfb33 100644 --- a/.github/scripts/junit.py +++ b/.github/scripts/junit.py @@ -144,7 +144,10 @@ def pretty_time_duration(seconds: float) -> str: required=False, default="build/junit-xml/**/*.xml", help="Path to XML files. Glob patterns are supported.") - + parser.add_argument("--done-file", + required=False, + default="", + help="A file that signals the test suite was completed") if not os.getenv("GITHUB_WORKSPACE"): print("This script is intended to by run by GitHub Actions.") exit(1) @@ -153,7 +156,7 @@ def pretty_time_duration(seconds: float) -> str: reports = glob(pathname=args.path, recursive=True) logger.debug(f"Found {len(reports)} JUnit results") - workspace_path = get_env("GITHUB_WORKSPACE") # e.g., /home/runner/work/apache/kafka + workspace_path = get_env("GITHUB_WORKSPACE") # e.g., /home/runner/work/apache/kafka total_file_count = 0 total_run = 0 # All test runs according to @@ -248,6 +251,15 @@ def pretty_time_duration(seconds: float) -> str: print(f"| {row_joined} |") print("\n") + + # Print special message if these are partial results + if args.done_file: + if not os.path.exists(args.done_file): + logger.debug(f"Did not find done file '{args.done_file}'. These are partial results!") + logger.debug(summary) + logger.debug("Failing this step because done file was missing.") + exit(1) + logger.debug(summary) if total_failures > 0: logger.debug(f"Failing this step due to {total_failures} test failures") diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d6da2a94fa5b4..b0a70c393a178 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -110,14 +110,15 @@ jobs: timeout-minutes: 180 # 3 hours continue-on-error: true run: | + rm -f build/junit-xml/done ./gradlew --build-cache --scan --continue \ -PtestLoggingEvents=started,passed,skipped,failed \ -PmaxParallelForks=2 \ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \ -PcommitId=xxxxxxxxxxxxxxxx \ test + touch build/junit-xml/done - name: Archive JUnit reports - if: always() uses: actions/upload-artifact@v4 id: junit-upload-artifact with: @@ -126,8 +127,7 @@ jobs: **/build/reports/tests/test/* if-no-files-found: ignore - name: Parse JUnit tests - if: always() - run: python .github/scripts/junit.py >> $GITHUB_STEP_SUMMARY + run: python .github/scripts/junit.py --done-file build/junit-xml/done >> $GITHUB_STEP_SUMMARY env: GITHUB_WORKSPACE: ${{ github.workspace }} REPORT_URL: ${{ steps.junit-upload-artifact.outputs.artifact-url }} From 4c726f8c98fe6557692f2f652e46db60c71d1bae Mon Sep 17 00:00:00 2001 From: David Arthur Date: Fri, 6 Sep 2024 20:53:03 -0400 Subject: [PATCH 02/13] Use Gradle to create done file --- .github/workflows/build.yml | 2 -- build.gradle | 11 +++++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b0a70c393a178..d0b798b2379f9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -110,14 +110,12 @@ jobs: timeout-minutes: 180 # 3 hours continue-on-error: true run: | - rm -f build/junit-xml/done ./gradlew --build-cache --scan --continue \ -PtestLoggingEvents=started,passed,skipped,failed \ -PmaxParallelForks=2 \ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \ -PcommitId=xxxxxxxxxxxxxxxx \ test - touch build/junit-xml/done - name: Archive JUnit reports uses: actions/upload-artifact@v4 id: junit-upload-artifact diff --git a/build.gradle b/build.gradle index 36df0b271c10a..6fc0a359ba2c9 100644 --- a/build.gradle +++ b/build.gradle @@ -824,6 +824,17 @@ gradle.taskGraph.whenReady { taskGraph -> } } +tasks.named("test") { + if (System.getenv('GITHUB_ACTIONS') != null) { + finalizedBy { + doLast { + def doneFile = rootProject.layout.buildDirectory.dir("junit-xml/done").get().asFile + doneFile.createNewFile() + } + } + } +} + def fineTuneEclipseClasspathFile(eclipse, project) { eclipse.classpath.file { beforeMerged { cp -> From 9ab5c7475ebb757b851c9bd06ada59a1954bfb79 Mon Sep 17 00:00:00 2001 From: David Arthur Date: Fri, 6 Sep 2024 21:23:46 -0400 Subject: [PATCH 03/13] fix finalizedBy --- build.gradle | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/build.gradle b/build.gradle index 6fc0a359ba2c9..d8b22d6b2c434 100644 --- a/build.gradle +++ b/build.gradle @@ -824,14 +824,16 @@ gradle.taskGraph.whenReady { taskGraph -> } } +task createDoneFile() { + doLast { + def doneFile = rootProject.layout.buildDirectory.dir("junit-xml/done").get().asFile + doneFile.createNewFile() + } +} + tasks.named("test") { if (System.getenv('GITHUB_ACTIONS') != null) { - finalizedBy { - doLast { - def doneFile = rootProject.layout.buildDirectory.dir("junit-xml/done").get().asFile - doneFile.createNewFile() - } - } + finalizedBy "createDoneFile" } } From 36e0be0a2875650445261425bd302d233b2d5239 Mon Sep 17 00:00:00 2001 From: David Arthur Date: Fri, 6 Sep 2024 23:55:29 -0400 Subject: [PATCH 04/13] create file, not dir --- .github/workflows/build.yml | 3 +++ build.gradle | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d0b798b2379f9..9ee43dfdc841d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -116,6 +116,9 @@ jobs: -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \ -PcommitId=xxxxxxxxxxxxxxxx \ test + - name: List files + run: + find . - name: Archive JUnit reports uses: actions/upload-artifact@v4 id: junit-upload-artifact diff --git a/build.gradle b/build.gradle index d8b22d6b2c434..5c9275a948d7d 100644 --- a/build.gradle +++ b/build.gradle @@ -826,7 +826,7 @@ gradle.taskGraph.whenReady { taskGraph -> task createDoneFile() { doLast { - def doneFile = rootProject.layout.buildDirectory.dir("junit-xml/done").get().asFile + def doneFile = rootProject.layout.buildDirectory.file("junit-xml/done").get().asFile doneFile.createNewFile() } } From 6f18f88b5990593700166dec3246325d8ce8dfa1 Mon Sep 17 00:00:00 2001 From: David Arthur Date: Sat, 7 Sep 2024 09:44:07 -0400 Subject: [PATCH 05/13] Explicitly call createDoneFile from GH --- .github/workflows/build.yml | 2 +- build.gradle | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9ee43dfdc841d..8baa53b4210dc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -115,7 +115,7 @@ jobs: -PmaxParallelForks=2 \ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \ -PcommitId=xxxxxxxxxxxxxxxx \ - test + test createDoneFile - name: List files run: find . diff --git a/build.gradle b/build.gradle index 5c9275a948d7d..6d8b5cae546d6 100644 --- a/build.gradle +++ b/build.gradle @@ -824,18 +824,19 @@ gradle.taskGraph.whenReady { taskGraph -> } } -task createDoneFile() { +tasks.register("createDoneFile") { + def doneFile = rootProject.layout.buildDirectory.file("junit-xml/done") + outputs.file doneFile + outputs.cacheIf { false } + doLast { - def doneFile = rootProject.layout.buildDirectory.file("junit-xml/done").get().asFile - doneFile.createNewFile() + println "Creating done file for GitHub Actions workflow." + def file = doneFile.get().asFile + file.parentFile.mkdirs() + file.createNewFile() } } -tasks.named("test") { - if (System.getenv('GITHUB_ACTIONS') != null) { - finalizedBy "createDoneFile" - } -} def fineTuneEclipseClasspathFile(eclipse, project) { eclipse.classpath.file { From 3336fc072e66b41565402a599439e8679c45002b Mon Sep 17 00:00:00 2001 From: David Arthur Date: Sat, 7 Sep 2024 11:34:05 -0400 Subject: [PATCH 06/13] change approach --- .github/scripts/junit.py | 18 ++++++------------ .github/workflows/build.yml | 16 ++++++++-------- build.gradle | 14 -------------- 3 files changed, 14 insertions(+), 34 deletions(-) diff --git a/.github/scripts/junit.py b/.github/scripts/junit.py index bb6dc33cbfb33..bcded558db978 100644 --- a/.github/scripts/junit.py +++ b/.github/scripts/junit.py @@ -144,10 +144,6 @@ def pretty_time_duration(seconds: float) -> str: required=False, default="build/junit-xml/**/*.xml", help="Path to XML files. Glob patterns are supported.") - parser.add_argument("--done-file", - required=False, - default="", - help="A file that signals the test suite was completed") if not os.getenv("GITHUB_WORKSPACE"): print("This script is intended to by run by GitHub Actions.") exit(1) @@ -251,14 +247,12 @@ def pretty_time_duration(seconds: float) -> str: print(f"| {row_joined} |") print("\n") - - # Print special message if these are partial results - if args.done_file: - if not os.path.exists(args.done_file): - logger.debug(f"Did not find done file '{args.done_file}'. These are partial results!") - logger.debug(summary) - logger.debug("Failing this step because done file was missing.") - exit(1) + # Print special message if there was a timeout + if get_env("GRADLE_EXIT_CODE") == 124: + logger.debug(f"Gradle command timed out. These are partial results!") + logger.debug(summary) + logger.debug("Failing this step because the tests timed out.") + exit(1) logger.debug(summary) if total_failures > 0: diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8baa53b4210dc..0be2c0b14bb3d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -107,18 +107,17 @@ jobs: # --scan: Attempt to publish build scans in PRs. This will only work on PRs from apache/kafka, not public forks. # --continue: Keep running even if a test fails # -PcommitId Prevent the Git SHA being written into the jar files (which breaks caching) - timeout-minutes: 180 # 3 hours - continue-on-error: true + id: junit-test run: | - ./gradlew --build-cache --scan --continue \ + set +e + timeout 5 ./gradlew --build-cache --scan --continue \ -PtestLoggingEvents=started,passed,skipped,failed \ -PmaxParallelForks=2 \ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \ -PcommitId=xxxxxxxxxxxxxxxx \ - test createDoneFile - - name: List files - run: - find . + test + exitcode="$?" + echo "exitcode=$exitcode" >> $GITHUB_OUTPUT - name: Archive JUnit reports uses: actions/upload-artifact@v4 id: junit-upload-artifact @@ -128,7 +127,8 @@ jobs: **/build/reports/tests/test/* if-no-files-found: ignore - name: Parse JUnit tests - run: python .github/scripts/junit.py --done-file build/junit-xml/done >> $GITHUB_STEP_SUMMARY + run: python .github/scripts/junit.py >> $GITHUB_STEP_SUMMARY env: GITHUB_WORKSPACE: ${{ github.workspace }} REPORT_URL: ${{ steps.junit-upload-artifact.outputs.artifact-url }} + GRADLE_EXIT_CODE: ${{ steps.junit-test.outputs.exitcode }} \ No newline at end of file diff --git a/build.gradle b/build.gradle index 6d8b5cae546d6..36df0b271c10a 100644 --- a/build.gradle +++ b/build.gradle @@ -824,20 +824,6 @@ gradle.taskGraph.whenReady { taskGraph -> } } -tasks.register("createDoneFile") { - def doneFile = rootProject.layout.buildDirectory.file("junit-xml/done") - outputs.file doneFile - outputs.cacheIf { false } - - doLast { - println "Creating done file for GitHub Actions workflow." - def file = doneFile.get().asFile - file.parentFile.mkdirs() - file.createNewFile() - } -} - - def fineTuneEclipseClasspathFile(eclipse, project) { eclipse.classpath.file { beforeMerged { cp -> From c3135a7eda85162facba2f7f7374df72f2f5b23a Mon Sep 17 00:00:00 2001 From: David Arthur Date: Sat, 7 Sep 2024 11:38:08 -0400 Subject: [PATCH 07/13] exit code as string --- .github/scripts/junit.py | 5 +++-- .github/workflows/build.yml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/scripts/junit.py b/.github/scripts/junit.py index bcded558db978..d922dadb850a4 100644 --- a/.github/scripts/junit.py +++ b/.github/scripts/junit.py @@ -144,6 +144,7 @@ def pretty_time_duration(seconds: float) -> str: required=False, default="build/junit-xml/**/*.xml", help="Path to XML files. Glob patterns are supported.") + if not os.getenv("GITHUB_WORKSPACE"): print("This script is intended to by run by GitHub Actions.") exit(1) @@ -152,7 +153,7 @@ def pretty_time_duration(seconds: float) -> str: reports = glob(pathname=args.path, recursive=True) logger.debug(f"Found {len(reports)} JUnit results") - workspace_path = get_env("GITHUB_WORKSPACE") # e.g., /home/runner/work/apache/kafka + workspace_path = get_env("GITHUB_WORKSPACE") # e.g., /home/runner/work/apache/kafka total_file_count = 0 total_run = 0 # All test runs according to @@ -248,7 +249,7 @@ def pretty_time_duration(seconds: float) -> str: print("\n") # Print special message if there was a timeout - if get_env("GRADLE_EXIT_CODE") == 124: + if get_env("GRADLE_EXIT_CODE") == "124": logger.debug(f"Gradle command timed out. These are partial results!") logger.debug(summary) logger.debug("Failing this step because the tests timed out.") diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0be2c0b14bb3d..79bd463ff4963 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -131,4 +131,4 @@ jobs: env: GITHUB_WORKSPACE: ${{ github.workspace }} REPORT_URL: ${{ steps.junit-upload-artifact.outputs.artifact-url }} - GRADLE_EXIT_CODE: ${{ steps.junit-test.outputs.exitcode }} \ No newline at end of file + GRADLE_EXIT_CODE: ${{ steps.junit-test.outputs.exitcode }} From 961236ded2e8315f96a57ef8742027449b293856 Mon Sep 17 00:00:00 2001 From: David Arthur Date: Sat, 7 Sep 2024 11:47:56 -0400 Subject: [PATCH 08/13] add type to get_env --- .github/scripts/junit.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/scripts/junit.py b/.github/scripts/junit.py index d922dadb850a4..da68d584e396c 100644 --- a/.github/scripts/junit.py +++ b/.github/scripts/junit.py @@ -37,10 +37,14 @@ SKIPPED = "SKIPPED 🙈" -def get_env(key: str) -> str: +def get_env(key: str, fn = str) -> Optional: value = os.getenv(key) - logger.debug(f"Read env {key}: {value}") - return value + if value is None: + logger.debug(f"Could not find env {key}") + return None + else: + logger.debug(f"Read env {key}: {value}") + return fn(value) @dataclasses.dataclass @@ -249,7 +253,7 @@ def pretty_time_duration(seconds: float) -> str: print("\n") # Print special message if there was a timeout - if get_env("GRADLE_EXIT_CODE") == "124": + if get_env("GRADLE_EXIT_CODE", int) == 124: logger.debug(f"Gradle command timed out. These are partial results!") logger.debug(summary) logger.debug("Failing this step because the tests timed out.") From 6160c8465c2b5ef1589af363b9e20346a75d1bc7 Mon Sep 17 00:00:00 2001 From: David Arthur Date: Sat, 7 Sep 2024 11:49:58 -0400 Subject: [PATCH 09/13] add more exit code handling --- .github/scripts/junit.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/.github/scripts/junit.py b/.github/scripts/junit.py index da68d584e396c..2c63efe0be833 100644 --- a/.github/scripts/junit.py +++ b/.github/scripts/junit.py @@ -253,18 +253,22 @@ def pretty_time_duration(seconds: float) -> str: print("\n") # Print special message if there was a timeout - if get_env("GRADLE_EXIT_CODE", int) == 124: + exit_code = get_env("GRADLE_EXIT_CODE", int) + if exit_code == 124: logger.debug(f"Gradle command timed out. These are partial results!") logger.debug(summary) logger.debug("Failing this step because the tests timed out.") exit(1) - - logger.debug(summary) - if total_failures > 0: - logger.debug(f"Failing this step due to {total_failures} test failures") - exit(1) - elif total_errors > 0: - logger.debug(f"Failing this step due to {total_errors} test errors") - exit(1) + elif exit_code in (0, 1): + logger.debug(summary) + if total_failures > 0: + logger.debug(f"Failing this step due to {total_failures} test failures") + exit(1) + elif total_errors > 0: + logger.debug(f"Failing this step due to {total_errors} test errors") + exit(1) + else: + exit(0) else: - exit(0) + logger.debug(f"Gradle had unexpected exit code {exit_code}. Failing this step") + exit(1) From fa4466d216d8fa81db1108a040469a05b5935fe5 Mon Sep 17 00:00:00 2001 From: David Arthur Date: Sat, 7 Sep 2024 11:53:01 -0400 Subject: [PATCH 10/13] fix timeout --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 79bd463ff4963..4a89af1afd9e7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -110,7 +110,7 @@ jobs: id: junit-test run: | set +e - timeout 5 ./gradlew --build-cache --scan --continue \ + timeout 180 ./gradlew --build-cache --scan --continue \ -PtestLoggingEvents=started,passed,skipped,failed \ -PmaxParallelForks=2 \ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \ From 0a0142bc9cade45f04b778dcf2ab7f67e84c4be8 Mon Sep 17 00:00:00 2001 From: David Arthur Date: Sat, 7 Sep 2024 11:59:58 -0400 Subject: [PATCH 11/13] add time unit suffix --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4a89af1afd9e7..a1a9442914826 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -110,7 +110,7 @@ jobs: id: junit-test run: | set +e - timeout 180 ./gradlew --build-cache --scan --continue \ + timeout 180m ./gradlew --build-cache --scan --continue \ -PtestLoggingEvents=started,passed,skipped,failed \ -PmaxParallelForks=2 \ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \ From 019a4e92b4cb8c29d664db88cdf3180cbc3b460c Mon Sep 17 00:00:00 2001 From: David Arthur Date: Sat, 7 Sep 2024 14:54:13 -0400 Subject: [PATCH 12/13] testing 10m timeout --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a1a9442914826..d054720d4dce3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -110,7 +110,7 @@ jobs: id: junit-test run: | set +e - timeout 180m ./gradlew --build-cache --scan --continue \ + timeout 10m ./gradlew --build-cache --scan --continue \ -PtestLoggingEvents=started,passed,skipped,failed \ -PmaxParallelForks=2 \ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \ From 56fa051a72098f852e12c14835f00a666e4cd064 Mon Sep 17 00:00:00 2001 From: David Arthur Date: Sat, 7 Sep 2024 15:10:31 -0400 Subject: [PATCH 13/13] Revert "testing 10m timeout" This reverts commit 019a4e92b4cb8c29d664db88cdf3180cbc3b460c. --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d054720d4dce3..a1a9442914826 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -110,7 +110,7 @@ jobs: id: junit-test run: | set +e - timeout 10m ./gradlew --build-cache --scan --continue \ + timeout 180m ./gradlew --build-cache --scan --continue \ -PtestLoggingEvents=started,passed,skipped,failed \ -PmaxParallelForks=2 \ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \