What is the purpose of the change

  • This PR is to fix 3 flaky tests:
Test1: org.springframework.boot.test.autoconfigure.web.client.RestClientTestNoComponentIntegrationTests.manuallyCreateBean
Test2: org.springframework.boot.test.autoconfigure.web.client.RestClientTestTwoComponentsIntegrationTests.client1RestCallViaCustomizer
Test3: org.springframework.boot.test.autoconfigure.web.client.RestClientTestTwoComponentsIntegrationTests.client2RestCallViaCustomizer

Why the tests can be flaky

Test1: - There is only one server that is already bound to the first RestTemplate. If multiple clients run, the server cannot be autoconfigured without MockServerRestTemplateCustomizer. We should reset the internal state by removing all expectations and recorded requests at the end of each test, to avoid pollution to other tests.

Test2 and Test3: - The internal states were not reset after the tests run which can lead to polluting other tests.

It may be better to clean state pollutions so that some other tests won't fail in the future due to the shared state pollution.

Brief changelog

Test1: - Use MockRestServiceServer by auto-configuration and reset internal state of the server at the end of the test.

Test2 and Test3: - Reset internal state of the server at the end of the test

Verifying this change

  • Rerun the tests for more than one time in the same JVM, we get the following failures:

Test1:

java.lang.IllegalStateException: Unable to use auto-configured MockRestServiceServer since MockServerRestTemplateCustomizer has been bound to more than one RestTemplate

Test2 and Test3:

java.lang.IllegalStateException: Unable to return a single MockRestServiceServer since MockServerRestTemplateCustomizer has been bound to more than one RestTemplate
  • With the proposed fix, the tests do not pollute the shared state (and pass when multiple tests that use this state are run in the same JVM).

Comment From: pivotal-cla

@LALAYANG Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

Comment From: pivotal-cla

@LALAYANG Thank you for signing the Contributor License Agreement!

Comment From: wilkinsona

Thanks for the proposal, but manually resetting the server should not be necessary. The reset calls should be made automatically via a test execution listener.

If this automatic reset is not happening for you then you may have found a bug. If that’s the case, please open a new issue with a small sample that reproduces the problem and we can investigate. Explicitly calling reset isn’t the right thing to do as it potentially masks the problem. Thanks anyway for the PR.