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.

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

Support multiple AfterAssertionErrorCollected callbacks #3313

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public interface AfterAssertionErrorCollected {
* assertion errors collection is implemented.
* <p>
* If you just use the standard soft assertions classes provided by AssertJ, you can register your callback with
* {@link AbstractSoftAssertions#setAfterAssertionErrorCollected(AfterAssertionErrorCollected)}.
* {@link AbstractSoftAssertions#addAfterAssertionErrorCollected(AfterAssertionErrorCollected)}.
* <p>
* Example with custom soft assertions:
* <pre><code class='java'> class TolkienSoftAssertions extends SoftAssertions {
Expand All @@ -51,7 +51,7 @@ public interface AfterAssertionErrorCollected {
* <pre><code class='java'> SoftAssertions softly = new SoftAssertions();
*
* // register our callback
* softly.setAfterAssertionErrorCollected(error -&gt; System.out.println(error));
* softly.addAfterAssertionErrorCollected(error -&gt; System.out.println(error));
*
* // the AssertionError corresponding to this failing assertion is printed to the console.
* softly.assertThat("The Beatles").isEqualTo("The Rolling Stones");</code></pre>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,13 @@ public class DefaultAssertionErrorCollector implements AssertionErrorCollector {
private volatile boolean wasSuccess = true;
private List<AssertionError> collectedAssertionErrors = synchronizedList(new ArrayList<>());

private AfterAssertionErrorCollected callback = this;
private List<AfterAssertionErrorCollected> callbacks = synchronizedList(new ArrayList<>());

private AssertionErrorCollector delegate = null;

public DefaultAssertionErrorCollector() {
super();
callbacks.add(this);
}

// I think ideally, this would be set in the constructor and made final;
Expand All @@ -57,7 +58,7 @@ public void collectAssertionError(AssertionError error) {
} else {
delegate.collectAssertionError(error);
}
callback.onAssertionErrorCollected(error);
callbacks.forEach(callback -> callback.onAssertionErrorCollected(error));
}

/**
Expand All @@ -74,6 +75,21 @@ public List<AssertionError> assertionErrorsCollected() {
return decorateErrorsCollected(errors);
}

/**
* Same as {@link DefaultAssertionErrorCollector#addAfterAssertionErrorCollected(AfterAssertionErrorCollected)}, but
* also removes all previously added callbacks. Please consider using
* {@link DefaultAssertionErrorCollector#addAfterAssertionErrorCollected(AfterAssertionErrorCollected)}
* instead, because another frameworks and integrations can also use this functionality.
*
* @param afterAssertionErrorCollected the callback.
*
* @since 3.17.0
*/
public void setAfterAssertionErrorCollected(AfterAssertionErrorCollected afterAssertionErrorCollected) {
callbacks.clear();
addAfterAssertionErrorCollected(afterAssertionErrorCollected);
}

/**
* Register a callback allowing to react after an {@link AssertionError} is collected by the current soft assertion.
* <p>
Expand Down Expand Up @@ -111,10 +127,10 @@ public List<AssertionError> assertionErrorsCollected() {
*
* @param afterAssertionErrorCollected the callback.
*
* @since 3.17.0
* @since 3.26.0
*/
public void setAfterAssertionErrorCollected(AfterAssertionErrorCollected afterAssertionErrorCollected) {
callback = afterAssertionErrorCollected;
public void addAfterAssertionErrorCollected(AfterAssertionErrorCollected afterAssertionErrorCollected) {
callbacks.add(afterAssertionErrorCollected);
Achitheus marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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.
*
* Copyright 2012-2024 the original author or authors.
*/
package org.assertj.core.api;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.List;

import static org.assertj.core.api.BDDAssertions.then;

class DefaultAssertionErrorCollector_addAfterAssertionErrorCollected_Test {
private List<String> errorMessages;
private List<Throwable> throwables;
private SoftAssertions softly;

@BeforeEach
void given() {
errorMessages = new ArrayList<>();
throwables = new ArrayList<>();
softly = new SoftAssertions();
}

@Test
void should_perform_both_specified_actions_on_each_assertion_error() {
// WHEN
softly.addAfterAssertionErrorCollected(err -> errorMessages.add(err.getMessage()));
softly.addAfterAssertionErrorCollected(throwables::add);
softly.assertThat(1)
.isEqualTo(1_000)
.isBetween(7, 15)
.isEven();
// THEN
then(errorMessages).hasSameSizeAs(throwables)
.hasSize(3);
}

@Test
void should_execute_both_specified_actions_on_each_manually_added_error() {
// WHEN
softly.addAfterAssertionErrorCollected(err -> errorMessages.add(err.getMessage()));
softly.addAfterAssertionErrorCollected(throwables::add);
softly.collectAssertionError(new AssertionError("hello"));
softly.collectAssertionError(new AssertionError("world"));
// THEN
then(errorMessages).hasSameSizeAs(throwables)
.hasSize(2);
}

@Test
void should_register_the_same_callback_several_times() {
// GIVEN
AfterAssertionErrorCollected callback = throwables::add;
// WHEN
for (int i = 0; i < 10; i++) {
softly.addAfterAssertionErrorCollected(callback);
}
softly.collectAssertionError(new AssertionError("hello"));
// THEN
then(throwables).hasSize(10);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a unit test to check that setAfterAssertionErrorCollected replace all registered callback with the one specified

Copy link
Contributor Author

@Achitheus Achitheus Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. But I would like to clarify whether this test should be moved from current class
DefaultAssertionErrorCollector_addAfterAssertionErrorCollected_Test
to this new one
DefaultAssertionErrorCollector_setAfterAssertionErrorCollected_Test
?
Or even moved to org/example/test/DefaultAssertionErrorCollector_Test.java
Or let it be in the current class, but rename the class to
DefaultAssertionErrorCollector_afterAssertionErrorCollected_methods_Test
or
DefaultAssertionErrorCollector_callbacks_Test
or something?


@Test
void setAfterAssertionErrorCollected_should_replace_all_registered_callbacks_with_one_specified() {
// GIVEN
for (int i = 0; i < 3; i++) {
softly.addAfterAssertionErrorCollected(err -> errorMessages.add(err.getMessage()));
}
// WHEN
softly.setAfterAssertionErrorCollected(throwables::add);
softly.collectAssertionError(new AssertionError("hello"));
// THEN
then(errorMessages).hasSize(0);
then(throwables).hasSize(1);
}
}
Loading