Support multiple AfterAssertionErrorCollected callbacks

[Achitheus]

Check List:

• Fixes -> ignore
• Unit tests : YES
• Javadoc with a code example (on API only) : NA
• PR meets the contributing guidelines

Let AssertJ integrations safely rely on this fuctionality!

Without this multiple callbacks support there is a problem with using DefaultAssertionErrorCollector.setAfterAssertionErrorCollected() method.
It can be used only by user at the moment, but not by another frameworks developers .
Why do I think so? Because callback field in the DefaultAssertionErrorCollector class is not instance of any Collection<AfterAssertionErrorCollected> type, but is just AfterAssertionErrorCollected. Thus setAfterAssertionErrorCollected() method can be executed only once for each SoftAssertions object (by exact single actor - user), because every next call leads to overriding and erasing previously set callback.

[joel-costigliola]

@Achitheus that's interesting, could you give us a real life use case to show why this could be useful ?

[Achitheus]

@Achitheus that's interesting, could you give us a real life use case to show why this could be useful ?

@joel-costigliola Im gonna add AssertJ soft assertions support for java Allure reports. I am already done with it and even tested, but haven't suggested PR at the moment. My solution relys on this functionality to detect assertion error and mark current step as failed and then escalate this status to all outer steps of the step chain.
But there is a question. What if user also try to set his own callback for logs for example or what if he add dependency which also use this feature? Conflicts will definitely happen. And in this conflict, whoever is last is the winner because he rewrited all previous callbacks (only the last one to be precise). That's why the change is usefull

[Achitheus]

I should have pushed several commits at once. Sorry. I hope you don't have github actions proccesing time restrictions or something :)

[Achitheus]

@joel-costigliola what do you think? Could you please run actions one more time?

[joel-costigliola]

Sorry I missed your reply, I'll think about it this week end, but gut feeling is it should be ok

[Achitheus]

@joel-costigliola I'm not rushing you but just reminding :)

[joel-costigliola]

Thanks for the reminder, I'll put it for the next version 3.26.0

[joel-costigliola]

@Achitheus thanks, PR commented.

[Achitheus]

@joel-costigliola Thank you for your insightful comments! Let me know if there is anything else I can do/fix.

[Achitheus]

@joel-costigliola I think I finally realized my mistake. I should use mvn verify instead of mvn test to see locally format violations and license issues.
I hope next time everything will be "green" :) Sorry

[joel-costigliola]

Second round of comments, thanks @Achitheus

[joel-costigliola]

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

[Achitheus]

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?

[Achitheus]

@joel-costigliola build success locally.
As I mentioned above, the queston of the right place for setAfter...() method test is still open for me. I left it in the current test class for now.
I'm ready for the third round :) Let me know if there is anything else I can do!

[joel-costigliola]

Integrated thanks @Achitheus !