root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Improve initialization of Assume class 

Prior to this commit, the org.springframework.tests.Assume class could
fail to load resulting in a NoClassDefFoundError if parsing of the
'testGroups' system property failed. This is because the parsing took
place while initializing a static field.

This commit addresses this issue by moving the 'testGroups' system
property lookup to a dedicated method that is lazily invoked upon
demand instead of eagerly when loading the Assume class itself.

In addition, when an error occurs, TestGroup.parse() now logs the
complete original value of the supplied test groups string instead of
potentially omitting the "all-" prefix. This results in more
informative error messages similar to the following.

  java.lang.IllegalStateException: Failed to parse 'testGroups' system
  property: Unable to find test group 'bogus' when parsing testGroups
  value: 'all-bogus'. Available groups include:
  [LONG_RUNNING,PERFORMANCE,JMXMP,CI]

Issue: SPR-15163
This commit is contained in:
Sam Brannen 2017-01-19 05:42:26 +01:00
parent bb3b1f2fe2
commit 264edb3fb5
4 changed files with 193 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
spring-core/src/test/java/org/springframework/tests/Assume.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -28,13 +28,14 @@ import static org.junit.Assume.*;
* conditions hold {@code true}. If the assumption fails, it means the test should be
* skipped.
*
* Tests can be categorized into {@link TestGroup}s. Active groups are enabled using
* <p>Tests can be categorized into {@link TestGroup}s. Active groups are enabled using
* the 'testGroups' system property, usually activated from the gradle command line:
* <pre>
*
* <pre class="code">
* gradle test -PtestGroups="performance"
* </pre>
*
* Groups can be specified as a comma separated list of values, or using the pseudo group
* <p>Groups can be specified as a comma separated list of values, or using the pseudo group
* 'all'. See {@link TestGroup} for a list of valid groups.
*
* @author Rob Winch
@ -46,7 +47,7 @@ import static org.junit.Assume.*;
*/
public abstract class Assume {
private static final Set<TestGroup> GROUPS = TestGroup.parse(System.getProperty("testGroups"));
static final String TEST_GROUPS_SYSTEM_PROPERTY = "testGroups";
/**
@ -55,8 +56,9 @@ public abstract class Assume {
* @throws AssumptionViolatedException if the assumption fails
*/
public static void group(TestGroup group) {
if (!GROUPS.contains(group)) {
throw new AssumptionViolatedException("Requires unspecified group " + group + " from " + GROUPS);
Set<TestGroup> testGroups = loadTestGroups();
if (!testGroups.contains(group)) {
throw new AssumptionViolatedException("Requires unspecified group " + group + " from " + testGroups);
}
}
@ -70,7 +72,8 @@ public abstract class Assume {
* @since 4.2
*/
public static void group(TestGroup group, Executable executable) throws Exception {
if (GROUPS.contains(group)) {
Set<TestGroup> testGroups = loadTestGroups();
if (testGroups.contains(group)) {
executable.execute();
}
}
@ -85,6 +88,21 @@ public abstract class Assume {
assumeFalse(log.isDebugEnabled());
}
/**
* Load test groups dynamically instead of during static
* initialization in order to avoid a {@link NoClassDefFoundError}
* being thrown while attempting to load the {@code Assume} class.
*/
private static Set<TestGroup> loadTestGroups() {
try {
return TestGroup.parse(System.getProperty(TEST_GROUPS_SYSTEM_PROPERTY));
}
catch (Exception ex) {
throw new IllegalStateException("Failed to parse '" + TEST_GROUPS_SYSTEM_PROPERTY
+ "' system property: " + ex.getMessage(), ex);
}
}
/**
* @since 4.2

124
spring-core/src/test/java/org/springframework/tests/AssumeTests.java Normal file
View File

@ -0,0 +1,124 @@
/*
* Copyright 2002-2017 the original author or authors.
*
* Licensed 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.springframework.tests;
import java.util.Arrays;
import org.junit.After;
import org.junit.AssumptionViolatedException;
import org.junit.Before;
import org.junit.Test;
import static java.util.stream.Collectors.*;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
import static org.springframework.tests.Assume.*;
import static org.springframework.tests.TestGroup.*;
/**
* Tests for {@link Assume}.
*
* @author Sam Brannen
* @since 5.0
*/
public class AssumeTests {
private String originalTestGroups;
@Before
public void trackOriginalTestGroups() {
this.originalTestGroups = System.getProperty(TEST_GROUPS_SYSTEM_PROPERTY);
}
@After
public void restoreOriginalTestGroups() {
if (this.originalTestGroups != null) {
setTestGroups(this.originalTestGroups);
}
else {
setTestGroups("");
}
}
@Test
public void assumeGroupWithNoActiveTestGroups() {
setTestGroups("");
Assume.group(JMXMP);
fail("assumption should have failed");
}
@Test
public void assumeGroupWithNoMatchingActiveTestGroup() {
setTestGroups(PERFORMANCE, CI);
Assume.group(JMXMP);
fail("assumption should have failed");
}
@Test
public void assumeGroupWithMatchingActiveTestGroup() {
setTestGroups(JMXMP);
try {
Assume.group(JMXMP);
}
catch (AssumptionViolatedException ex) {
fail("assumption should NOT have failed");
}
}
@Test
public void assumeGroupWithBogusActiveTestGroup() {
assertBogusActiveTestGroupBehavior("bogus");
}
@Test
public void assumeGroupWithAllMinusBogusActiveTestGroup() {
assertBogusActiveTestGroupBehavior("all-bogus");
}
private void assertBogusActiveTestGroupBehavior(String testGroups) {
// Should result in something similar to the following:
//
// java.lang.IllegalStateException: Failed to parse 'testGroups' system property:
// Unable to find test group 'bogus' when parsing testGroups value: 'all-bogus'.
// Available groups include: [LONG_RUNNING,PERFORMANCE,JMXMP,CI]
setTestGroups(testGroups);
try {
Assume.group(JMXMP);
fail("assumption should have failed");
}
catch (IllegalStateException ex) {
assertThat(ex.getMessage(),
startsWith("Failed to parse '" + TEST_GROUPS_SYSTEM_PROPERTY + "' system property: "));
assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class));
assertThat(ex.getCause().getMessage(),
equalTo("Unable to find test group 'bogus' when parsing testGroups value: '" + testGroups
+ "'. Available groups include: [LONG_RUNNING,PERFORMANCE,JMXMP,CI]"));
}
}
private void setTestGroups(TestGroup... testGroups) {
setTestGroups(Arrays.stream(testGroups).map(TestGroup::name).collect(joining(", ")));
}
private void setTestGroups(String testGroups) {
System.setProperty(TEST_GROUPS_SYSTEM_PROPERTY, testGroups);
}
}

21
spring-core/src/test/java/org/springframework/tests/TestGroup.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -31,6 +31,7 @@ import static java.lang.String.*;
* @see Assume#group(TestGroup)
* @author Phillip Webb
* @author Chris Beams
* @author Sam Brannen
*/
public enum TestGroup {
@ -65,23 +66,27 @@ public enum TestGroup {
* Parse the specified comma separated string of groups.
* @param value the comma separated string of groups
* @return a set of groups
* @throws IllegalArgumentException if any specified group name is not a
* valid {@link TestGroup}
*/
public static Set<TestGroup> parse(String value) {
if (value == null || "".equals(value)) {
public static Set<TestGroup> parse(String value) throws IllegalArgumentException {
if (!StringUtils.hasText(value)) {
return Collections.emptySet();
}
String originalValue = value;
value = value.trim();
if ("ALL".equalsIgnoreCase(value)) {
return EnumSet.allOf(TestGroup.class);
}
if (value.toUpperCase().startsWith("ALL-")) {
Set<TestGroup> groups = new HashSet<>(EnumSet.allOf(TestGroup.class));
groups.removeAll(parseGroups(value.substring(4)));
Set<TestGroup> groups = EnumSet.allOf(TestGroup.class);
groups.removeAll(parseGroups(originalValue, value.substring(4)));
return groups;
}
return parseGroups(value);
return parseGroups(originalValue, value);
}
private static Set<TestGroup> parseGroups(String value) {
private static Set<TestGroup> parseGroups(String originalValue, String value) throws IllegalArgumentException {
Set<TestGroup> groups = new HashSet<>();
for (String group : value.split(",")) {
try {
@ -90,7 +95,7 @@ public enum TestGroup {
catch (IllegalArgumentException ex) {
throw new IllegalArgumentException(format(
"Unable to find test group '%s' when parsing testGroups value: '%s'. " +
"Available groups include: [%s]", group.trim(), value,
"Available groups include: [%s]", group.trim(), originalValue,
StringUtils.arrayToCommaDelimitedString(TestGroup.values())));
}
}

46
spring-core/src/test/java/org/springframework/tests/TestGroupTests.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -18,7 +18,6 @@ package org.springframework.tests;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Set;
import org.junit.Rule;
@ -32,6 +31,7 @@ import static org.junit.Assert.*;
* Tests for {@link TestGroup}.
*
* @author Phillip Webb
* @author Sam Brannen
*/
public class TestGroupTests {
@ -40,29 +40,34 @@ public class TestGroupTests {
@Test
public void parseNull() throws Exception {
assertThat(TestGroup.parse(null), equalTo(Collections.<TestGroup> emptySet()));
public void parseNull() {
assertThat(TestGroup.parse(null), equalTo(Collections.emptySet()));
}
@Test
public void parseEmptyString() throws Exception {
assertThat(TestGroup.parse(""), equalTo(Collections.<TestGroup> emptySet()));
public void parseEmptyString() {
assertThat(TestGroup.parse(""), equalTo(Collections.emptySet()));
}
@Test
public void parseWithSpaces() throws Exception {
assertThat(TestGroup.parse("PERFORMANCE, PERFORMANCE"),
equalTo((Set<TestGroup>) EnumSet.of(TestGroup.PERFORMANCE)));
public void parseBlankString() {
assertThat(TestGroup.parse(" "), equalTo(Collections.emptySet()));
}
@Test
public void parseInMixedCase() throws Exception {
public void parseWithSpaces() {
assertThat(TestGroup.parse(" PERFORMANCE, PERFORMANCE "),
equalTo(EnumSet.of(TestGroup.PERFORMANCE)));
}
@Test
public void parseInMixedCase() {
assertThat(TestGroup.parse("performance, PERFormaNCE"),
equalTo((Set<TestGroup>) EnumSet.of(TestGroup.PERFORMANCE)));
equalTo(EnumSet.of(TestGroup.PERFORMANCE)));
}
@Test
public void parseMissing() throws Exception {
public void parseMissing() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("Unable to find test group 'missing' when parsing " +
"testGroups value: 'performance, missing'. Available groups include: " +
@ -71,15 +76,24 @@ public class TestGroupTests {
}
@Test
public void parseAll() throws Exception {
assertThat(TestGroup.parse("all"), equalTo((Set<TestGroup>)EnumSet.allOf(TestGroup.class)));
public void parseAll() {
assertThat(TestGroup.parse("all"), equalTo(EnumSet.allOf(TestGroup.class)));
}
@Test
public void parseAllExcept() throws Exception {
Set<TestGroup> expected = new HashSet<>(EnumSet.allOf(TestGroup.class));
public void parseAllExceptPerformance() {
Set<TestGroup> expected = EnumSet.allOf(TestGroup.class);
expected.remove(TestGroup.PERFORMANCE);
assertThat(TestGroup.parse("all-performance"), equalTo(expected));
}
@Test
public void parseAllExceptMissing() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("Unable to find test group 'missing' when parsing " +
"testGroups value: 'all-missing'. Available groups include: " +
"[LONG_RUNNING,PERFORMANCE,JMXMP,CI]");
TestGroup.parse("all-missing");
}
}