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

Add OpenTest4J support #1656

Open
kriegfrj opened this issue Mar 5, 2019 · 10 comments
Open

Add OpenTest4J support #1656

kriegfrj opened this issue Mar 5, 2019 · 10 comments

Comments

@kriegfrj
Copy link
Contributor

kriegfrj commented Mar 5, 2019

This has spawned out of #1649.

There is a movement in the testing community to work towards a common exceptions framework for test exceptions. This movement was initiated (and adopted) by the JUnit 5 development effort, but other testing libraries (like AssertJ) have also adopted it. It would perhaps be nice if Mockito also worked towards adopting this common test exceptions framework.

It looks like part of this will be accomplished by the implementation of #1649. However, at least one other area where Mockito could work towards compliance would be in VerificationCollector, where OpenTest4J's MultipleFailuresError could be used if available instead of a raw MockitoAssertionError.

There may be other opportunities inside Mockito to work towards adopting OpenTest4J.

@TimvdLippe
Copy link
Contributor

@sbrannen Are you interested in Mockito joining the OpenTest4J organisation? What would the concrete benefits for us and you be?

@bric3
Copy link
Contributor

bric3 commented Mar 6, 2019

For reference the project is here : https://github.com/ota4j-team/opentest4j

@sbrannen
Copy link

sbrannen commented Mar 7, 2019

Are you interested in Mockito joining the OpenTest4J organisation?

Sure. The more the merrier!

What would the concrete benefits for us and you be?

The concrete benefit is that testing frameworks can standardize on common building blocks, and tools (e.g., build tools and IDES) can also benefit from common exception types, etc.

@mockitoguy
Copy link
Member

My understanding is that OpenTest4J is a new jar that test frameworks need to depend on? In Mockito, we only allow new dependencies when we have a very, very strong reason. Common exceptions is a nice idea, though I don't see a practical benefit for Mockito users.

I do want to have "assert equals"-like pop-up in IDEA for some of the Mockito exceptions. For that we can use reflection just like we currently do it for JUnit4.

Thoughts?

@TimvdLippe
Copy link
Contributor

I am open for adding the jar if that is the "standard way of reporting exceptions by commonly used libraries". Mockito is a commonly used library and since the other testing/assertion frameworks use it, I think it would be a good fit.

That said, we should try it out and see if there is a noticeable difference in terms of IDE integration. If yes, I think that is a tangible benefit for our users. If there is no difference, then there is indeed no reason to add the jar.

@kriegfrj
Copy link
Contributor Author

kriegfrj commented Mar 8, 2019

I believe that it would be possible to achieve the best of both worlds. Well, at least 90% of the best of it.

I think the primary benefit for Mockito users would be better IDE integration by using the standardised testing exceptions. 90% of this can be achieved through reflection/lazy class loading tricks such as that used in #1649. A similar trick can be used for the org.opentest4j.MultipleFailuresError and VerificationCollector. This avoids a hard dependency on OpenTest4J - if it's available at runtime it will be used; otherwise it will fall back to the status quo. This is how AssertJ has handled the situation.

The only question then would be what to do with exceptions that derive from MockitoAssertionError. To fully implement OpenTest compliance, these would need to inherit from org.opentest4j.AssertionFailedError to take full advantage of the OpenTest framework, but this can't be done without creating a hard dependency on OpenTest4J. That said, OpenTest4J is pretty small (and hopefully will remain so), so maybe creating a dependency on it wouldn't be too bad?

I have another idea for getting around this problem without creating a hard dependency, but it will require an enhancement to OpenTest4J, so I might comment over there and link back here.

@kriegfrj
Copy link
Contributor Author

I have another idea for getting around this problem without creating a hard dependency, but it will require an enhancement to OpenTest4J, so I might comment over there and link back here.

For reference, the other idea is in ota4j-team/opentest4j#55.

@mockitoguy
Copy link
Member

I opened a separate ticket to improve dev UX with JUnit5 and IDEs. @kriegfrj, do you want to take a stab at #1663?

@TimvdLippe, I'm with you that we can consider adding new dependency if there is a significant benefit. In this instance, we can get the benefit without adding a new dependency, see #1663.

Thank you!

@TimvdLippe
Copy link
Contributor

Closing this per the above comment, as we implemented this in #1667

@kriegfrj
Copy link
Contributor Author

kriegfrj commented May 6, 2019

Hi @TimvdLippe,

Just FYI, I think that #1667 was only part of the implementation of this. Per the OP, the other part would be to use OpentTest4J's MultipleFailuresError as part of the VerificationCollector implementation.

@TimvdLippe TimvdLippe reopened this May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants