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

Add convenient method for matching absence of form param in a request #2193

Conversation

G-Basak
Copy link
Contributor

@G-Basak G-Basak commented May 28, 2023

This pull request adds a convenient method for matching absence of form param in a request.
Usage example :

//after this pull request
verify(
    getRequestedFor(urlPathEqualTo("without/formParam"))
        .withoutFormParam("test-form-param"));

It was possible to verify the same even without this method as shown below, but felt counter-intuitive (given there is already a withoutHeader method).

//before this pull request
verify(
    getRequestedFor(urlPathEqualTo("without/formParam"))
        .withFormParam("test-form-param", absent()));

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

@G-Basak G-Basak marked this pull request as ready for review May 28, 2023 12:40
@G-Basak G-Basak marked this pull request as draft May 28, 2023 12:43
@G-Basak G-Basak marked this pull request as ready for review May 28, 2023 12:52
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.

LGTM

@oleg-nenashev oleg-nenashev requested a review from a team as a code owner July 12, 2023 10:03
@oleg-nenashev
Copy link
Member

Fixed merged conflicts, I hope

Comment on lines +27 to +34
import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.common.FatalStartupException;
import com.github.tomakehurst.wiremock.common.FileSource;
import com.github.tomakehurst.wiremock.http.ResponseDefinition;
import com.github.tomakehurst.wiremock.matching.RequestPattern;
import com.github.tomakehurst.wiremock.stubbing.StubMapping;
import java.io.PrintStream;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only formatting changes because of spotless

Comment on lines +106 to +128
System.setErr(
new PrintStream(err) {
@Override
public void println(String s) {
if (!s.startsWith("SLF4J")) {
super.println(s);
}
}

@Override
public void println(char[] chars) {
if (!new String(chars).startsWith("SLF4J")) {
super.println(chars);
}
}

@Override
public void println(Object o) {
if (!o.toString().startsWith("SLF4J")) {
super.println(o);
}
}
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only formatting changes because of spotless

@G-Basak
Copy link
Contributor Author

G-Basak commented Jul 12, 2023

Fixed merged conflicts, I hope

Finally figured that we can't use multipart/form-data. So switched test to application/form-url-encoded.

Comment on lines 42 to 53
@JsonProperty
@Override
public String key() {
return super.key();
}

@JsonProperty
@Override
public List<String> values() {
return super.values();
}

Copy link
Member

Choose a reason for hiding this comment

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

Should exposing those APIs to JSON be considered another new feature in the changelog?

Copy link
Contributor Author

@G-Basak G-Basak Jul 13, 2023

Choose a reason for hiding this comment

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

Sorry, I am not sure what you meant by release logs. Is it the PR title?

I have added a point in the PR description why this change is necessary.
The absence of this is actually a bug. The near-miss response deserialization is not possible without the annotation. I can try to reproduce the same and put out another PR just fixing this aspect before we merge this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oleg-nenashev
I checked it a bit more. Looks like those JSON changes are no longer needed for the tests to pass.
Something must have changed after I had raised the PR. In any case I removed these changes.

@oleg-nenashev oleg-nenashev merged commit c041ae3 into wiremock:master Jul 17, 2023
7 checks passed
@G-Basak G-Basak deleted the add-convenient-method-for-matching-absence-of-form-param-in-a-request branch July 17, 2023 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants