Skip to content
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

Jump to bottom

Issue #8088 Add STOP.EXIT System property to configure ShutdownMonitor.exitVm #8133

Merged
merged 2 commits into from Jun 8, 2022
Merged

Issue #8088 Add STOP.EXIT System property to configure ShutdownMonitor.exitVm #8133

merged 2 commits into from Jun 8, 2022

Conversation

janbartel
Copy link
Contributor

Cherry pick fix from 9.4.x to 10.0.x

@janbartel
Issue #8088 Add STOP.EXIT System property to configure ShutdownMonito…
183b3c0 
…r.exitVm (#8089)

* Issue #8088 Add STOP.EXIT System property to configure ShutdownMonitor.exitVM

* Ensure missing STOP.EXIT doesn't override default exitVm=true

* Disable another test
@janbartel janbartel self-assigned this Jun 7, 2022
@janbartel janbartel added this to In progress in Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 via automation Jun 7, 2022
@janbartel janbartel requested review from gregw and joakime June 7, 2022 04:06
@joakime joakime moved this from In progress to Review in progress in Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 Jun 7, 2022
@joakime
Copy link
Contributor

joakime commented Jun 7, 2022

This is not passing it's own tests.

[ERROR] Failures: 
[ERROR]   ShutdownMonitorTest.testNoExitSystemProperty:142 expected: <false> but was: <true>

@joakime joakime linked an issue Jun 7, 2022 that may be closed by this pull request
Add option to configure exitVm on ShutdownMonitor from System properties #8088
Closed
@janbartel
Copy link
Contributor Author

@joakime local checkout of the branch tests fine ... must be something different in the CI environment?

@janbartel
Copy link
Contributor Author

@joakime local checkout of the branch tests fine ... must be something different in the CI environment?

I think it is probably the execution test order on the CI environment. As the singleton might already have been constructed, setting the System.property won't change it. For now, I've disabled that test along with the others that are affected by the behaviour of the singleton constructor.

Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 automation moved this from Review in progress to Reviewer approved Jun 8, 2022
@joakime joakime added the Sponsored This issue affects a user with a commercial support agreement label Jun 8, 2022
@janbartel janbartel merged commit d725855 into jetty-10.0.x Jun 8, 2022
Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 automation moved this from Reviewer approved to Done Jun 8, 2022
@janbartel janbartel deleted the jetty-10.0.x-8088-shutdownmonitor-exitvm branch June 8, 2022 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joakime joakime joakime approved these changes

@gregw gregw Awaiting requested review from gregw

Assignees

@janbartel janbartel

Labels
Sponsored This issue affects a user with a commercial support agreement
Projects
No open projects
Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add option to configure exitVm on ShutdownMonitor from System properties
2 participants
@janbartel @joakime