Add scalatest plus mockito as a dependency. #296
Conversation
toshetah
commented
Nov 26, 2020
toshetah
commented
- Add scalatest plus mockito as a dependency.
- Fix docs accordingly.
- Fix brokenn link to travis at the first page.
|
Hi @toshetah, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement: |
| @@ -128,7 +129,6 @@ lazy val docs = project | |||
| commonSettings, | |||
| libraryDependencies ++= Seq( | |||
| "org.mockito" % "mockito-core" % MockitoVersion % Test, | |||
| "org.scalatestplus" %% "mockito-3-2" % ScalatestMockitoVersion % Test, | |||
There was a problem hiding this comment.
I think we should keep this here: mockito is an optional dependency, and it's up to the user to add it if they want.
There was a problem hiding this comment.
Why? It is same as scalatest. I created a new project, and copied to it UserServiceSpec.scala, and it fails on the import of MockitoSugar. Why having one of them imported and not the other? It will cause the user to go and look for the compatible version, while you already did it.
There was a problem hiding this comment.
We need to choose whether we are providing a complete solution or we don't. If we decided to upgrade ScalaTest to beyond 3.0.8, we have to bring the dependencies to make it actually work, and not let the user to try all versions until he finds a good one. @raboof
There was a problem hiding this comment.
Another question we need to ask ourself is, will we have any user, that will use this library, and won't use mockito-3-2 ? The answer is probably no, because otherwise he won't have MockitoSugar.
There was a problem hiding this comment.
If you look at for example https://www.playframework.com/documentation/2.8.x/ScalaTestingWithScalaTest#Unit-Testing-Controllers, that is a useful scalatestplus-play spec that doesn't use mockito, right? or am I reading that wrong?
There was a problem hiding this comment.
You are right about the example you provided. But take a look at this one. It won't work as it is.
There was a problem hiding this comment.
The reason it works at the docs is that mockito-3-2 is added at the docs project. A user that will copy paste it into a new project will fail importing.
There was a problem hiding this comment.
Exactly, so I propose we add the documentation on how to add the scalatestplus-mockito dependency to your build in that section. As far as I can see it is only used in that section, right? So it is an optional tool you can use with scalatestplus-play, but not one that is required for all use cases.
|
Don't you agree with my claims? I just want the best for this great project! |
I generally prefer to keep optional dependencies optional and not pull them in when not all users would use them - but I don't feel too strongly about it ;)
Your improvements are definitely much appreciated and make this project even better! Thanks! |
|
@raboof, I forgot to tell you that the reason I got to this, was answering a StackOverflow post asking where has |
