-
Notifications
You must be signed in to change notification settings - Fork 41.2k
Improve test coverage of launch script #20335
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
Comments
@wilkinsona Hello! I want to start to contribute and fix this issue. Do you mind if I start working on it? |
Not at all. Thank you very much for the offer. |
@wilkinsona - I would also like to give this a shot. Without a ton of context, are you saying that issues #18540 and #19716 cannot be addressed until test coverage of launch script is improved. Is there a test that needs additional test cases? Is this RepackagerTests.java? What tests do you see as being missing? |
Thanks for the offer, @mikesmithson, but @aivinog1 is already looking at this one. |
@wilkinsona Hello. I have a question. Do we need in this task implement tests that start an application as systemd service? I don't see such tests in the project right now. And I stuck in writing a test that verifies that |
Testing as a systemd service would be nice in the longer term, but I don't think it's of most importance. I would focus on launching the jar directly as a root and non-root user.
The behaviour of |
@wilkinsona Hello, I've developed some tests and I want to understand, do I do them properly. How can you review them? I can send a Github link on my commit (by the way here it is: aivinog1@a1a97bb). Or I can open a new PR. Thanks. |
@aivinog1 They look good to me. Thank you very much. Please do open a PR when you have a moment and then we can go through a proper review. |
Closing in favor of PR #21388 |
Fixing #18540 and #19716 is higher risk that we would like as the changes that appear to be necessary aren't covered particularly well by the launch script's integration tests. For example the tests focus on starting an app via init.d and the problem reported in #19716 happens when executing the jar directly.
The text was updated successfully, but these errors were encountered: