Skip to content

MockMvc HtmlUnit support shares CookieManager #1009

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

Closed
wants to merge 1 commit into from

Conversation

rwinch
Copy link
Member

@rwinch rwinch commented Mar 17, 2016

Previously MockMvc builders failed to share the WebConnection used for
managing cookies in the MockMvcWebConnection. This meant that the various
CookieManagers would have different states.

This commit ensures that the WebConnection is set on the
MockMvcWebConnection.

Fixes SPR-14066

@rwinch rwinch force-pushed the SPR-14066 branch 2 times, most recently from 20e96a6 to 2028535 Compare March 17, 2016 21:28
@rstoyanchev
Copy link
Contributor

@rwinch thanks for the quick pull request. I'm wondering should we also update the constructors of MockMvcWebConnection to the effect that a WebClient is always expected? Presumably a MockMvcWebConnection is created for use with a WebClient and it makes sense to expect the reference to that client vs creating a new instance?

Previously MockMvc builders failed to share the WebConnection used for
managing cookies in the MockMvcWebConnection. This meant that the various
CookieManagers would have different states.

This commit ensures that the WebConnection is set on the
MockMvcWebConnection.

Fixes SPR-14066
@rwinch
Copy link
Member Author

rwinch commented Mar 18, 2016

@rstoyanchev Thanks for the feedback. You are right it would be valuable to require WebClient in the constructor. I have updated the PR with the changes.

rstoyanchev added a commit that referenced this pull request Mar 18, 2016
@rstoyanchev
Copy link
Contributor

This is now merged.

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

Successfully merging this pull request may close these issues.

2 participants