alagodasii commented on code in PR #1716:
URL: https://github.com/apache/jackrabbit-oak/pull/1716#discussion_r1760866487
##########
oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/ConfigHelperTest.java:
##########
Review Comment:
It's non-functional change, but could improve structure, readability and
extensibility of the test. I suggest parameterising it. JUnit 4 doesn't support
it well, but still we have capabilities of doing so. Here's the proposed
solution:
_package org.apache.jackrabbit.oak.index.indexer.document.flatfile.pipelined;
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.junit.Assert.assertEquals;
import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.RestoreSystemProperties;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
import org.junit.runners.Suite;
import java.util.Collection;
import java.util.List;
@RunWith(Suite.class)
@Suite.SuiteClasses({ConfigHelperTest.SystemPropertyExistsTest.class,
ConfigHelperTest.MissingSystemPropertyTest.class})
public class ConfigHelperTest {
@Rule
public final RestoreSystemProperties restoreSystemProperties = new
RestoreSystemProperties();
@RunWith(Parameterized.class)
public static class SystemPropertyExistsTest {
private final String key;
private final String value;
private final List<String> expected;
public SystemPropertyExistsTest(String key, String value, List<String>
expected) {
this.key = key;
this.value = value;
this.expected = expected;
}
@Parameters
public static Collection<Object[]> data() {
return List.of(new Object[][]{
{"key1", "value1", singletonList("value1")},
{"key2", " ", emptyList()},
{"key3", "v1;v2", List.of("v1", "v2")},
{"key4", "v1; v2", List.of("v1", "v2")}
});
}
@Test
public void
givenDefaultValue_whenSystemPropertyExists_thenGetSystemPropertyAsString() {
// given
var defaultValue = "default";
var separator = ';';
System.setProperty(key, value);
// when
final var result = ConfigHelper.getSystemPropertyAsStringList(key,
defaultValue, separator);
// then
assertEquals(expected, result);
}
}
@RunWith(Parameterized.class)
public static class MissingSystemPropertyTest {
private final String defaultValue;
private final List<String> expected;
public MissingSystemPropertyTest(String defaultValue, List<String>
expected) {
this.defaultValue = defaultValue;
this.expected = expected;
}
@Parameters
public static Collection<Object[]> data() {
return List.of(new Object[][]{
{"", emptyList()},
{"default", singletonList("default")},
{"default1;default2", List.of("default1", "default2")}
});
}
@Test
public void
givenDefaultValue_whenSystemPropertyNotExists_thenReturnDefaultValueAsList() {
// given
final var key = "not.defined";
final var separator = ';';
// when
final var result = ConfigHelper.getSystemPropertyAsStringList(key,
defaultValue, separator);
// then
assertEquals(expected, result);
}
}
}_
Also could you provide the scenario where separator differs from the one
used in system property? I.e. `default1;defalut2` and `,`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
