Deprecate MockitoHamcrest #1819
Conversation
This class was used during the migration period from Mockito 1 and Mockito 2, but is no longer necessary. To be able to remove our dependency on Hamcrest, we should remove MockitoHamcrest.
|
Friendly ping on this PR. |
|
I will submit this PR next week if there are no objections. |
|
There were no objections, so I am merging this. |
|
Sorry I'm late here! I suggest we don't deprecate it. The use case (as documented in the Javadoc) is the following: "Hamcrest integration is provided so that users can take advantage of existing Hamcrest matchers". That use case is still valid (perhaps the docs need to be clearer to address #1817). For code that does not use Hamcrest, users should be following standard, non-Hamcrest argThat() API. Hope that helps! |
The problem is that this project requires Hamcrest as compile-time dependency to be built. This is a problem for users who do not want to use Hamcrest, but still build the source code (as they can't use the pre-built jars) If we want to keep supporting Hamcrest users with this API, I would suggest a WDYT? |
|
I'm novice to testing because I have to wear many hats (and I avoided getting into it like the plague in the past). Returning to Spring development in my career, and discovering Mockito this year changed my [integration testing] life. I rely heavily on googling stack overflow examples until it become memorized (if ever I do). It seems like there is a much larger documented wealth of knowledge on hamcrest on stack overflow (maybe cause its been around a long time), and so its easy to find solutions to basic challenges while someone new to Mockito gets used to it. For example hasEntry from this SO helped me today https://stackoverflow.com/questions/2580369/using-mockito-how-do-i-match-against-the-key-value-pair-of-a-map I like TimvdLippe's suggestion because it leverages giving your users the ability to tap into that knowledge/solutions while slimming down the dependencies for the core Mockito code base. Thanks for all your contributions in this area, and letting us stand on your shoulders. |
|
Thanks for that response @mansours ! I will look into introducing a |
Can you elaborate this problem? We use "compileOnly" dependency for Hamcrest, just like we do for JUnit4 or OpenTest4j. This should not cause problems for consumers. |
|
Yes we are facing the same issues with JUnit4 and OpenTest4J. JUnit4 is not an issue atm, as we are also using it, but OpenTest4J is currently an issue. We are not able to include jars, as we require source code to be included and built (for security analyses). Thus, the inclusion of the dependencies during compile time does lead to problems when we want to build Mockito in isolation. I am currently investigating solutions for JUnit4 and OpenTest4J and how that could be decoupled. I went ahead with this PR, as it was my understanding with Mockito 2 is that we wanted to remove our dependency of Hamcrest. Therefore, I assumed that deprecation was appropriate. Especially as there were no objections posted on this PR for almost 2 weeks, I understood that this was okay. I will revert the PR for us to revisit, but I would like to come to a decision on our Hamcrest support and whether we want to support it or not at all. When we come to a decision, I would like to update our JavaDoc and wiki on that to clarify that for our users. |
Of course! Thank you for waiting 2 weeks. Don't block on us.
Can you elaborate the problem? Is it a Google mono-repo use case? (I don't object reworking the artifacts - I want understanding for me and others :)) |
This is for every repository that includes third_party code that we need to have the original source code for security analyses. That includes the mono repository, as well as some other repositories that we have. Since the Hamcrest API is not type-safe (as it requires casting with By having the Hamcrest-compatible API in the Mockito artifact, users can still use Mockito in combination with the Hamcrest API. I would personally say that, while the Hamcrest API was originally useful for defining reusable matchers, the lack of type-safety does more harm than good. I have received good responses when I finished the migration of the |
So the problem is that in addition to Mockito source, you have to pull in Hamcrest source for the analysis? (I still don't feel I fully understand this use case).
That's a fair argument.
To keep backwards compatibility we would need "mockito-core" -> "mockito-hamcrest". Are you thinking of reversing the dependency in future major version? |
This reverts commit d4544a8.
|
Revert of this PR is in ad2f352 |
Correct. Since jars can be built with any arbitrary code, we prefer to not check in a jar, as we cant be certain that it was built from the original source code. That's why we check in the original source code and build that instead.
Yes, but since Maven does not allow a cyclic dependency we have to figure out what the possibilities are. |
This class was used during the migration period from Mockito 1 and Mockito 2, but is no longer necessary. To be able to remove our dependency on Hamcrest, we should remove MockitoHamcrest.
This reverts commit d4544a8.
This class was used during the migration period from Mockito 1 and
Mockito 2, but is no longer necessary. To be able to remove our
dependency on Hamcrest, we should remove MockitoHamcrest.
In response to #1817