assertj: SoftAssertions for JUnit Jupiter should reset tracked exceptions between tests

Overview

The following code looks up tracked exceptions in an instance field.

https://github.com/joel-costigliola/assertj-core/blob/9692f7dc339765e9d7ef2725c89b3241c613268f/src/main/java/org/assertj/core/api/JUnitJupiterSoftAssertions.java#L45

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

Most upvoted comments

OK. I’ve already implemented a spike.

/*
 * Copyright 2012-2019 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
 *
 *      https://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.assertj.core.api.junit.jupiter;

import org.assertj.core.api.SoftAssertions;
import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.junit.jupiter.api.extension.ExtensionContext.Store;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolver;

/**
 * Extension for JUnit Jupiter that provides support for injecting an
 * instance of {@link SoftAssertions} into test methods and test-level lifecycle
 * methods (i.e., {@code @BeforeEach} and {@code @AfterEach} methods).
 *
 * <h3>Applicability</h3>
 *
 * <p>In this context, the term "test method" refers to any method annotated with
 * {@code @Test}, {@code @RepeatedTest}, {@code @ParameterizedTest},
 * {@code @TestFactory}, or {@code @TestTemplate}. This extension does not inject
 * {@link SoftAssertions} arguments into test constructors or class-level lifecycle
 * methods.
 *
 * <h3>Scope</h3>
 *
 * <p>The scope of the {@code SoftAssertions} instance managed by this extension
 * begins when a parameter of type {@code SoftAssertions} is resolved for a
 * {@code @BeforeEach} method, test method, or {@code @AfterEach} method. The
 * scope of the instance ends after all {@code @AfterEach} methods have been
 * executed for the corresponding test. When the scope ends,
 * {@link SoftAssertions#assertAll() assertAll()} will invoked on the instance
 * to verify that no soft assertions failed.
 *
 * <h3>Example</h3>
 *
 * <pre class="code">
 * {@literal @}ExtendWith(SoftAssertionsExtension.class)
 * class ExampleTestCase {
 * 
 *    {@literal @}Test
 *    void multipleFailures(SoftAssertions softly) {
 *       softly.assertThat(2 * 3).isEqualTo(0);
 *       softly.assertThat(Arrays.asList(1, 2)).containsOnly(1);
 *       softly.assertThat(1 + 1).isEqualTo(2);
 *    }
 * }</pre>
 *
 * @author Sam Brannen
 * @since 3.13
 */
public class SoftAssertionsExtension implements ParameterResolver, AfterEachCallback {

	@Override
	public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
		boolean isTestLevel = extensionContext.getTestMethod().isPresent();
		boolean isSoftAssertionsParameter = parameterContext.getParameter().getType() == SoftAssertions.class;
		return (isTestLevel && isSoftAssertionsParameter);
	}

	@Override
	public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
		return getStore(extensionContext).getOrComputeIfAbsent(SoftAssertions.class);
	}

	@Override
	public void afterEach(ExtensionContext extensionContext) throws Exception {
		SoftAssertions softAssertions = getStore(extensionContext).get(SoftAssertions.class, SoftAssertions.class);
		if (softAssertions != null) {
			softAssertions.assertAll();
		}
	}

	private Store getStore(ExtensionContext extensionContext) {
		return extensionContext.getStore(Namespace.create(getClass(), extensionContext.getUniqueId()));
	}

}

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’s ExtensionContext.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.

import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;

import org.assertj.core.api.JUnitJupiterSoftAssertions;
import org.junit.jupiter.api.MethodOrderer.Alphanumeric;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.api.extension.RegisterExtension;

@TestMethodOrder(Alphanumeric.class)
@TestInstance(PER_CLASS)
class AssertJSoftAssertionsExtensionTests {

	@RegisterExtension
	final JUnitJupiterSoftAssertions softly = new JUnitJupiterSoftAssertions();

	@Test
	void test1() {
		// should fail
		softly.assertThat(1).isLessThan(0);
	}

	@Test
	void test2() {
		// should pass
		softly.assertThat(0).isLessThan(1);
	}

}

@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 just commented the PR I have created from your branch, ignore the comments if they are not relevant anymore. Let me know when it is ready / good enough for a review.

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.

IMO the scope should only be the @Test methods, @BeforeEach or @AfterEach are not places to perform assertions.

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 of SoftAssertions 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 a ThreadLocal.

Such an extension would likely be better registered via @ExtendWith instead of @RegisterExtension since the field would no longer be useful to the user.