assertj: SoftAssertions for JUnit Jupiter should reset tracked exceptions between tests
Overview
The following code looks up tracked exceptions in an instance field.
The problem with doing that is that the tracked exceptions do not get reset between test method invocations if the test class is configured with @TestInstance(PER_CLASS)
lifecycle semantics, since the test instance is cached along with the instance of JUnitJupiterSoftAssertions
stored in an instance field.
I became aware of this due to the following comment in JUnit’s issue tracker: https://github.com/junit-team/junit5/issues/1500#issuecomment-459720295
Affected Extensions
JUnitJupiterSoftAssertions
JUnitJupiterBDDSoftAssertions
- any other extensions implemented in a similar manner
Proposals
Modify JUnitJupiterSoftAssertions
so that it implements JUnit Jupiter’s BeforeEachCallback
extension API and clears the collected errors in SoftProxies
before each test method.
Another option would be to store the collected errors in Jupiter’s ExtensionContext.Store
for the currently executing scope.
Relate Issues
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 44 (41 by maintainers)
Commits related to this issue
- [WIP] Introduce SoftAssertionsExtension for JUnit Jupiter Closes #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Polishing See #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Introduce @Nested tests for SoftAssertionsExtension See #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Support BDDSoftAssertions in SoftAssertionsExtension See #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Polishing See #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Polishing See #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Limit scope to @Testable methods in SoftAssertionsExtension See #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Simplify implementation of SoftAssertionsExtension ... based on feedback from @joel-costigliola. See #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Test SoftAssertionsExtension with the JUnit Platform Test Kit See #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Throw exception if SoftAssertionsExtension is used with constructor or lifecycle method See #1418 — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Introduce SoftAssertionsExtension for JUnit Jupiter Prior to this commit, AssertJ offered JUnitJupiterSoftAssertions and JUnitJupiterBDDSoftAssertions for working with SoftAssertions or BDDSoftAssert... — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Introduce RegisterExtension for JUnit Jupiter Prior to this commit, AssertJ offered JUnitJupiterSoftAssertions and JUnitJupiterBDDSoftAssertions for working with SoftAssertions or BDDSoftAssertions i... — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Disable example test cases for SoftAssertionsExtension Prior to this commit, the example test cases used to test the SoftAssertionsExtension were properly excluded from the Maven build by intentional... — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Disable example test cases for SoftAssertionsExtension Prior to this commit, the example test cases used to test the SoftAssertionsExtension were properly excluded from the Maven build by intentional... — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Disable example test cases for SoftAssertionsExtension Prior to this commit, the example test cases used to test the SoftAssertionsExtension were properly excluded from the Maven build by intentional... — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Disable example test cases for SoftAssertionsExtension Prior to this commit, the example test cases used to test the SoftAssertionsExtension were properly excluded from the Maven build by intentional... — committed to sbrannen/assertj-core by sbrannen 5 years ago
- Disable example test cases for SoftAssertionsExtension Prior to this commit, the example test cases used to test the SoftAssertionsExtension were properly excluded from the Maven build by intentional... — committed to assertj/assertj by sbrannen 5 years ago
OK. I’ve already implemented a spike.
You’re very welcome.
And… now that I’ve thought about it again, I think the only robust solution is to store the current instance of
SoftProxies
in Jupiter’sExtensionContext.Store
.Otherwise, you will also run into issues with parallel test execution as well as the aforementioned issues with test instance per-class lifecycle semantics and nested test class.
Here is a test case in Java (using
@TestMethodOrder
support from the upcoming JUnit Jupiter 5.4 release) that reproduces the problem.@sbrannen - Came here just to find out why
JUnitJupiterSoftAssertions
was deprecated, ended up learning so much about JUnit5 extensions. Your patient and detailed explanations are truly inspiring, thanks and keep it up.I’ll deprecate them in favor of the extension, I don’t think they must be fixed.
@sbrannen I’m happy with the approach 😃 waiting for your PR.
I have addressed your comments and made changes.
At this point, I think the
SoftAssertionsExtension
implementation is nearly complete (if not already complete). So, feel free to review that.The tests, however, are just a playground, and some of them fail. So that’s obviously not up for review yet. I’ll have to use the
EngineTestKit
to test those properly.I just made that the case in https://github.com/sbrannen/assertj-core/commit/88736c26762d34191daa82fa48607bbc360caa2a. 😉
I pushed my work to a branch in my fork, in case that makes it easier for you to view: https://github.com/sbrannen/assertj-core/commits/issues/1418-jupiter-soft-assertions
Another option would be for the extension to implement
ParameterResolver
and then inject an instance ofSoftAssertions
into each method that needs it.I think that would actually be cleaner, since it should theoretically work via the
ExtensionContext.Store
and would no longer require aThreadLocal
.Such an extension would likely be better registered via
@ExtendWith
instead of@RegisterExtension
since the field would no longer be useful to the user.