spring-cloud / spring-cloud-contract

Support for Consumer Driven Contracts in Spring
https://cloud.spring.io/spring-cloud-contract
Apache License 2.0
719 stars 438 forks source link

Spring Cloud Contract WireMock: Peformance, test isolation and complex bodies matches and diff improvements #1878

Open estigma88 opened 1 year ago

estigma88 commented 1 year ago

Is your feature request related to a problem? Please describe.

DISCLAIMER: I haven't worked with Spring Cloud Contract for the last 2 years, what I establish in this ticket might or not might be totally accurate

In our project, we have the following goals related to tests in Spring Boot: Test as much as we can, run as fast as we can, with as few resources as we can

With that in mind, we favor API tests over unit tests, where an API test is the following:

Test -> Controller -> Service -> Call to Other HTTP services

And we mock the edge: Other HTTP services with Wiremock

The following are the challenges we found using Wiremock, taking into account we have now hundreds of tests:

Several instances of ApplicationContext and Wiremock

For each @SpringBootTest, Spring Boot starts a new ApplicationContext, if the configuration differs from one tests class to another (having a cache miss), we end up with a lot of ApplicationContext, which means, the test suites waste time starting and stopping multiple ApplicationsContext. Minimizing the amount of ApplicationContext in a test suite is ideal.

For each @AutoConfigureWiremock, Spring starts a Wiremock sever by ApplicationContext (AFAIR), so, from the previous point, we will have one Wiremock server by ApplicationContext.

Having a Wiremock server by each ApplicationContext seems like a waste of resources.

Test isolation

We want each test in our suite to have inputs, outputs and mocks totally isolated, we don't want to share any of those between tests.

@AutoConfigureWiremock allows defining mocks folder at class level only, meaning we cannot isolate mocks by @Test inside a class.

If we want to isolate the mocks by @Test, we end with a @SpringBootTest and @Test by class, which is a lot of waste.

Why isolation is important? when you have hundreds of tests, you might find tests using similar mocks but with different logic state, for instance, different database state, therefore, multiple similar mocks, for different use cases, can exist. Having all these mocks in the same folder and created at the same time inside Wiremock, might create conflicts, and debugging will be hard. Also, modifying mocks in one test might change the behavior of other tests, if those mocks are shared.

Wiremock requests beyond JSON bodies

We have seen a lot of external services using non-JSON requests bodies, Wiremock is pretty good at matching JSON requests, but for instance, AWS EC2 SDK and Slack SDK uses application/x-www-form-urlencoded, which Wiremock cannot show a good diff as it does with JSON. This limit the debugability of those mocks.

As an example, a Slack request:

{
  "request": {
    "method": "POST",
    "url": "/slack/api/chat.postMessage",
    "headers": {},
    "bodyPatterns": [
      {
        "equalTo": "channel=XXXXX&thread_ts=46654654.000016&text=Hey%20buddy%2C%20here%20is%20how%20you%20can%20ask%20me%20something%3A&link_names=0...."
      }
    ]
  },
  "response": {
    "status": 200,
    "jsonBody": {
      "ok": true,
      "teamId": "genius"
    }
  }
}

There, we found two challenges:

Describe the solution you'd like

Describe alternatives you've considered We built our own JUnit 5 extension doing all of those (except the log match and diff for complex bodies)

Additional context We are using the Spring Cloud Contract project only to bring the Wiremock dependency in and be sure it complies with Spring Boot Bom

marcingrzejszczak commented 1 year ago

We built our own JUnit 5 extension doing all of those (except the log match and diff for complex bodies)

If you fixed the whole thing yourself why not contribute this back to this project?

estigma88 commented 1 year ago

I don't think there is a 1 to 1 match, we built it using pure JUnit 5 and Wiremock, no Spring involved. I think I will need to rebuild those features from scratch inside Spring Cloud Wiremock to comply on the current architecture.

I am open to evaluate how we can do that starting for exploring the current code and finding incremental and iterative ways to do it, let's see if my available time allows me.

marcingrzejszczak commented 1 year ago

That would be great cause we started a discussion with @sbrannen

Since Sam pointed me to @simonbasle to talk about pushing HTTP stubbing support (e.g. via WireMock) down to Spring, I'm mentioning him in this issue.

estigma88 commented 1 year ago

In a project time ago, we have a centralized Wiremock installed in our environment, and we created a step in our ci/cd to push the mocks there, before running the tests. The strategy was valid and useful, with its drawbacks as everything.

Supporting that strategy shouldn't be hard I think, just a matter of Wiremock client configuration.

estigma88 commented 1 year ago

I was exploring the code and the following are some questions we should agree on before moving forward:

NOTE: with tests suite, I mean the whole tests Maven/Gradle runs, for instance: gradle :test NOTE2: I will focus on JUnit 5

Support for only one Wiremock server for the whole test suite run

  1. How are we going to have one Wiremock instance for the whole tests suite?

I found that the SpringExtension creates one TestContextManager by test class, and that is the one Spring delegates the test execution. I don't see a clear way to save global state (one Wiremock instance by tests suite) inside the TestContext.

In our built in extension, we save the Wiremock instance in the JUnit 5 ExtensionContext.Store object, in the root, but, seems like the TestContextManager doesn't have access to it.

Support for declaring Wiremock mocks at @Test level

  1. How are we going to tell Spring Cloud Contract Wiremock, we want to use only one instance for the whole tests suite?

We have the AutoConfigureWireMock annotation, which only works at the class level, we might think about allowing it at @Testmethod level, however, how do we tell we need only one instance? Let's add a new property to it, something like: globalInstance. This solution might look like this:

@AutoConfigureWireMock(..., globalInstance = true)
class MyTest{

 @Test
 @AutoConfigureWireMock(..., globalInstance = true)
 public void test1(){

 }

 @Test
 @AutoConfigureWireMock(..., globalInstance = true)
 public void test2(){

 }

 ...
}

The drawback of that is backward compatibility, DRY and mixing strategies, for instance, multiple annotations, some with globalInstance = true and other no:

@AutoConfigureWireMock(...)
class MyTest{

 @Test
 @AutoConfigureWireMock(...)
 public void test1(){

 }

 @Test
 @AutoConfigureWireMock(..., globalInstance = true)
 public void test2(){

 }

 ...
}

How should we handle that case? we might just use the global Wiremock for any annotation with the property globalInstance = true and the ones without the property, we just use the normal strategy?

Also, if we have multiple annotations, with different ports, and globalInstance = true, which port should we choose to start Wiremock?

Another option would be creating a new annotation, something like @AutoConfigureGlobalWireMock, the port defined in the properties file.

  1. How does the annotation behave at @Test method level?:

The extension should push (local) the mocks to the global Wiremock instance, before the test runs, and should reset the mocks after it runs. There is an interesting edge case here: concurrent tests.

If we allow concurrent tests, and we are reusing the same global Wiremock, we cannot reset the whole server after each test, because other tests might be running and using their own mocks. To solve that, we can tag the mocks inside Wiremock, adding some metadata, for instance: testName=Class.method. With this testName, we can reset/clear the mocks after the test runs, for that specific test, without affecting mocks for other tests. The metadata can be added at mock creation time, and there are methods to reset/clear mocks querying metadata also, in Wiremock.