Assertj-core: Softly/SoftAssertionsExtension revamp

[kriegfrj]

I have a number of suggestions regarding these two extensions that I wanted to float - they could be done as separate issues but they are tightly coupled so I'm going to include them in one issue for now and we can split them up later if you feel that this is appropriate.

SoftlyExtension doesn't support custom SoftAssertionsProvider

I find the field-injection style of SoftlyExtension much preferable to parameter injections. Not only does it simplify the method signatures of the tests, but you can easily have common methods that can perform a common set of (soft) assertions on some objects without having to pass the SoftAssertions reference around. I find that in many of my SoftAssertionExtension implementations, I have a line at the top of them all that says "this.softly = softly", which is boilerplate code polluting every test. The alternative is to declare SoftAssertions as a parameter to all of the common methods too, which is even less attractive.

However, SoftlyExtension has the drawback that it is hard-coded to use SoftAssertions. This means you miss out on the ability to use

Combine SoftlyExtension and SoftAssertionsExtension

In osgi-test, we've settled upon a pattern of extensions that support both field and parameter injection. This allows them to make use of common code (eg, instantiating the object to be injected) and it makes it easier for the user as they don't have to make a special effort to remember which one to use.

I can't see any good reason not to merge these two, but perhaps someone else can. Failing that, I suggest that we merge them (which will go a significant part of the way to solving the first point). Given that SoftAssertionsExtension has been around longer, I'm going to suggest merging SoftlyExtension into it and then deprecating SoftlyExtension.

Adding API for third-party extensions

If you're using multiple soft assertion-type libraries, it is nice if you can combine the two into a common "multiple failure". This was the thrust behind #1459.

@fduminy came up with a novel way to combine it with Mockito soft verification (see https://github.com/fduminy/assertj-mockito). The only problem with this implementation is that it relies on what is effectively private API of SoftAssertionsExtension to work, as it maintains its own copy of has to reach into the extension context and peek into SoftAssertionExtension's private store to get the current soft assertion instance to re-use. It would be nice if this aspect of it was publicised so that AssertJMockitoExtension and other 3rd-party extensions can get access to the soft assertion object for the current context.

In osgi-test, we hit upon the idea of using a public static method that takes an ExtensionContext as a parameter, which is responsible for fetching the object from the store (and creating it if necessary). It is possible to use a static method because the extensions themselves are stateless (or supposed to be, anyway!) and store all of their state in the ExtensionContext.

It would be nice if these extensions offered the same ability, which would help clean up code like AssertJMockitoExtension. If we merge the two extensions, this will only need to be done once - if we do not merge them, then we could consider making one depend on the other's public static initalisation method (which would go part way to solving the first point too).

[joel-costigliola]

Will look into this in the next release.

[kriegfrj]

I'm keen for this ASAP. If I do the work can we squeeze it in to 3.17?

[scordio]

Unfortunately it's not possible, 3.17 is already out. But we could have a faster 3.18 :-)

[kriegfrj]

I'm disappointed in your lack of commitment.
What, your time travel device isn't working or something? 😄

[joel-costigliola]

I guess your time travel device is not working too @kriegfrj :laughing:

[scordio]

It's been broken for a while!

[joel-costigliola]

SoftlyExtension doesn't support custom SoftAssertionsProvider

I agree it should, will also add support for BDDSoftAssertions.

Combine SoftlyExtension and SoftAssertionsExtension

Agreed too if that is technically feasible (which I think it is).

Adding API for third-party extensions

I'm ok with the idea as long as it does not add too much complexity.

@kriegfrj the first two suggestions can be addressed in the same PR, I prefer the last one to be done separately.

[kriegfrj]

SoftlyExtension doesn't support custom SoftAssertionsProvider

I agree it should, will also add support for BDDSoftAssertions.

:+1:

Combine SoftlyExtension and SoftAssertionsExtension

Agreed too if that is technically feasible (which I think it is).

👍If I do this, should i deprecate SoftlyExtension at the same time?

Adding API for third-party extensions

I'm ok with the idea as long as it does not add too much complexity.

On the contrary we've found it simplifies it a bit.

@kriegfrj the first two suggestions can be addressed in the same PR, I prefer the last one to be done separately.

Ok. In that case, i will address the last first, otherwise I'll have to refactor the other PR once the last is done.