Skip to content

[ci] Add surefire fork timeouts to prevent CI hangs #6186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joewiz wants to merge 2 commits into from
Closed

[ci] Add surefire fork timeouts to prevent CI hangs #6186

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5de7278
[ci] Add surefire fork timeouts to prevent CI hangs
joewiz Mar 26, 2026
fda23d2
[ci] Add wagon timeouts to Maven Build; run license check offline
joewiz Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ jobs:
run: ${{ steps.install-mvnd.outputs.mvnd-dir }}/mvnd -V -B --no-transfer-progress test -DtrimStackTrace=false -D'dependency-check.skip' -D'license.skip' -D'mvnd.maxLostKeepAlive=6000'
- name: Maven Integration Tests
if: matrix.test-type == 'integration'
timeout-minutes: 45
timeout-minutes: 30
Copy link
Copy Markdown
Contributor

@duncdrum duncdrum Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen 37min successful runs on CI. I would suggest to be more generous with the CI timeout until we can establish a new baseline of successful CI runs, and then lower the workflow timeouts accordingly.

Copy link
Copy Markdown
Member Author

@joewiz joewiz Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[This response was co-authored with Claude Code. -Joe]

Good to know — reverting to 45 minutes. The 30-minute reduction was premature given you've seen legitimate 37-minute runs.

run: ${{ steps.install-mvnd.outputs.mvnd-dir }}/mvnd -V -B --no-transfer-progress verify -DskipUnitTests=true -DtrimStackTrace=false -D'dependency-check.skip' -D'license.skip' -D'mvnd.maxLostKeepAlive=6000'
- name: Javadoc (ubuntu unit only)
if: matrix.os == 'ubuntu-latest' && matrix.test-type == 'unit'
Expand Down
8 changes: 8 additions & 0 deletions exist-parent/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,10 @@
However it can make it hard to diagnose problems if tests leak state; If you experience
such a problem you may want to set it to `false` whilst debugging -->
<reuseForks>true</reuseForks>
<!-- Kill forked JVMs that hang (e.g. DeadlockIT, MoveResourceTest).
10 minutes is generous; clean runs complete in ~3.5 minutes. -->
<forkedProcessTimeoutInSeconds>600</forkedProcessTimeoutInSeconds>
Copy link
Copy Markdown
Contributor

@duncdrum duncdrum Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea with #6045 was to move slow tests into the IT phase. Unit test should be faster, so I would expect the timeouts here to be lower compared to failsafe tests. If there are slow tests left in surefire, I would rather move them to IT.
Does surefire even fork a jvm? I thought this was a failsafe thing?

Copy link
Copy Markdown
Member Author

@joewiz joewiz Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[This response was co-authored with Claude Code. -Joe]

Surefire does fork here — forkCount=2C with reuseForks=true in exist-parent/pom.xml, so forkedProcessTimeoutInSeconds is applicable. That said, your point about lower timeouts for unit tests is well taken, and moving slow stragglers to the IT phase (#6045) is the right long-term answer. For now I'll remove the forkedProcessTimeoutInSeconds from surefire and keep it only in failsafe, where it has the clearest justification.

<forkedProcessExitTimeoutInSeconds>60</forkedProcessExitTimeoutInSeconds>
<argLine>@{jacocoArgLine} --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.ref=ALL-UNNAMED --add-opens=java.base/java.lang.reflect=ALL-UNNAMED -Dfile.encoding=${project.build.sourceEncoding}</argLine>
<systemPropertyVariables>
<user.country>UK</user.country>
Expand All @@ -863,6 +867,10 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<version>3.5.5</version>
<configuration>
<forkedProcessTimeoutInSeconds>600</forkedProcessTimeoutInSeconds>
<forkedProcessExitTimeoutInSeconds>60</forkedProcessExitTimeoutInSeconds>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
Expand Down
Loading