-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add support for additional configuration parameters resources #3345
Add support for additional configuration parameters resources #3345
Conversation
Question: I seem to have forgot about the picocli console runner. Should I add new picocli options for selecting a config resource file? ( |
Yes, that sounds like a good idea. Though maybe |
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.
Thanks for submitting the PR.
I requested a few minor changes.
...launcher/src/main/java/org/junit/platform/launcher/core/LauncherDiscoveryRequestBuilder.java
Outdated
Show resolved
Hide resolved
...rm-suite-api/src/main/java/org/junit/platform/suite/api/ConfigurationParametersResource.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/junit/platform/suite/commons/SuiteLauncherDiscoveryRequestBuilderTests.java
Outdated
Show resolved
Hide resolved
...rm-suite-api/src/main/java/org/junit/platform/suite/api/ConfigurationParametersResource.java
Outdated
Show resolved
Hide resolved
Or just make
Edit: Nevermind, I don't think it applies to this anyway. The launcher request builder is used from here, so the logic will be the same regardless. |
628a605
to
1d12ffc
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
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.
This is coming along nicely. 👍
I went ahead and reviewed the status quo and added a few comments.
Thank you for your efforts!
documentation/src/docs/asciidoc/user-guide/advanced-topics/junit-platform-reporting.adoc
Outdated
Show resolved
Hide resolved
...rm-suite-api/src/main/java/org/junit/platform/suite/api/ConfigurationParametersResource.java
Outdated
Show resolved
Hide resolved
...ons/src/main/java/org/junit/platform/suite/commons/SuiteLauncherDiscoveryRequestBuilder.java
Outdated
Show resolved
Hide resolved
...-platform-console/src/main/java/org/junit/platform/console/options/TestDiscoveryOptions.java
Outdated
Show resolved
Hide resolved
...form-console/src/main/java/org/junit/platform/console/options/TestDiscoveryOptionsMixin.java
Outdated
Show resolved
Hide resolved
junit-platform-runner/src/main/java/org/junit/platform/runner/JUnitPlatform.java
Outdated
Show resolved
Hide resolved
If you would like us to be able to process this pull request, please provide the requested information or make the requested changes. If the information is not provided or the requested changes are not made within the next 3 weeks, we will be unable to proceed and this pull request will be closed. |
Closing due to lack of requested feedback. If you would like to proceed with your contribution, please provide the requested information or make the requested changes, and we will re-open this pull request. |
Hi @robinjhector, I realize quite a bit of time has passed since you originally worked on this, but do you think you'll find time to make the suggested changes and complete this PR? If not, I imagine that someone in the team can pick it up from here. Please let us know how you'd like to proceed. Thanks |
This commit adds support for specifying a properties file, for use with configuration parameters. - Added a new field configurationParametersResources of type List<String> to store the configuration parameters resources. - Added new methods configurationParametersResource(String propertiesFile) and configurationParametersResources(List<String> propertiesFiles) to add configuration parameters resources to the request builder. - Updated the buildLauncherConfigurationParameters() method to include the configurationParametersResources in the Builder instance. Related to issue: junit-team#3340
Adjusted annotation parameter name to `value()`. Scrapped duplicate configurationParametersResources method on discovery request builder. (Is now var-args)
228d6cb
to
a219358
Compare
a219358
to
07b0921
Compare
Hey @sbrannen ! Thanks for getting back to me, I have rebased and adjusted the code + documentation after your review. Sorry about the delay! |
991729b
to
17b1308
Compare
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.
Looking good! I'll take it from here.
...form-console/src/main/java/org/junit/platform/console/options/TestDiscoveryOptionsMixin.java
Outdated
Show resolved
Hide resolved
...form-console/src/main/java/org/junit/platform/console/options/TestDiscoveryOptionsMixin.java
Outdated
Show resolved
Hide resolved
...m-suite-api/src/main/java/org/junit/platform/suite/api/ConfigurationParametersResources.java
Outdated
Show resolved
Hide resolved
@robinjhector Thank you for your contribution! 👍 |
Overview
This commit adds support for specifying a properties file, for use with configuration parameters.
Related to issue: #3340
Open questions:
The@API
annotation values, what should they be? Are they experimental or set to STABLE at once? Which version nr?I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@API
annotations