junit5 icon indicating copy to clipboard operation
junit5 copied to clipboard

Add support for configuration parameters resources

Open robinjhector opened this issue 2 years ago • 6 comments

Overview

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: #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

robinjhector avatar Jun 08 '23 12:06 robinjhector

Question: I seem to have forgot about the picocli console runner. Should I add new picocli options for selecting a config resource file? (--config-resource perhaps?) in TestDiscoveryOptionsMixin.RuntimeConfigurationOptions

robinjhector avatar Jun 09 '23 07:06 robinjhector

Question: I seem to have forgot about the picocli console runner. Should I add new picocli options for selecting a config resource file? (--config-resource perhaps?) in TestDiscoveryOptionsMixin.RuntimeConfigurationOptions

Yes, that sounds like a good idea.

Though maybe --config-resources which accepts a comma-separated list of resource paths.

sbrannen avatar Jun 09 '23 08:06 sbrannen

Yes, that sounds like a good idea.

Though maybe --config-resources which accepts a comma-separated list of resource paths.

Or just make --config-resource repeatable, like the existing --configcli option? I think I would prefer that, instead of CSVing it.

~~Also; I couldn't help but notice that the CLI configuration parameters have a strict validation of unique key-value pairs (#1308). And am a bit confused how configuration resources should behave with that in mind.~~

~~I would prefer a similar approach as the suite annotations, eg. the explicit parameters take precedence over any property file based ones. Any suggestions or does that sound good?~~

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. I've updated the PR with a commit for new CLI options 👍

robinjhector avatar Jun 09 '23 09:06 robinjhector

Ping @sbrannen, do you have time to check my fixes to your requested changes?

robinjhector avatar Nov 01 '23 10:11 robinjhector

Hi @robinjhector,

Ping @sbrannen, do you have time to check my fixes to your requested changes?

Sorry for the belated reply.

#3340 is assigned to the 5.11 M3 milestone, so somebody within the core team will hopefully find time to follow up on your PR in the relatively near future.

sbrannen avatar May 28 '24 10:05 sbrannen

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.

github-actions[bot] avatar Jun 12 '24 01:06 github-actions[bot]

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.

github-actions[bot] avatar Jul 03 '24 01:07 github-actions[bot]

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

sbrannen avatar Jul 03 '24 09:07 sbrannen

Hey @sbrannen ! Thanks for getting back to me, I have rebased and adjusted the code + documentation after your review. Sorry about the delay!

robinjhector avatar Jul 03 '24 11:07 robinjhector

@robinjhector Thank you for your contribution! :+1:

marcphilipp avatar Jul 19 '24 13:07 marcphilipp