Skip to content

Commit

Permalink
Review fixes
Browse files Browse the repository at this point in the history 
Adjusted annotation parameter name to `value()`.
Scrapped duplicate configurationParametersResources method on discovery request builder. (Is now var-args)
  • Loading branch information
@robinjhector
robinjhector committed Jun 9, 2023
1 parent fd45908 commit e5ff689
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 43 deletions.
4 changes: 3 additions & 1 deletion ...tform-console/src/main/java/org/junit/platform/console/tasks/DiscoveryRequestCreator.java
View file
Expand Up @@ -54,7 +54,9 @@ LauncherDiscoveryRequest toDiscoveryRequest(TestDiscoveryOptions options) {
requestBuilder.selectors(selectors);
addFilters(requestBuilder, options, selectors);
requestBuilder.configurationParameters(options.getConfigurationParameters());
requestBuilder.configurationParametersResources(options.getConfigurationParametersResources());
requestBuilder.configurationParametersResources(
options.getConfigurationParametersResources().toArray(new String[0])
);
return requestBuilder.build();
}

Expand Down
39 changes: 14 additions & 25 deletions ...ncher/src/main/java/org/junit/platform/launcher/core/LauncherDiscoveryRequestBuilder.java
View file
Expand Up @@ -10,15 +10,6 @@

package org.junit.platform.launcher.core;

import static org.apiguardian.api.API.Status.DEPRECATED;
import static org.apiguardian.api.API.Status.STABLE;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.apiguardian.api.API;
import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.commons.util.Preconditions;
Expand All @@ -33,6 +24,16 @@
import org.junit.platform.launcher.core.LauncherConfigurationParameters.Builder;
import org.junit.platform.launcher.listeners.discovery.LauncherDiscoveryListeners;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.apiguardian.api.API.Status.DEPRECATED;
import static org.apiguardian.api.API.Status.STABLE;

/**
* The {@code LauncherDiscoveryRequestBuilder} provides a light-weight DSL for
* generating a {@link LauncherDiscoveryRequest}.
Expand Down Expand Up @@ -199,27 +200,15 @@ public LauncherDiscoveryRequestBuilder configurationParameters(Map<String, Strin
return this;
}

/**
* Add the supplied configuration parameters resource file to the request.
* @param propertiesFile the classpath location of the properties file
* never {@code null}
* @return this builder for method chaining
*/
public LauncherDiscoveryRequestBuilder configurationParametersResource(String propertiesFile) {
Preconditions.notNull(propertiesFile, "properties file must not be null");
configurationParametersResources.add(propertiesFile);
return this;
}

/**
* Add all of the supplied configuration parameters resource files to the request.
* @param propertiesFiles the classpath locations of the properties files
* @param paths the classpath locations of the properties files
* never {@code null}
* @return this builder for method chaining
*/
public LauncherDiscoveryRequestBuilder configurationParametersResources(List<String> propertiesFiles) {
Preconditions.notNull(propertiesFiles, "properties files must not be null");
propertiesFiles.forEach(this::configurationParametersResource);
public LauncherDiscoveryRequestBuilder configurationParametersResources(String... paths) {
Preconditions.notNull(paths, "property file paths must not be null");
Collections.addAll(configurationParametersResources, paths);
return this;
}

Expand Down
5 changes: 2 additions & 3 deletions ...suite-api/src/main/java/org/junit/platform/suite/api/ConfigurationParametersResource.java
View file
Expand Up @@ -32,20 +32,19 @@
* @see Suite
* @see org.junit.platform.runner.JUnitPlatform
* @see org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder#configurationParameter(String, String)
* @see org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder#configurationParametersResource(String)
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Inherited
@Documented
// TODO: Before PR merge, change to STABLE/MAINTAINED
// TODO: Is version 1.11 correct?
@API(status = EXPERIMENTAL, since = "1.11")
@Repeatable(ConfigurationParametersResources.class)
public @interface ConfigurationParametersResource {

/**
* The classpath location for the desired properties file; never {@code null} or blank.
*/
String resource();
String value();

}
2 changes: 0 additions & 2 deletions ...uite-api/src/main/java/org/junit/platform/suite/api/ConfigurationParametersResources.java
View file
Expand Up @@ -36,8 +36,6 @@
@Target(ElementType.TYPE)
@Inherited
@Documented
// TODO: Before PR merge, change to STABLE/MAINTAINED
// TODO: Is version 1.11 correct?
@API(status = EXPERIMENTAL, since = "1.11")
public @interface ConfigurationParametersResources {

Expand Down
6 changes: 3 additions & 3 deletions .../src/main/java/org/junit/platform/suite/commons/SuiteLauncherDiscoveryRequestBuilder.java
View file
Expand Up @@ -116,8 +116,8 @@ public SuiteLauncherDiscoveryRequestBuilder configurationParameters(Map<String,
return this;
}

public SuiteLauncherDiscoveryRequestBuilder configurationParametersResource(String resourceFile) {
delegate.configurationParametersResource(resourceFile);
public SuiteLauncherDiscoveryRequestBuilder configurationParametersResource(String path) {
delegate.configurationParametersResources(path);
return this;
}

Expand All @@ -140,7 +140,7 @@ public SuiteLauncherDiscoveryRequestBuilder suite(Class<?> suiteClass) {
findRepeatableAnnotations(suiteClass, ConfigurationParameter.class)
.forEach(configuration -> configurationParameter(configuration.key(), configuration.value()));
findRepeatableAnnotations(suiteClass, ConfigurationParametersResource.class)
.forEach(configResource -> configurationParametersResource(configResource.resource()));
.forEach(configResource -> configurationParametersResource(configResource.value()));
findAnnotation(suiteClass, DisableParentConfigurationParameters.class)
.ifPresent(__ -> enableParentConfigurationParameters = false);
findAnnotationValues(suiteClass, ExcludeClassNamePatterns.class, ExcludeClassNamePatterns::value)
Expand Down
16 changes: 16 additions & 0 deletions ...rm-tests/src/test/java/org/junit/platform/console/tasks/DiscoveryRequestCreatorTests.java
View file
Expand Up @@ -317,6 +317,22 @@ void convertsConfigurationParameters() {
assertThat(configurationParameters.getBoolean("baz")).contains(true);
}

@Test
@SuppressWarnings("deprecation")
void convertsConfigurationParametersResources() {
options.setScanClasspath(true);
options.setConfigurationParameters(mapOf(entry("foo", "bar"), entry("com.example.prop.first", "baz")));
options.setConfigurationParametersResources(List.of("config-test.properties"));

var request = convert();
var configurationParameters = request.getConfigurationParameters();

assertThat(configurationParameters.size()).isEqualTo(2);
assertThat(configurationParameters.get("foo")).contains("bar");
assertThat(configurationParameters.get("com.example.prop.first")).contains("baz");
assertThat(configurationParameters.get("com.example.prop.second")).contains("second value");
}

private LauncherDiscoveryRequest convert() {
var creator = new DiscoveryRequestCreator();
return creator.toDiscoveryRequest(options);
Expand Down
8 changes: 4 additions & 4 deletions .../src/test/java/org/junit/platform/launcher/core/LauncherDiscoveryRequestBuilderTests.java
View file
Expand Up @@ -318,7 +318,7 @@ void multipleConfigurationParametersAddedByMap_areStoredInDiscoveryRequest() {
void configurationParametersResource_areStoredInDiscoveryRequest() {
// @formatter:off
var discoveryRequest = request()
.configurationParametersResource("config-test.properties")
.configurationParametersResources("config-test.properties")
.build();
// @formatter:on

Expand All @@ -332,7 +332,7 @@ void configurationParametersResource_areStoredInDiscoveryRequest() {
void configurationParametersResource_explicitConfigParametersOverrideResource() {
// @formatter:off
var discoveryRequest = request()
.configurationParametersResource("config-test.properties")
.configurationParametersResources("config-test.properties")
.configurationParameter("com.example.prop.first", "first value override")
.build();
// @formatter:on
Expand All @@ -346,8 +346,8 @@ void configurationParametersResource_explicitConfigParametersOverrideResource()
void configurationParametersResource_lastDeclaredResourceFileWins() {
// @formatter:off
var discoveryRequest = request()
.configurationParametersResource("config-test.properties")
.configurationParametersResource("config-test-override.properties")
.configurationParametersResources("config-test.properties")
.configurationParametersResources("config-test-override.properties")
.build();
// @formatter:on

Expand Down
9 changes: 4 additions & 5 deletions ...test/java/org/junit/platform/suite/commons/SuiteLauncherDiscoveryRequestBuilderTests.java
View file
Expand Up @@ -53,7 +53,6 @@
import org.junit.platform.launcher.PostDiscoveryFilter;
import org.junit.platform.suite.api.ConfigurationParameter;
import org.junit.platform.suite.api.ConfigurationParametersResource;
import org.junit.platform.suite.api.ConfigurationParametersResources;
import org.junit.platform.suite.api.DisableParentConfigurationParameters;
import org.junit.platform.suite.api.ExcludeClassNamePatterns;
import org.junit.platform.suite.api.ExcludeEngines;
Expand Down Expand Up @@ -90,7 +89,7 @@ class Suite {

@Test
void configurationParametersResource() {
@ConfigurationParametersResource(resource = "config-test.properties")
@ConfigurationParametersResource("config-test.properties")
class Suite {
}

Expand All @@ -102,8 +101,8 @@ class Suite {

@Test
void configurationParametersResources() {
@ConfigurationParametersResources({ @ConfigurationParametersResource(resource = "config-test.properties"),
@ConfigurationParametersResource(resource = "config-test-override.properties") })
@ConfigurationParametersResource("config-test.properties")
@ConfigurationParametersResource("config-test-override.properties")
class Suite {
}

Expand All @@ -117,7 +116,7 @@ class Suite {

@Test
void configurationParametersResource_explicitParametersTakePrecedence() {
@ConfigurationParametersResource(resource = "config-test.properties")
@ConfigurationParametersResource("config-test.properties")
@ConfigurationParameter(key = "com.example.prop.first", value = "first value from explicit parameter")
class Suite {
}
Expand Down

0 comments on commit e5ff689

Please sign in to comment.