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

Confusing mockMvc asserts on async result [SPR-17543] #22075

Closed
spring-projects-issues opened this issue Nov 27, 2018 · 2 comments
Closed

Confusing mockMvc asserts on async result [SPR-17543] #22075

spring-projects-issues opened this issue Nov 27, 2018 · 2 comments
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply

Comments

@spring-projects-issues
Copy link
Collaborator

Lukas Krecan opened SPR-17543 and commented

Let's take the following test of an asynchronous API

MvcResult mvcResult = mockMvc.perform(post("/asyncEndpoint")
            .andExpect(status().isOk()) // passes but should not
            .andReturn();

mockMvc.perform(asyncDispatch(mvcResult))
            .andExpect(status().isConflict()); // the real status

We execute the request and check the status at once. Surprisingly it passes, even if the result is not ready yet and the final status will be different. I would except first .andExpect(status().isOk()) to fail since the result has not yet arrived (IllegalStateException?).

It's especially surprising if you forget that you have to test async controllers differently. Then you spend nontrivial amount of time investigating why your controller is returning 200 when you expect it to return an error.


Affects: 5.1.2

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

I see your point but I'm afraid there isn't anything we can do to fail proactively at that point. In MockHttpServletResponse and in the Servlet API, the status is 200 from the very start by default, and technically it can change multiple times up until when the response is committed. So the only way to fail proactively at the point of "passes but should not" is to know that this is the end of the test, and that an async dispatch will not be performed. Aside from that the status is 200 (OK) at that point of the processing. It's not too hard to imagine a test verifying the status before vs after the async dispatch.

As far as spending a lot of time, the print() feature should have helped to point out the issue:

 

MockHttpServletRequest:
 HTTP Method = GET
 Request URI = /1
 Parameters = {deferredResult=[true]}
 Headers = {}
 Body = <no character encoding set>
 Session Attrs = {}
Handler:
 Type = org.springframework.test.web.servlet.samples.standalone.AsyncTests$AsyncController
 Method = public org.springframework.web.context.request.async.DeferredResult<org.springframework.test.web.Person> org.springframework.test.web.servlet.samples.standalone.AsyncTests$AsyncController.getDeferredResult()
Async:
 Async started = true
 Async result = null
Resolved Exception:
 Type = null
...

 

@spring-projects-issues spring-projects-issues added status: waiting-for-triage An issue we've not yet triaged or decided on in: test Issues in the test module type: enhancement A general enhancement and removed type: enhancement A general enhancement labels Jan 11, 2019
@snicoll
Copy link
Member

snicoll commented Sep 18, 2023

Closing based on the comment above.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Sep 18, 2023
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

2 participants