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

Allow MockRest to match header/queryParam value list with one Matcher #29953

Merged

Conversation

simonbasle
Copy link
Contributor

This commit adds a header variant and a queryParam variant to the
MockRestRequestMatchers API which take a single Matcher over the
list of values.

Contrary to the vararg variants, the whole list is evaluated and the
caller can choose the desired semantics using readily-available iterable
matchers like everyItem, hasItems, hasSize, contains or
containsInAnyOrder...

The fact that the previous variants don't strictly check the size of the
actual list == the number of provided matchers or expected values is
now documented in their respective javadocs.

Closes gh-28660

This commit adds a `header` variant and a `queryParam` variant to the
`MockRestRequestMatchers` API which take a single `Matcher` over the
list of values.

Contrary to the vararg variants, the whole list is evaluated and the
caller can choose the desired semantics using readily-available iterable
matchers like `everyItem`, `hasItems`, `hasSize`, `contains` or
`containsInAnyOrder`...

The fact that the previous variants don't strictly check the size of the
actual list == the number of provided matchers or expected values is
now documented in their respective javadocs.

Closes spring-projectsgh-28660
@simonbasle simonbasle added this to the 6.0.5 milestone Feb 9, 2023
@simonbasle simonbasle added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement labels Feb 9, 2023
@simonbasle simonbasle self-assigned this Feb 9, 2023
Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just minor formatting comment. For Javadoc we wrap around 90, or even 80.

@simonbasle simonbasle added the status: backported An issue that has been backported to maintenance branches label Feb 13, 2023
@simonbasle simonbasle merged commit 189d4e3 into spring-projects:main Feb 13, 2023
@simonbasle simonbasle deleted the 28660-requestMatcherWholeValues branch February 13, 2023 16:51
simonbasle added a commit that referenced this pull request Feb 13, 2023
This commit adds a `header` variant and a `queryParam` variant to the
`MockRestRequestMatchers` API which take a single `Matcher` over the
list of values.

Contrary to the vararg variants, the whole list is evaluated and the
caller can choose the desired semantics using readily-available iterable
matchers like `everyItem`, `hasItems`, `hasSize`, `contains` or
`containsInAnyOrder`...

The fact that the previous variants don't strictly check the size of the
actual list == the number of provided matchers or expected values is
now documented in their respective javadocs.

See gh-29953
Closes gh-29964
simonbasle added a commit to simonbasle/spring-framework that referenced this pull request Mar 30, 2023
This commit changes the name of two recently introduced methods in the
`MockRestRequestMatchers` class for header and queryParam. These have
been found to cause false negatives in user tests, due to the new
overload taking precedence in some cases.

Namely, using a `Matcher` factory method which can apply to both `List`
and `String` will cause the compiler to select the newest list overload,
by instantiating a `Matcher<Object>`.

This can cause false negatives in user tests, failing tests that used
to pass because the Matcher previously applied to the first String in
the header or queryParam value list. For instance, `equalsTo("a")`.

The new overloads are recent enough and this has enough potential to
cause an arbitrary number of user tests to fail that we break the API
to eliminate the ambiguity, by renaming the methods with a `*List`
suffix.

Closes spring-projectsgh-30220
Close spring-projectsgh-30238
See spring-projectsgh-29953
See spring-projectsgh-28660
simonbasle added a commit that referenced this pull request Apr 4, 2023
This commit changes the name of two recently introduced methods in the
`MockRestRequestMatchers` class for header and queryParam. These have
been found to cause false negatives in user tests, due to the new
overload taking precedence in some cases.

Namely, using a `Matcher` factory method which can apply to both `List`
and `String` will cause the compiler to select the newest list overload,
by instantiating a `Matcher<Object>`.

This can cause false negatives in user tests, failing tests that used
to pass because the Matcher previously applied to the first String in
the header or queryParam value list. For instance, `equalsTo("a")`.

The new overloads are recent enough and this has enough potential to
cause an arbitrary number of user tests to fail that we break the API
to eliminate the ambiguity, by renaming the methods with a `*List`
suffix.

Closes gh-30220
Closes gh-30238
See gh-29953
See gh-28660
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 in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support in MockRestServiceServer to assert ALL header and queryParam values with a single Matcher
2 participants