Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

@DirtiesContext not applied when class-level @EnabledIf evaluates to false #26694

Closed
igorakkerman opened this issue Mar 17, 2021 · 8 comments
Closed

@DirtiesContext not applied when class-level @EnabledIf evaluates to false #26694

igorakkerman opened this issue Mar 17, 2021 · 8 comments
Assignees
@sbrannen
Labels
in: test Issues in the test module status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone
5.3.6

Comments

@igorakkerman
Copy link

When a JUnit 5 test class is annotated with @EnabledIf and @DirtiesContext with @EnabledIf's property loadContext set to true, if its expression evaluates to false, the context loaded through the annotation is not cleaned up afterwards.

If the expression evaluates to true and the tests are actually run, the context is cleaned up correctly.

A workaround is to put @EnabledIf on each test method, instead of the class.

I ran into this issue running Kafka container tests conditionally, when Kafka consumers were created through @EnableAutoConfiguration and were left over after the test was skipped. They were consuming messages of other tests that would run afterwards, breaking those tests.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 17, 2021
@sbrannen sbrannen added the in: test Issues in the test module label Mar 17, 2021
@sbrannen
Copy link
Member

That sounds like an interesting combination of actors at play.

I'll investigate if we can improve that.

Thanks for raising the issue!

@sbrannen sbrannen self-assigned this Mar 17, 2021
@sbrannen sbrannen added this to the 5.3.6 milestone Mar 17, 2021
@sbrannen sbrannen added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 17, 2021
@sbrannen
Copy link
Member

Just to clarify, do you have a bare @DirtiesContext declaration at the class level (or an explicit @DirtiesContext(classMode = ClassMode.AFTER_CLASS))?

@sbrannen sbrannen added the status: waiting-for-feedback We need additional information before we can continue label Mar 17, 2021
@sbrannen
Copy link
Member

The reason you are experiencing this behavior is that a JUnit Jupiter ExecutionCondition that evaluates to disabled results in the test class not being executed, which means that class-level callbacks are not executed, which means that the DirtiesContextTestExecutionListener is never invoked for a disabled test class.

So, to get this to work, we'll have to add some special logic to the AbstractExpressionEvaluatingCondition in spring-test to honor the @DirtiesContext annotation in such scenarios.

@igorakkerman
Copy link
Author

Just to clarify, do you have a bare @DirtiesContext declaration at the class level (or an explicit @DirtiesContext(classMode = ClassMode.AFTER_CLASS))?

I had tried all three options (bare, AFTER_CLASS and AFTER_EACH_TEST_METHOD) and verified it just now. All three show the same behavior.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 17, 2021
@sbrannen
Copy link
Member

I had tried all three options (bare, AFTER_CLASS and AFTER_EACH_TEST_METHOD) and verified it just now. All three show the same behavior.

Thanks for the feedback.

See #26694 (comment) for my analysis of why it doesn't work for all options.

@sbrannen sbrannen added type: bug A general bug for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x and removed status: feedback-provided Feedback has been provided type: enhancement A general enhancement labels Mar 18, 2021
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.2.x Marks an issue as a candidate for backport to 5.2.x labels Mar 18, 2021
Closed
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Mar 18, 2021
@sbrannen
Honor class-level @DirtiesContext if test class is disabled via SpEL
4a6fea3 
Prior to this commit, if a test class annotated with @DirtiesContext
and @EnabledIf/@DisabledIf with `loadContext = true` was disabled due
to the evaluated SpEL expression, the ApplicationContext would not be
marked as dirty and closed.

The reason is that @EnabledIf/@DisabledIf are implemented via JUnit
Jupiter's ExecutionCondition extension API which results in the entire
test class (as well as any associated extension callbacks) being
skipped if the condition evaluates to `disabled`. This effectively
prevents any of Spring's TestExecutionListener APIs from being invoked.
Consequently, the DirtiesContextTestExecutionListener does not get a
chance to honor the class-level @DirtiesContext declaration.

This commit fixes this by implementing part of the logic of
DirtiesContextTestExecutionListener in
AbstractExpressionEvaluatingCondition (i.e., the base class for
@EnabledIf/@DisabledIf support). Specifically, if the test class for an
eagerly loaded ApplicationContext is disabled,
AbstractExpressionEvaluatingCondition will now mark the test
ApplicationContext as dirty if the test class is annotated with
@DirtiesContext.

Closes spring-projectsgh-26694
@igorakkerman
Copy link
Author

Wow, amazing response and resolution time. Thank you so much for acting so quickly!

@sbrannen
Copy link
Member

Wow, amazing response and resolution time. Thank you so much for acting so quickly!

You're welcome.

Please note that this has been fixed in master and backported to 5.2.x.

Feel free to try it out in the latest snapshots for 5.3.6 and 5.2.14 and let us know if it works for you.

@igorakkerman
Copy link
Author

Works for me with Spring Boot 2.4.4 and Spring Test 5.3.6-SNAPSHOT.
Thanks!

@sync-by-unito sync-by-unito bot mentioned this issue Apr 14, 2021
Merged
igorakkerman added a commit to igorakkerman/kafka-kotlin-demo that referenced this issue Apr 17, 2021
@igorakkerman
@testcontainers annotation moved to TestContainersConfiguration,
bcc2759 
avoiding the Spring bug spring-projects/spring-framework#26694
igorakkerman added a commit to igorakkerman/kafka-kotlin-demo that referenced this issue Apr 17, 2021
@igorakkerman
@testcontainers annotation moved to TestContainersConfiguration,
31c0a4c 
avoiding the Spring bug spring-projects/spring-framework#26694
Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this issue Aug 20, 2021
@sbrannen @Zoran0104
Honor class-level @DirtiesContext if test class is disabled via SpEL
920fb23 
Prior to this commit, if a test class annotated with @DirtiesContext
and @EnabledIf/@DisabledIf with `loadContext = true` was disabled due
to the evaluated SpEL expression, the ApplicationContext would not be
marked as dirty and closed.

The reason is that @EnabledIf/@DisabledIf are implemented via JUnit
Jupiter's ExecutionCondition extension API which results in the entire
test class (as well as any associated extension callbacks) being
skipped if the condition evaluates to `disabled`. This effectively
prevents any of Spring's TestExecutionListener APIs from being invoked.
Consequently, the DirtiesContextTestExecutionListener does not get a
chance to honor the class-level @DirtiesContext declaration.

This commit fixes this by implementing part of the logic of
DirtiesContextTestExecutionListener in
AbstractExpressionEvaluatingCondition (i.e., the base class for
@EnabledIf/@DisabledIf support). Specifically, if the test class for an
eagerly loaded ApplicationContext is disabled,
AbstractExpressionEvaluatingCondition will now mark the test
ApplicationContext as dirty if the test class is annotated with
@DirtiesContext.

Closes spring-projectsgh-26694
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this issue Mar 26, 2022
@sbrannen @lxbzmy
Honor class-level @DirtiesContext if test class is disabled via SpEL
1d46e79 
Prior to this commit, if a test class annotated with @DirtiesContext
and @EnabledIf/@DisabledIf with `loadContext = true` was disabled due
to the evaluated SpEL expression, the ApplicationContext would not be
marked as dirty and closed.

The reason is that @EnabledIf/@DisabledIf are implemented via JUnit
Jupiter's ExecutionCondition extension API which results in the entire
test class (as well as any associated extension callbacks) being
skipped if the condition evaluates to `disabled`. This effectively
prevents any of Spring's TestExecutionListener APIs from being invoked.
Consequently, the DirtiesContextTestExecutionListener does not get a
chance to honor the class-level @DirtiesContext declaration.

This commit fixes this by implementing part of the logic of
DirtiesContextTestExecutionListener in
AbstractExpressionEvaluatingCondition (i.e., the base class for
@EnabledIf/@DisabledIf support). Specifically, if the test class for an
eagerly loaded ApplicationContext is disabled,
AbstractExpressionEvaluatingCondition will now mark the test
ApplicationContext as dirty if the test class is annotated with
@DirtiesContext.

Closes spring-projectsgh-26694
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sbrannen sbrannen

Labels
in: test Issues in the test module status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Milestone
5.3.6
Development

No branches or pull requests
3 participants
@sbrannen @igorakkerman @spring-projects-issues