Remove specs from finagle-http tests. #292
Conversation
Problem In order to upgrade to 2.11 (and as part of a larger twitter initiative) specs has to be removed. This is one small step in doing so. Solution Remove specs, replace with the standard FunSuite / assert way of testing. Result. Hopefully nothing will change. Except for removing specs.
|
Leaving the mockito bits in is fine. Could you refactor to use org.scalatest.mock.MockitoSugar? I'll update the coordination issue to also mention that. |
|
Alright, MockitoSugar has been added. |
|
LGTM! This is awesome. |
|
LGTM! |
|
@mosesn Being equally excited about the complete move to ScalaTest for Finagle, I keep wondering if there is any argument against using Is there any interop or legacy concern to consider? The outcome would be a massive drop in total test time(> 60%). |
|
@alexflav23 have you tried it on any specific tests? My impression is that scalatest already runs suites in parallel by default, so I don't understand where the speedup would be coming from. However, I'm definitely interested in work to make tests go faster, so if you find experimentally that it works well, we would be happy to see it. Maybe we should take this discussion off of this PR and move it either to an issue or to the mailing list? |
|
@mosesn Makes perfect sense, I will continue this on the mailing list. The speed up comes from running every spec(not every file) in parallel and not blocking for results. Examples to come on the mailing list. |
|
For the record, I'm pulling this internally. |
|
This is now on GitHub—thanks, Adam! |
As a follow up to the smaller PR I issued before.
This PR removes specs from being used in the finagle-http tests (and re-name all tests/files to *Test). Some of the tests however did use mockito, so I've left that in. Let me know if that's something we should remove also.