Skip to content

Commit 18d572c

Browse files
Switch to ParameterizedTest, suppress FB warning
* simplify the tests with JUnit 5 fun * SCMFileSystem.of can return a null but spotbugs will issue 2 redundant null check errors.
1 parent 1719a48 commit 18d572c

File tree

2 files changed

+37
-76
lines changed

2 files changed

+37
-76
lines changed

src/main/java/org/jenkinsci/plugins/workflow/multibranch/template/ConfigFileSCMBinder.java

+3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
package org.jenkinsci.plugins.workflow.multibranch.template;
2626

27+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2728
import hudson.AbortException;
2829
import hudson.Extension;
2930
import hudson.FilePath;
@@ -78,6 +79,8 @@ public Object readResolve() {
7879

7980
public static final String INAPPROPRIATE_CONTEXT = "inappropriate context";
8081

82+
@SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE",
83+
justification = "SCMFileSystem.of can return a null but spotbugs does not understand")
8184
@Override public FlowExecution create(FlowExecutionOwner handle, TaskListener listener, List<? extends Action> actions) throws Exception {
8285
Queue.Executable exec = handle.getExecutable();
8386
if (!(exec instanceof WorkflowRun)) {
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
package org.jenkinsci.plugins.workflow.multibranch.template.finder;
22

3-
import org.junit.jupiter.api.Test;
3+
import org.junit.jupiter.params.ParameterizedTest;
4+
import org.junit.jupiter.params.provider.Arguments;
5+
import org.junit.jupiter.params.provider.MethodSource;
6+
7+
import java.util.stream.Stream;
48

59
import static org.junit.jupiter.api.Assertions.*;
610

711
class ConfigurationValueFinderTest {
812

13+
public static final String EXPECTED_RESULT = "expectedResult";
14+
915
/**
1016
* Simple helper method to call ConfigurationValueFinder.findFirstConfigurationValue
1117
* to improve test readability.
@@ -18,87 +24,39 @@ private String find(String configurationContents, String keyToFind) {
1824
return ConfigurationValueFinder.findFirstConfigurationValue(configurationContents, keyToFind);
1925
}
2026

21-
@Test
22-
void findNullKeyOutOfConfigReturnsNull() {
23-
assertNull(find("test", null));
24-
}
25-
26-
@Test
27-
void findStringKeyOutOfNullConfigReturnsNull() {
28-
assertNull(find(null, "test1"));
29-
}
30-
31-
@Test
32-
void cannotFindKeyReturnsNull() {
33-
assertNull(find("keynotfound", "test1"));
34-
}
35-
36-
@Test
37-
void findFirstKeyWithNoNewLine() {
38-
assertEquals("hi",
39-
find("nonewline:hi", "nonewline"));
40-
}
41-
42-
@Test
43-
void foundFirstKey() {
44-
assertEquals("hi",
45-
find("bare:hi\n test1:second\n test1:third",
46-
"bare"));
27+
private static Stream<Arguments> keyNotFoundProvider() {
28+
return Stream.of(
29+
Arguments.of("null key", null, "test"),
30+
Arguments.of("null config", "someKey", null),
31+
Arguments.of("key not found", "someKey", "keynotfound")
32+
);
4733
}
4834

49-
@Test
50-
void foundLastKey() {
51-
assertEquals("third",
52-
find("bare:hi\n test1:second\n last:third",
53-
"last"));
35+
@ParameterizedTest
36+
@MethodSource("keyNotFoundProvider")
37+
void cannotFindKey(String testCase, String keyToFind, String configurationContents) {
38+
assertNull(find(configurationContents, keyToFind), "Should not have found value for " + testCase);
5439
}
5540

56-
@Test
57-
void foundKeyWithQuotesAroundKey() {
58-
assertEquals("hi",
59-
find("\"quotes\":\"hi\"\n test1:second\n test1:third",
60-
"quotes"));
61-
}
62-
63-
@Test
64-
void foundKeyWithTicksAroundKey() {
65-
assertEquals("hi",
66-
find("'ticks':'hi'\n test1:second\n test1:third",
67-
"ticks"));
68-
}
69-
70-
@Test
71-
void foundKeyWithTicksAroundKeyAndManySpaces() {
72-
assertEquals("hi",
73-
find("'spaces' : 'hi' \n test1:second\n test1:third",
74-
"spaces"));
75-
}
76-
77-
@Test
78-
void foundKeyWithEquals() {
79-
assertEquals("hi",
80-
find("equals=\"hi\"\ntest1=second\ntest2=third",
81-
"equals"));
82-
}
83-
84-
@Test
85-
void foundLastKeyWithEquals() {
86-
assertEquals("third",
87-
find("equals=\"hi\"\ntest1=second\nlast=third",
88-
"last"));
89-
}
9041

91-
@Test
92-
void foundKeyWithSpacesAroundEquals() {
93-
assertEquals("hi",
94-
find("equals_space = \"hi\"\ntest1 = second\ntest2 = third",
95-
"equals_space"));
42+
private static Stream<Arguments> keysFoundProvider() {
43+
return Stream.of(
44+
Arguments.of("noNewLine", "noNewLine:" + EXPECTED_RESULT),
45+
Arguments.of("firstKey", "firstKey:" + EXPECTED_RESULT + "\n test1:second\n test1:third"),
46+
Arguments.of("lastKey", "bare:hi\n test1:second\n lastKey:" + EXPECTED_RESULT),
47+
Arguments.of("keyWithQuotesAround", "\"keyWithQuotesAround\":\"" + EXPECTED_RESULT + "\"\n test1:second\n test1:third"),
48+
Arguments.of("keyWithTicksAround", "'keyWithTicksAround':'" + EXPECTED_RESULT + "'\n test1:second\n test1:third"),
49+
Arguments.of("keyWithTicksSpaces", "'keyWithTicksSpaces' : '" + EXPECTED_RESULT + "' \n test1:second\n test1:third"),
50+
Arguments.of("keyWithEqualsDelim", "keyWithEqualsDelim=\"" + EXPECTED_RESULT + "\"\ntest1=second\ntest2=third"),
51+
Arguments.of("lastKeyWithEqualsDelim", "equals=\"hi\"\ntest1=second\nlastKeyWithEqualsDelim=" + EXPECTED_RESULT),
52+
Arguments.of("keyWithSpaceAroundEquals", "keyWithSpaceAroundEquals = \"" + EXPECTED_RESULT + "\"\ntest1 = second\ntest2 = third"),
53+
Arguments.of("lastKeyWithSpaceyEquals", "equals_space = \"hi\"\ntest1 = second\nlastKeyWithSpaceyEquals = " + EXPECTED_RESULT)
54+
);
9655
}
9756

98-
@Test
99-
void foundLastKeyWithSpacesAroundEquals() {
100-
assertEquals("third",
101-
find("equals_space = \"hi\"\ntest1 = second\nlast = third",
102-
"last"));
57+
@ParameterizedTest
58+
@MethodSource("keysFoundProvider")
59+
void findKey(String keyToFind, String configurationContents) {
60+
assertEquals(EXPECTED_RESULT, find(configurationContents, keyToFind), "Did not find expected value for " + keyToFind);
10361
}
10462
}

0 commit comments

Comments
 (0)