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:
`/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
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]
