Browse Source

Support @EnabledIf & @DisabledIf w/o loading ApplicationContext

Prior to this commit, when using @EnabledIf or @DisabledIf in Spring's
JUnit Jupiter support, the test's ApplicationContext was always eagerly
loaded, even if the ApplicationContext would never be used (i.e., the
test was disabled). This behavior can lead to undesirable side effects
-- for example, attempting to load an application context that requires
services only available on the CI server when the test is not actually
running on the CI server.

This commit addresses this issue by introducing new boolean
`loadContext` flags in @EnabledIf and @DisabledIf. By default these
flags are set to false which means that the user's test application
context will not be loaded to evaluate the expression. On the contrary,
a dummy application context will be loaded instead, and expressions
will be evaluated against that dummy context. Consequently, if the user
wishes to interact with properties from the Spring Environment or with
beans from the test application context, the `loadContext` must be
explicitly set to true.

In addition, expressions which evaluate to a String must now evaluate
to exactly "true" or "false" (ignoring case and surrounding whitespace).

Issue: SPR-14649
pull/1223/head
Sam Brannen 8 years ago
parent
commit
5c9350b8cb
  1. 55
      spring-test/src/main/java/org/springframework/test/context/junit/jupiter/AbstractExpressionEvaluatingCondition.java
  2. 20
      spring-test/src/main/java/org/springframework/test/context/junit/jupiter/DisabledIf.java
  3. 3
      spring-test/src/main/java/org/springframework/test/context/junit/jupiter/DisabledIfCondition.java
  4. 20
      spring-test/src/main/java/org/springframework/test/context/junit/jupiter/EnabledIf.java
  5. 3
      spring-test/src/main/java/org/springframework/test/context/junit/jupiter/EnabledIfCondition.java
  6. 23
      spring-test/src/test/java/org/springframework/test/context/junit/jupiter/DisabledIfConditionTestCase.java
  7. 17
      spring-test/src/test/java/org/springframework/test/context/junit/jupiter/DisabledIfTestCase.java
  8. 15
      spring-test/src/test/java/org/springframework/test/context/junit/jupiter/EnabledIfTestCase.java

55
spring-test/src/main/java/org/springframework/test/context/junit/jupiter/AbstractExpressionEvaluatingCondition.java

@ -34,6 +34,7 @@ import org.springframework.beans.factory.config.BeanExpressionResolver; @@ -34,6 +34,7 @@ import org.springframework.beans.factory.config.BeanExpressionResolver;
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.support.GenericApplicationContext;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
@ -75,6 +76,8 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti @@ -75,6 +76,8 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti
* the annotation
* @param reasonExtractor a function that extracts the reason from the
* annotation
* @param loadContextExtractor a function that extracts the {@code loadContext}
* flag from the annotation
* @param enabledOnTrue indicates whether the returned {@code ConditionEvaluationResult}
* should be {@link ConditionEvaluationResult#enabled enabled} if the expression
* evaluates to {@code true}
@ -83,8 +86,8 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti @@ -83,8 +86,8 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti
* or test should be enabled; otherwise {@link ConditionEvaluationResult#disabled disabled}
*/
protected <A extends Annotation> ConditionEvaluationResult evaluateAnnotation(Class<A> annotationType,
Function<A, String> expressionExtractor, Function<A, String> reasonExtractor, boolean enabledOnTrue,
ExtensionContext context) {
Function<A, String> expressionExtractor, Function<A, String> reasonExtractor,
Function<A, Boolean> loadContextExtractor, boolean enabledOnTrue, ExtensionContext context) {
AnnotatedElement element = context.getElement().get();
Optional<A> annotation = findMergedAnnotation(element, annotationType);
@ -104,7 +107,8 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti @@ -104,7 +107,8 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti
"The expression in @%s on [%s] must not be blank", annotationType.getSimpleName(), element)));
// @formatter:on
boolean evaluatedToTrue = evaluateExpression(expression, annotationType, context);
boolean loadContext = annotation.map(loadContextExtractor).get();
boolean evaluatedToTrue = evaluateExpression(expression, loadContext, annotationType, context);
if (evaluatedToTrue) {
String adjective = (enabledOnTrue ? "enabled" : "disabled");
@ -129,17 +133,27 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti @@ -129,17 +133,27 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti
}
}
private <A extends Annotation> boolean evaluateExpression(String expression, Class<A> annotationType,
ExtensionContext extensionContext) {
private <A extends Annotation> boolean evaluateExpression(String expression, boolean loadContext,
Class<A> annotationType, ExtensionContext extensionContext) {
ApplicationContext applicationContext = SpringExtension.getApplicationContext(extensionContext);
AnnotatedElement element = extensionContext.getElement().get();
GenericApplicationContext gac = null;
ApplicationContext applicationContext;
if (loadContext) {
applicationContext = SpringExtension.getApplicationContext(extensionContext);
} else {
gac = new GenericApplicationContext();
gac.refresh();
applicationContext = gac;
}
if (!(applicationContext instanceof ConfigurableApplicationContext)) {
if (logger.isWarnEnabled()) {
String contextType = (applicationContext != null ? applicationContext.getClass().getName() : "null");
logger.warn(String.format("@%s(\"%s\") could not be evaluated on [%s] since the test " +
"ApplicationContext [%s] is not a ConfigurableApplicationContext",
annotationType.getSimpleName(), expression, extensionContext.getElement(), contextType));
annotationType.getSimpleName(), expression, element, contextType));
}
return false;
}
@ -151,12 +165,29 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti @@ -151,12 +165,29 @@ abstract class AbstractExpressionEvaluatingCondition implements ContainerExecuti
Object result = expressionResolver.evaluate(configurableBeanFactory.resolveEmbeddedValue(expression),
beanExpressionContext);
Assert.state((result instanceof Boolean || result instanceof String),
() -> String.format("@%s(\"%s\") must evaluate to a String or a Boolean, not %s",
annotationType.getSimpleName(), expression, (result != null ? result.getClass().getName() : "null")));
if (gac != null) {
gac.close();
}
return (result instanceof Boolean && ((Boolean) result).booleanValue())
|| (result instanceof String && Boolean.parseBoolean((String) result));
if (result instanceof Boolean) {
return ((Boolean) result).booleanValue();
}
else if (result instanceof String) {
String str = ((String) result).trim().toLowerCase();
if ("true".equals(str)) {
return true;
}
Assert.state("false".equals(str),
() -> String.format("@%s(\"%s\") on %s must evaluate to \"true\" or \"false\", not \"%s\"",
annotationType.getSimpleName(), expression, element, result));
return false;
}
else {
String message = String.format("@%s(\"%s\") on %s must evaluate to a String or a Boolean, not %s",
annotationType.getSimpleName(), expression, element,
(result != null ? result.getClass().getName() : "null"));
throw new IllegalStateException(message);
}
}
private static <A extends Annotation> Optional<A> findMergedAnnotation(AnnotatedElement element,

20
spring-test/src/main/java/org/springframework/test/context/junit/jupiter/DisabledIf.java

@ -64,8 +64,8 @@ import org.springframework.core.annotation.AliasFor; @@ -64,8 +64,8 @@ import org.springframework.core.annotation.AliasFor;
public @interface DisabledIf {
/**
* Alias for {@link #expression}; only intended to be used if an
* explicit {@link #reason} is not provided.
* Alias for {@link #expression}; only intended to be used if {@link #reason}
* and {@link #loadContext} are not specified.
*
* @see #expression
*/
@ -97,6 +97,7 @@ public @interface DisabledIf { @@ -97,6 +97,7 @@ public @interface DisabledIf {
* and {@code @DisabledIf("false")} is logically meaningless.
*
* @see #reason
* @see #loadContext
* @see #value
*/
@AliasFor("value")
@ -109,4 +110,19 @@ public @interface DisabledIf { @@ -109,4 +110,19 @@ public @interface DisabledIf {
*/
String reason() default "";
/**
* Whether the {@code ApplicationContext} associated with the current test
* should be eagerly loaded in order to evaluate the {@link #expression}.
*
* <p>Defaults to {@code false} so that test application contexts are not
* eagerly loaded unnecessarily. If an expression is based solely on system
* properties or environment variables or does not interact with beans in
* the test's application context, there is no need to load the context
* prematurely since doing so would be a waste of time if the test ends up
* being disabled.
*
* @see #expression
*/
boolean loadContext() default false;
}

3
spring-test/src/main/java/org/springframework/test/context/junit/jupiter/DisabledIfCondition.java

@ -61,7 +61,8 @@ public class DisabledIfCondition extends AbstractExpressionEvaluatingCondition { @@ -61,7 +61,8 @@ public class DisabledIfCondition extends AbstractExpressionEvaluatingCondition {
}
private ConditionEvaluationResult evaluateDisabledIf(ExtensionContext context) {
return evaluateAnnotation(DisabledIf.class, DisabledIf::expression, DisabledIf::reason, false, context);
return evaluateAnnotation(DisabledIf.class, DisabledIf::expression, DisabledIf::reason,
DisabledIf::loadContext, false, context);
}
}

20
spring-test/src/main/java/org/springframework/test/context/junit/jupiter/EnabledIf.java

@ -63,8 +63,8 @@ import org.springframework.core.annotation.AliasFor; @@ -63,8 +63,8 @@ import org.springframework.core.annotation.AliasFor;
public @interface EnabledIf {
/**
* Alias for {@link #expression}; only intended to be used if an
* explicit {@link #reason} is not provided.
* Alias for {@link #expression}; only intended to be used if {@link #reason}
* and {@link #loadContext} are not specified.
*
* @see #expression
*/
@ -96,6 +96,7 @@ public @interface EnabledIf { @@ -96,6 +96,7 @@ public @interface EnabledIf {
* and {@code @EnabledIf("true")} is logically meaningless.
*
* @see #reason
* @see #loadContext
* @see #value
*/
@AliasFor("value")
@ -108,4 +109,19 @@ public @interface EnabledIf { @@ -108,4 +109,19 @@ public @interface EnabledIf {
*/
String reason() default "";
/**
* Whether the {@code ApplicationContext} associated with the current test
* should be eagerly loaded in order to evaluate the {@link #expression}.
*
* <p>Defaults to {@code false} so that test application contexts are not
* eagerly loaded unnecessarily. If an expression is based solely on system
* properties or environment variables or does not interact with beans in
* the test's application context, there is no need to load the context
* prematurely since doing so would be a waste of time if the test ends up
* being disabled.
*
* @see #expression
*/
boolean loadContext() default false;
}

3
spring-test/src/main/java/org/springframework/test/context/junit/jupiter/EnabledIfCondition.java

@ -60,7 +60,8 @@ public class EnabledIfCondition extends AbstractExpressionEvaluatingCondition { @@ -60,7 +60,8 @@ public class EnabledIfCondition extends AbstractExpressionEvaluatingCondition {
}
private ConditionEvaluationResult evaluateEnabledIf(ExtensionContext context) {
return evaluateAnnotation(EnabledIf.class, EnabledIf::expression, EnabledIf::reason, true, context);
return evaluateAnnotation(EnabledIf.class, EnabledIf::expression, EnabledIf::reason,
EnabledIf::loadContext, true, context);
}
}

23
spring-test/src/test/java/org/springframework/test/context/junit/jupiter/DisabledIfConditionTestCase.java

@ -76,11 +76,26 @@ class DisabledIfConditionTestCase { @@ -76,11 +76,26 @@ class DisabledIfConditionTestCase {
@Test
void invalidExpressionEvaluationType() {
String methodName = "nonBooleanOrStringExpression";
IllegalStateException exception = expectThrows(IllegalStateException.class,
() -> condition.evaluate(buildExtensionContext("nonBooleanOrStringExpression")));
() -> condition.evaluate(buildExtensionContext(methodName)));
Method method = ReflectionUtils.findMethod(getClass(), methodName);
assertThat(exception.getMessage(),
is(equalTo("@DisabledIf(\"#{6 * 7}\") on " + method + " must evaluate to a String or a Boolean, not java.lang.Integer")));
}
@Test
void unsupportedStringEvaluationValue() {
String methodName = "stringExpressionThatIsNeitherTrueNorFalse";
IllegalStateException exception = expectThrows(IllegalStateException.class,
() -> condition.evaluate(buildExtensionContext(methodName)));
Method method = ReflectionUtils.findMethod(getClass(), methodName);
assertThat(exception.getMessage(),
is(equalTo("@DisabledIf(\"#{6 * 7}\") must evaluate to a String or a Boolean, not java.lang.Integer")));
is(equalTo("@DisabledIf(\"#{'enigma'}\") on " + method + " must evaluate to \"true\" or \"false\", not \"enigma\"")));
}
@Test
@ -151,6 +166,10 @@ class DisabledIfConditionTestCase { @@ -151,6 +166,10 @@ class DisabledIfConditionTestCase {
private void nonBooleanOrStringExpression() {
}
@DisabledIf("#{'enigma'}")
private void stringExpressionThatIsNeitherTrueNorFalse() {
}
@DisabledIf(expression = "#{6 * 7 == 42}", reason = "Because... 42!")
private void customReason() {
}

17
spring-test/src/test/java/org/springframework/test/context/junit/jupiter/DisabledIfTestCase.java

@ -23,7 +23,8 @@ import org.springframework.context.annotation.Bean; @@ -23,7 +23,8 @@ import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.TestPropertySource;
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.fail;
/**
* Integration tests which verify support for {@link DisabledIf @DisabledIf}
@ -63,13 +64,19 @@ class DisabledIfTestCase { @@ -63,13 +64,19 @@ class DisabledIfTestCase {
}
@Test
@DisabledIf("${foo}")
@DisabledIf("${__EnigmaPropertyShouldNotExist__:true}")
void disabledIfWithPropertyPlaceholderForNonexistentPropertyWithDefaultValue() {
fail("This test must be disabled");
}
@Test
@DisabledIf(expression = "${foo}", loadContext = true)
void disabledIfWithPropertyPlaceholder() {
fail("This test must be disabled");
}
@Test
@DisabledIf("\t${foo} ")
@DisabledIf(expression = "\t${foo} ", loadContext = true)
void disabledIfWithPropertyPlaceholderWithSurroundingWhitespace() {
fail("This test must be disabled");
}
@ -105,13 +112,13 @@ class DisabledIfTestCase { @@ -105,13 +112,13 @@ class DisabledIfTestCase {
}
@Test
@DisabledIf("#{@booleanTrueBean}")
@DisabledIf(expression = "#{@booleanTrueBean}", loadContext = true)
void disabledIfWithSpelBooleanTrueBean() {
fail("This test must be disabled");
}
@Test
@DisabledIf("#{@stringTrueBean}")
@DisabledIf(expression = "#{@stringTrueBean}", loadContext = true)
void disabledIfWithSpelStringTrueBean() {
fail("This test must be disabled");
}

15
spring-test/src/test/java/org/springframework/test/context/junit/jupiter/EnabledIfTestCase.java

@ -18,6 +18,7 @@ package org.springframework.test.context.junit.jupiter; @@ -18,6 +18,7 @@ package org.springframework.test.context.junit.jupiter;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.TestPropertySource;
@ -64,13 +65,19 @@ class EnabledIfTestCase { @@ -64,13 +65,19 @@ class EnabledIfTestCase {
}
@Test
@EnabledIf("${foo}")
@EnabledIf("${__EnigmaPropertyShouldNotExist__:false}")
void enabledIfWithPropertyPlaceholderForNonexistentPropertyWithDefaultValue() {
fail("This test must be disabled");
}
@Test
@EnabledIf(expression = "${foo}", loadContext = true)
void enabledIfWithPropertyPlaceholder() {
fail("This test must be disabled");
}
@Test
@EnabledIf("\t${foo} ")
@EnabledIf(expression = "\t${foo} ", loadContext = true)
void enabledIfWithPropertyPlaceholderWithSurroundingWhitespace() {
fail("This test must be disabled");
}
@ -108,13 +115,13 @@ class EnabledIfTestCase { @@ -108,13 +115,13 @@ class EnabledIfTestCase {
}
@Test
@EnabledIf("#{@booleanFalseBean}")
@EnabledIf(expression = "#{@booleanFalseBean}", loadContext = true)
void enabledIfWithSpelBooleanFalseBean() {
fail("This test must be disabled");
}
@Test
@EnabledIf("#{@stringFalseBean}")
@EnabledIf(expression = "#{@stringFalseBean}", loadContext = true)
void enabledIfWithSpelStringFalseBean() {
fail("This test must be disabled");
}

Loading…
Cancel
Save