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

MockWebServer: Clear requests before each test #6212

Open
GabrielBB opened this issue Aug 5, 2020 · 17 comments
Open

MockWebServer: Clear requests before each test #6212

GabrielBB opened this issue Aug 5, 2020 · 17 comments
Labels
enhancement Feature not a bug

Comments

@GabrielBB
Copy link

GabrielBB commented Aug 5, 2020

Can we have a method to clear requests? Something like this:

    @BeforeEach
    fun cleanRequests() {
        mockServer.clearRequests();
    }

My tests are taking requests from previous tests. I wouldn't want to shutdown entirely

@GabrielBB GabrielBB added the enhancement Feature not a bug label Aug 5, 2020
@GabrielBB GabrielBB changed the title FR -> MockWebServer: Clear requests before each test MockWebServer: Clear requests before each test Aug 5, 2020
@JakeWharton
Copy link
Member

Are you using the test rule? There should be a new instance for every test.

@GabrielBB
Copy link
Author

Yes, I'm using @TestInstance(TestInstance.Lifecycle.PER_CLASS)

@JakeWharton
Copy link
Member

Can you switch to PER_METHOD? Assuming such a thing exists.

@swankjesse
Copy link
Member

Yeah, MockWebServer instances are not expected to be reused.

@GabrielBB
Copy link
Author

Will a clear method be enough to be reusable ? I can open PR, why not I guess ?

@swankjesse
Copy link
Member

No, I don't think it's a good fit for a PR. The clear method encourages a pattern we don't support well.

@yschimke yschimke closed this as completed Aug 6, 2020
@nightswimmings
Copy link

nightswimmings commented Nov 16, 2021

@JakeWharton @swankjesse What if we need an static MockWebServer in order to register @DynamicPropertySource properties with the mockWebServer.getPort()? In that case we need a single instance and resetting it, right?

@yschimke
Copy link
Collaborator

Can't you change your logic to update the DynamicPropertyRegistry as each test runs with the new port/url?

@nightswimmings
Copy link

DynamicPropertyRegistry method must be static as per documentation, so I guess it does not reevaluate within the class. I think that would imply restarting the whole spring context since the mechanism is similar to that of @SpringBootTest(properties=X). So we'd need the mockrestserver instance to be static as well, I am not sure it makes sense to reinstantiate it globally and start/shutwoning on each test, I find the clearRequests() approach
more comfy for that scenario

@yschimke
Copy link
Collaborator

The method is static, but it provides a Supplier that gets called on demand. I'm not saying it's ideal for you, just that it doesn't appear that you are blocked as you could update the port each time a new MockWebServer is started for your test.

@nightswimmings
Copy link

nightswimmings commented Nov 16, 2021

I don't understand the Supplier part but I am interested.. :)

I have this currently, how would you exactly do it?

     static MockWebServer mockWebServer;

    @DynamicPropertySource
    static void properties(DynamicPropertyRegistry propsRegistry) {
         propsRegistry.add("spring.cloud.gateway.routes[0].uri", () -> "http://localhost:" + mockWebServer.getPort())
    }
    
    @BeforeAll
    static void beforeAll() throws IOException {
        mockWebServer = new MockWebServer();
        mockWebServer.start();
    }

    @AfterAll
    static void afterAll() throws IOException {
        mockWebServer.shutdown();
    }

@yschimke
Copy link
Collaborator

Maybe something like the following, where port should be read everytime the property is accessed.

     MockWebServer mockWebServer;
     static int port;

    @DynamicPropertySource
    static void properties(DynamicPropertyRegistry propsRegistry) {
         propsRegistry.add("spring.cloud.gateway.routes[0].uri", () -> "http://localhost:" + port)
    }
    
    @Before
    void before() throws IOException {
        mockWebServer = new MockWebServer();
        mockWebServer.start();
    }

    @After
    void afterAll() throws IOException {
        mockWebServer.shutdown();
    }

@nightswimmings
Copy link

Ahh I think I understand you. @DynamicPropertySource is only evaluated once, but by your words I understand that the property supplier itself does it every time the property is accessed. If so I guess your example is missing a port = mockWebServer.getPort() at the end of the before(), right?

@nightswimmings
Copy link

nightswimmings commented Nov 17, 2021

@yschimke I did some tests and the evaluation for the supplier is done once at context startup and that occurs before the BeforeEach, so even if we managed to tell spring to reevaluate whole context before each test, I should make sure that the BeforeEach is still executed before the property evaluation, otherwise I need to do an ugly hack for reinitializing the mockwebserver on the supplier only as long as it takes part during the same test. Frankly I still see clearRequest() a much nicer approach (and more performant)

@yschimke
Copy link
Collaborator

Is it possible the item reading the property is in the wrong scope? Singleton?

https://www.baeldung.com/spring-dynamicpropertysource

If our PostgreSQL container is going to listen to a random port every time, then we should somehow set and change the spring.datasource.url configuration property dynamically. Basically, each test should have its own version of that configuration property.

@tberger-plugsurfing
Copy link

May I ask if there is any solution here? Is okttp Mockserver just unusable with Spring tests and their lifecycle when a @DynamicPropertiesSource is used in combination with a static instance? I have the same problem as stated above and I actually don't understand why the queue can't just be cleared via API.

@swankjesse swankjesse reopened this Jan 19, 2024
@swankjesse
Copy link
Member

Sharing state between tests is problematic, especially since MockWebServer isn’t dynamic on features like whether TLS is enabled.

The ‘more performant’ argument is a rationalization, not a reason; MockWebServer performance is completely reasonable without reusing instances.

I’m reopening this to find a way to use MockWebServer despite Spring Boot’s limitations. It might be something as basic as providing a factory of MockWebServer instances.

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

No branches or pull requests

6 participants