-
Notifications
You must be signed in to change notification settings - Fork 654
GH2160: Set application exit code #2161
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.
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
Conversation
@arturcic did I break some other tests? Not sure what to do there. |
I think we need some integration tests that reproduce the issue with the current behavior, then to have a fix that makes the test green again, what do you think? |
Is this failure actually expected given my change? |
If you point me in the right direction, I'll try to add an integration test. |
So it looks like those integration tests already check the exit code, and I changed the last one in this PR, since it flipped from zero to non-zero after this change. I think it's enough to have at least some tests that check the exit code. Is there something more specific you want to test for here? And I'm still not sure why the docker tests are failing. Any ideas? |
Nope, no idea, I think we can postpone this issue after the release of 5.2.0, what do you think? |
Sure. I'd rather have 5.2.0. 😛 |
@arturcic rebasing this PR after you released 5.2.0 seems to have fixed the docker tests. Kinda makes sense given the tests were failing with errors like:
|
Raw Cake logs from GitHub Actions captured before this change:
Raw Cake logs from GitHub Actions captured after this change:
As shown, the obscure exception reported by Cake changed from:
to the more relevant:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
thank you so much for your contributions 👍 @gitfool |
🎉 This issue has been resolved in version 5.2.1 🎉 Your GitReleaseManager bot 📦🚀 |
Fixes #2160.