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.

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

Introduce @ConfigurationParametersResource for use with @Suite classes #3340

Open
2 tasks
robinjhector opened this issue Jun 5, 2023 · 8 comments
Open
2 tasks

Comments

@robinjhector
Copy link

robinjhector commented Jun 5, 2023

(This issue will describe my problems using JUnit together with another engine: Cucumber. But hopefully the proposed solution would increase JUnit Suite usability overall)

According to the documentation. the preferred way of running Cucumber, is via the JUnit Platform Suite.
I can set configuration parameters for the suite launcher discovery request, which then the underlaying engine can use, via @ConfigurationParameter, or @ConfigurationParameters.
It also defaults to parsing a junit-platform.properties file on classpath if found.

As long as I have set my configuration parameters as described above, the cucumber engine boots nicely, and everything works as expected.

However, when running Cucumber on the CLI, or via my IDE´s "run" functionality, Cucumber is not run via the the JUnit platform.
This makes configuring my tests to work with both JUnit and in standalone mode a bit tricky. Cucumber, in standalone mode, will try and look for a cucumber.properties file on classpath.

So I have a few options:

  1. Duplicate the junit-platform.properties file, and save it as cucumber.properties.
    (Sad, keeping two files in sync is always a pain)
  2. Keep my cucumber.properties file, and use annotations for the JUnit Suite (@ConfigurationParameter)
    (Same here, I will have some configuration in-code, and some in a properties file)
  3. Do neither, and use System properties
    (I dislike it because it requires environmental setup for new developers, and may cause system-system variances)

Instead, it would be super helpful if we could feed configuration parameters to the JUnit Platform Suite, from a properties file of choice. If so I could have a single properties file for both my standalone cucumber runs, and my platform suite runs.

Suggestion

My idea is to introduce a new annotation, like:

@ConfigurationParameterFile("myown.properties")

(Naming tbd..)

Which would work just like @ConfigurationParameter, by reading the file and setting the config parameters.
Annotating with both @ConfigurationParameterFile and @ConfigurationParameter should be allowed. Where as @ConfigurationParameter takes precedence for a given key-value.

Deliverables

  • New annotation for supplying configuration parameters via properties file.
  • Extend SuiteLauncherDiscoveryRequestBuilder to parse the properties file, as specified in the new annotation

I'd be happy to take a stab at implementing this via a PR.
Feedback always welcome. Hope my points came across clearly, otherwise please let me know!

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Jun 5, 2023

Fundamentally this problem is caused by a lack of first-class support for the JUnit Platform (#2849). In practice we are still dealing with JUnit 4 support with a translation layer bolted on for JUnit 5. This means that features such as selecting a line in a file for test execution using the file selector are simply not implemented by the popular IDEs and build systems.

If you happen to use InteliJ IDEA it might help to upvote IDEA-227508.

However, when running Cucumber on the CLI, or via my IDE´s "run" functionality, Cucumber is not run via the the JUnit platform.

This could also partially be resolved by the solutions outlined in cucumber/cucumber-jvm#2695.

I have no immediate objections to this proposal but I do have a mild preference towards a solution in Cucumber rather than one in JUnit 5. I would also very much welcome a PR for cucumber/cucumber-jvm#2695.

@sbrannen sbrannen changed the title JUnit-suite API configuration parameters from properties file Introduce @ConfigurationParametersFile for use with @Suite classes Jun 6, 2023
@sbrannen
Copy link
Member

sbrannen commented Jun 6, 2023

In general, I understand the desire to have such a feature.

It could be beneficial not only for use with @Suite classes but also when using the Launcher API programmatically.

For example, we could consider introducing @ConfigurationParametersFile as well as a new configurationParameters(Path) method in LauncherDiscoveryRequestBuilder as a complement to the existing configurationParameters(Map) method.

but I do have a mild preference towards a solution in Cucumber rather than one in JUnit 5.

If a solution tailored to Cucumber can be implemented, that would likely be ideal.

However, as stated above, I think having the ability to provide a "file" as a source for configuration parameters could be beneficial and would help alleviate duplication of configuration.

I would also very much welcome a PR in that direction.

You mean a PR for the Cucumber project?

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Jun 6, 2023

You mean a PR for the Cucumber project?

Yes. Edited for clarity.

However, as stated above, I think having the ability to provide a "file" as a source for configuration parameters could be beneficial and would help alleviate duplication of configuration.

Do you think that there are use cases for file based configuration outside of this specific example? If Cucumber was not involved, I reckon most of these hypothetical use cases could be addressed by using @ConfigurationParameter as a meta-annotation.

That said, again, I don't have any immediate objection to this feature proposal on its own. It would in fact smooth over a problem that Cucumber users have. Though do think that it solves the underlying problem (a lack of support) in the wrong location.

@marcphilipp
Copy link
Member

Team decision: Introduce @ConfigurationParametersResource and LauncherDiscoveryRequestBuilder.configurationParametersResource that allow specifying multiple properties files as classpath resources with last-one-wins semantics.

@marcphilipp marcphilipp added this to the 5.11 M1 milestone Jun 7, 2023
@marcphilipp
Copy link
Member

@robinjhector Would you be interested in submitting a PR for this?

@robinjhector
Copy link
Author

@robinjhector Would you be interested in submitting a PR for this?

Sure thing! I'll give it a go :)

robinjhector added a commit to robinjhector/junit5 that referenced this issue Jun 8, 2023
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
robinjhector added a commit to robinjhector/junit5 that referenced this issue Jun 8, 2023
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
robinjhector added a commit to robinjhector/junit5 that referenced this issue Jun 8, 2023
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
@robinjhector
Copy link
Author

I made a PR: #3345

@sbrannen
Copy link
Member

sbrannen commented Jun 8, 2023

Hi @mpkorstanje,

Yes. Edited for clarity.

Thanks

Do you think that there are use cases for file based configuration outside of this specific example?

Yes. Teams may have more than one "test" task in their build -- for example, one for unit tests and one for integration tests, etc.

In such scenarios, it is impossible to have multiple junit-platform.properties files for each "test" task. Having a way to specify custom properties files would address that shortcoming.

Furthermore, with this feature one could still have a core junit-platform.properties file with additional suite-specific properties that override values in junit-platform.properties.

@sbrannen sbrannen changed the title Introduce @ConfigurationParametersFile for use with @Suite classes Introduce @ConfigurationParametersResource for use with @Suite classes Jun 9, 2023
@marcphilipp marcphilipp modified the milestones: 5.11 M1, 5.12 Feb 2, 2024
@marcphilipp marcphilipp modified the milestones: 5.11 M2, 5.11 M3 May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants