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

feat: add a requestedFor method allowing to pass Http method as parameter #2175

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

ytvnr
Copy link
Contributor

@ytvnr ytvnr commented May 11, 2023

It is useful for parameterized test, when you also use request(String method, UrlPattern urlPattern)

So you are able to write your test like this:

@ParameterizedTest
@ValueSource(strings = {"GET", "POST", "PUT", "HEAD", "TRACE", "PATCH", "OPTIONS", "DELETE", "ANY", "RANDOM"})
public void verifyRequestedForSameMethodAsRequest(String method) {
    wiremock.stubFor(request(method, urlEqualTo("/methods")).willReturn(ok("response"));
    // ... Test stuff here
    verify(requestedFor(method, urlEqualTo("/methods")));
}

References

Submitter checklist

  • The PR request is well described and justified, including the body and the references
  • The PR title represents the desired changelog entry
  • The repository's code style is followed (see the contributing guide)
  • Test coverage that demonstrates that the change works as expected
  • For new features, there's necessary documentation in this pull request or in a subsequent PR to wiremock.org

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Definitely +1. I would rather deprecate the older methods TBH, because they may cause quite a lot of confusion. Also agree about the documentation, though it would be great to document this particular method there

@oleg-nenashev oleg-nenashev added the needs-tom Tom's Train Project :) label May 14, 2023
@ytvnr
Copy link
Contributor Author

ytvnr commented May 15, 2023

@oleg-nenashev, thank you for the review.
Do you want me to deprecate the older methods in another commit ?
Maybe it would be the opportunity to add methods overload taking the RequestMethod as a parameter and not just a string ?
We would have the additional:

  • RequestPatternBuilder requestedFor(HttpMethod method, UrlPattern urlPattern)
  • MappingBuilder request(HttpMethod method, UrlPattern urlPattern)
    wdyt ?

@oleg-nenashev
Copy link
Member

No strong opinion, I guess @tomakehurst should make a call on deprecation

@tomakehurst
Copy link
Member

Please could you run ./gradlew spotlessApply and push the changes?

@tomakehurst
Copy link
Member

Re deprecation, I assume you're talking about getRequestedFor(...), postRequestedFor(...) etc.?

If so, I don't think we ought to deprecate. They're convenience DSL methods for the most commonly used HTTP methods, and can co-exist with the general case.

@ytvnr ytvnr force-pushed the add-requestFor-with-method branch from e0fe729 to 00403d1 Compare May 31, 2023 14:56
@ytvnr
Copy link
Contributor Author

ytvnr commented May 31, 2023

Hi @tomakehurst, I just forced push.
Sorry I did not know about this command.

@tomakehurst
Copy link
Member

Merging the latest from master should fix the test error

…eter.

It is useful for parameterized test, when you also use `request(String method, UrlPattern urlPattern)`
@ytvnr ytvnr force-pushed the add-requestFor-with-method branch from 00403d1 to d12821e Compare June 12, 2023 05:00
@ytvnr
Copy link
Contributor Author

ytvnr commented Jun 12, 2023

It should be ok @tomakehurst (at least ./gradlew check worked locally 😄 )

@oleg-nenashev oleg-nenashev merged commit a4958ec into wiremock:master Jul 10, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs-tom Tom's Train Project :)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants