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

Consolidate usage of http mock servers in tests #1392

Closed
fatroom opened this issue Jan 31, 2023 · 4 comments · Fixed by #1441
Closed

Consolidate usage of http mock servers in tests #1392

fatroom opened this issue Jan 31, 2023 · 4 comments · Fixed by #1441
Assignees
Labels
Milestone

Comments

@fatroom
Copy link
Member

fatroom commented Jan 31, 2023

Context

Currently the different tests in the code base use MockRestServiceServer from spring framework as well as RestDriver (same named library).
RestDriver is based on jetty 9 which is incompatible with the spring framework 6.
For moving forward we need to avoid direct relation on java.servlet based packages.

Possible Implementation

Keeping #837 in mind it's better to avoid usage of MockRestServiceServer in the tests, so we can use use MockWebServer from OkHttp.
The library is independent from javax.server namespace problems, as well as providing good enough level control.

Another option would be to wait for wiremock 3 release (beta version of wire mock 3 already available, though not sure if that would be possible without simultaneous migration of the spring framework to version 6 due to JEE api changes)

@Semernitskaya
Copy link
Contributor

Semernitskaya commented Mar 27, 2023

Mock servers comparison approach

I've compared implementations suggested in the task description.

I've collected main use cases of RestDriver in the project tests (e.g. verify(), respond with delay, use custom org.apache.http.client.HttpClient), wrote a test with these use cases using RestDriver - see HttpMockTestClientDriver, then I tried to rewrite this test using different mock servers implementations.

Notes:

  1. Test is located in riptide-failsafe module as I'm more familiar with it from my previous task and many use cases are based on tests from riptide-failsafe (e.g. processing delayed response)
  2. After rewriting, every test case can be executed successfully, and the same logic is verified
  3. Some of mentioned Pros and Cons are subjective (e.g. good documentation), they can be discussed in more details if needed

Mock servers comparison results

Spring MockRestServiceServer

Required changes

Pros:

  • Has good documentation
  • Has verify() method
  • No much code changes are required
  • Spring dependency can be used with test scope => all transitive dependencies will be present only in tests (see Maven docs)

Cons:

  • MockSetup class is needed for set up (currently it is duplicated in several project modules), less flexibility in the tests as some parts are initialised in MockSetup
  • No possibility to use custom org.springframework.http.client.ClientHttpRequestFactory, see use case
  • Still depending on Spring framework

OkHttp MockWebServer

Required changes

Pros:

  • Has documentation (semi pros, as many methods are still not documented)
  • Simple set up, similar to RestDriver
  • Custom org.springframework.http.client.ClientHttpRequestFactory can be used

Cons:

  • Dispatching request for several paths is more complicated, see test case
  • Doesn't have verify() method

WireMock WireMock class

Required changes

Pros:

  • Server created and started for you (provided as a test method parameter)
  • Multiple find..() methods for requests - can simplify replacement of verify() method

Cons:

  • No documentation at all for the beta version, makes library usage complicated (not clear if it will be fixed in the release version)
  • Doesn't have verify() method
  • Sending different response for the same endpoint in multiple invocations requires workaround, see implemented RequestMatcher

Notes:

  • Version 3 is still in beta, not clear when it will be released and if breakable changes can be introduced in the release version
  • Problem with Jetty compatibility was solved by adding Jetty 11 dependencies with test scope - cumbersome solution but it allows to migrate before Spring migration

@fatroom
Copy link
Member Author

fatroom commented Apr 3, 2023

@Semernitskaya thank you for this research. It seems the least cons on the option OkHttp.MockWebServer.
On this I would recommend to set on this approach.

Would you like to continue work on switching code base to selected option?

@Semernitskaya
Copy link
Contributor

@fatroom I agree, yes sure - I'll continue working on this task. My plan:

  • Migrate modules one by one
  • Send draft PR when first couple of modules are migrated to make sure that I'm on the right track
  • Delete MockSetup classes (they are used for Spring MockRestServiceServer initialisation)
  • Keep Spring MockRestServiceServer in spring related modules (riptide-spring-boot-starter, riptide-spring-boot-autoconfigure)

@fatroom
Copy link
Member Author

fatroom commented Apr 4, 2023

@Semernitskaya sounds perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants