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

chore: Add status code matcher example #354

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

tienvx
Copy link
Contributor

@tienvx tienvx commented Oct 28, 2023

This matcher can't be used to match the response code unfortunately, because currently pactffi_response_status only allow integer status.

But it can be useful to match a value in body (like this example), query parameter or header.

@mefellows
Copy link
Member

pactffi_response_status is only designed for use on the status code of the response, it can't be used elsewhere.

There is a status code matcher in the core, but it's not yet available via the "integration JSON" (i.e. via FFI)

@mefellows
Copy link
Member

@tienvx
Copy link
Contributor Author

tienvx commented Oct 30, 2023

@mefellows Yes, it's the statusCode matcher mentioned in that document.

@mefellows
Copy link
Member

ha interesting. Are you sure that matcher works on items that aren't the response code? Does it just look at numbers in general?

@tienvx
Copy link
Contributor Author

tienvx commented Oct 30, 2023

I tested again, and it doesn't work as expected (any number e.g. 204 still valid for status code serverError).

So this matcher is useless now. Let me update this PR, remove this matcher completely

@mefellows
Copy link
Member

Ah, I think that might have been why that matcher doesn't work - you can't pass it in to pactffi_response_status. I guess we'd need a variant of that method that accepts the matcher?

@tienvx
Copy link
Contributor Author

tienvx commented Oct 30, 2023

I checked again, there are several scenarios about that matcher in compatibility suite's v4/matching_rule

So instead of remove that matcher, I think creating a pactffi_response_status_v2 like your suggestion is better. I will make this PR as Draft for now.

@tienvx tienvx marked this pull request as draft October 30, 2023 10:24
@tienvx tienvx force-pushed the add-status-code-matcher-example branch 2 times, most recently from 0e39d41 to 5c93471 Compare November 22, 2023 18:18
@tienvx
Copy link
Contributor Author

tienvx commented Nov 23, 2023

pactffi_response_status_v2 is implemented in pact-foundation/pact-reference#346.

This PR is green when that PR is merged and new version is released

@tienvx tienvx force-pushed the add-status-code-matcher-example branch 4 times, most recently from 4cc3355 to be6c131 Compare November 28, 2023 03:32
@tienvx tienvx force-pushed the add-status-code-matcher-example branch from be6c131 to e5db014 Compare November 28, 2023 03:47
@tienvx tienvx marked this pull request as ready for review November 28, 2023 04:04
@tienvx
Copy link
Contributor Author

tienvx commented Nov 29, 2023

I will bypass the review process for this PR

@tienvx tienvx merged commit 0d4e29d into pact-foundation:ffi Nov 29, 2023
20 of 26 checks passed
@tienvx tienvx deleted the add-status-code-matcher-example branch November 29, 2023 02:55
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