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

Consider adding overloaded methods to MockHttpServletRequestBuilder for headers and params #32918

Closed
philwebb opened this issue May 28, 2024 · 3 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

@philwebb
Copy link
Member

philwebb commented May 28, 2024

See #32913 and this prototype work by @snicoll for background.

In the example above, it's quite common to see code like this:

assertThat(this.mvc.get()
			.uri("/actuator/auditevents")
			.param("principal", "alice")
			.param("after", queryTimestamp)
			.param("type", "logout"))
		.hasStatusOk();

Repeating the .param call is quite noisy, and it would be nice if you could do something similar to Map.of(...):

assertThat(this.mvc.get()
			.uri("/actuator/auditevents")
			.params("principal", "alice", "after", queryTimestamp, "type", "logout"))
		.hasStatusOk();

Same with the header call.

Both headers and params are multi-value maps, so it's perhaps not entirely straightforward. Perhaps an instanceof check is also needed to support something like:

.params("h1", List.of("v1, "v2"), "h2", "v3")
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 28, 2024
@snicoll snicoll added the in: test Issues in the test module label May 28, 2024
@snicoll
Copy link
Member

snicoll commented May 31, 2024

We've discussed this one and we're not sure that the override is worth it, especially considering the use of varargs. Would a Consumer works?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Jun 2, 2024
@philwebb
Copy link
Member Author

philwebb commented Jun 3, 2024

There's already a with method which I think could be used to add parameters. I think we should just close this one.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 3, 2024
@snicoll
Copy link
Member

snicoll commented Jun 4, 2024

Alright, if we want something a bit more concrete, we could add something like B params(Consumer<MultivalueMap<String,String>> in the future.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Jun 4, 2024
@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 status: feedback-provided Feedback has been provided labels Jun 4, 2024
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

3 participants