Replace Assert implementation by AssertJ

[juherr]

Recently, we had many issues with Assertion. See #2643 #2636 #2540

I think it could be helpful to deprecate the internal assertion API and let the user choose the library he prefers.
It will allow us to drop some annex subjects and be more focused on the test engine.

One option could be to shade AssertJ into the TestNG jar and use it as the default implementation of Assertion.

@krmahadevan WDYT?

[Elisedlund-ericsson]

I personally always recommend AssertJ as the preferred Assertion API
However we could never get away from using org.testng.Assert as the amount of tests that uses it are astronomical, Some untouched since forever but still executed on a regular basis.

We never recommend anyone to migrate from Assert to AssertJ for any existing test cases.
but we do recommend new test cases to use AssertJ instead.

Would deprecation mean that it would be removed sometime in the future?
Possibly moving it out to an optional module could be an alternative, Then not having it shaded into the TestNG jar, Existing old users add it as a separate dependency for legacy compatibility

[juherr]

True, optional dependency is an option too.

Shades the dependency under a private package will allow users to use another version than the one used by TestNG itself.

[krmahadevan]

@juherr so if I understand this correctly you are looking at having the Assert class internally work with an assertion implementation ( by default testng would use assertj) and if a user wants they can provide an implementation of our interface and plugin the implementation via a configuration and testng would use that one ? Is that what you are hinting at ?

Yes I also feel that the testng assertions can be deprecated and users can instead use an assertion dedicated library such as assertj. As you said it would help us focus on the test engine.

[juherr]

I don't think we need a spi here.
The idea is just replacing the current implementation by assertj and transfert the responsability there.
For a better transition, I'd prefer the shade it because it will avoid the clash of version if a user already uses assertj.
Once removed from the project, we will be able to provide it in a dedicated jar.

[Elisedlund-ericsson]

I dont see what kind of interfaces or spi that would be needed,

I think most assertion libraries only relies on the "existing spi" like:
java.lang.AssertionError and org.testng.SkipException
and possibly for some mechanism for automatically apply soft assertions at the end of a test case. (which does not require Listerner use)

[krmahadevan]

@juherr

I don't think we need a spi here.

I was not referring to spi, but I was referring to the sort of implementation that we did for the scripting support in method selectors (beanshell).

The idea is just replacing the current implementation by assertj and transfert the responsability there.

Also if we end up adding assertj calls to all the APIs exposed by Assert class, then we would be adding an explicit dependency on assertj. Is that not going to be a problem ?

For a better transition, I'd prefer the shade it because it will avoid the clash of version if a user already uses assertj. Once removed from the project, we will be able to provide it in a dedicated jar.

Can you please elaborate on what do you mean by shading it? Are you referring to the shading functionality as done by this maven plugin ? https://maven.apache.org/plugins/maven-shade-plugin/index.html

We dont give a fat jar to our users right. We give them as a maven dependency.

[krmahadevan]

@Elisedlund-ericsson

I dont see what kind of interfaces or spi that would be needed,

I think most assertion libraries only relies on the "existing spi" like: java.lang.AssertionError and org.testng.SkipException and possibly for some mechanism for automatically apply soft assertions at the end of a test case. (which does not require Listerner use)

I was referring to an interface driven approach which internally uses an assertj default implementation so that it becomes completely plug and play. This way one could use any different assertion implementation as well (something like a logging assertion, which asserts and logs into a log file or into a test result etc.,)

[juherr]

I propose a step by step strategy where the objective is to drop the current assert implementation.

1. Deprecate + Use assertj under the hood, included into the jar under a custom package (no dependency issue for users)
2. Remove the deprecated classes and provide them in a dedicated project (with assertj as dependency)

[krmahadevan]

@juherr

Deprecate + Use assertj under the hood, included into the jar under a custom package (no dependency issue for users)

Are you talking about something like this ? https://medium.com/@akhaku/java-class-shadowing-and-shading-9439b0eacb13

Remove the deprecated classes and provide them in a dedicated project (with assertj as dependency)

Why would we be doing this, when the whole point of us working towards removing Assert is because we don't want to be sidelined by the assertion implementations etc., and instead have all users use an assertion dedicated library. If we host them as a dedicated project wouldn't we still have to maintain it ?

[juherr]

Are you talking about something like this ? medium.com/@akhaku/java-class-shadowing-and-shading-9439b0eacb13

Yes, exactly something like that.

Why would we be doing this, when the whole point of us working towards removing Assert is because we don't want to be sidelined by the assertion implementations etc., and instead have all users use an assertion dedicated library.

Because it will allow us to remove it easily from the main project and I think it will too breaking change for the community without an easy workaround.

If we host them as a dedicated project wouldn't we still have to maintain it ?

Yes, but expect an AssertJ upgrade, I don't see any additional support, do you?

[krmahadevan]

Yes, but expect an AssertJ upgrade, I don't see any additional support, do you?

Well, I hope so too. It would depend a lot on how we are plumbing the assertion implementations in TestNG to use assertj behind the scenes. If its a direct 1:1 api mapping, I agree with you that we wont be having to do any additional support. Lets see how it pans out. Will try to wrap up that over this week and raise a PR.

[vlsi]

One option could be to shade AssertJ

That includes "shading CVEs" 😱

Possibly moving it out to an optional module could be an alternative, Then not having it shaded into the TestNG jar, >Existing old users add it as a separate dependency for legacy compatibility

+1

There might be a case for https://github.com/openrewrite/rewrite-testing-frameworks migration (e.g. from TestNG Assert to AssertJ / Truth)