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

revert: Values matcher accept keys again #359

Merged
merged 1 commit into from Nov 17, 2023

Conversation

tienvx
Copy link
Contributor

@tienvx tienvx commented Nov 6, 2023

Revert #356

I was wrong #356 is a feature, not a bug.

  • When you pass array without keys to values matcher, it compare values using its index (i.e. starting with 0) in the array.
  • When you pass array with keys to values matcher, it compare values using those keys.
  • Missing keys/indexes are OK.
  • Additional keys/indexes are NOT OK.
    • I'm not sure about this behavior.
    • Pact complain that the value of additional keys/indexes must equals to the first value (e.g. a) ???
    • Probably because I'm testing this values matcher on Response?
    • If using this values matcher on Request, additional keys/indexes are OK. See this behavior on compatibility suite

I'm not really understand the behavior of missing/additional keys/indexes. Look like it's the Postel's law, but in revert?

@tienvx
Copy link
Contributor Author

tienvx commented Nov 15, 2023

A friendly remind for @YOU54F and @JP-Ellis . Please take a look when you have time. Thanks

@JP-Ellis
Copy link

My bad! This one did drop off my radar. I'm look at it tomorrow

Copy link

@JP-Ellis JP-Ellis left a comment

Choose a reason for hiding this comment

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

As far as the PR changes, this all looks good to me.

As for the questions you raise above, that's maybe best answered by @rholshausen?

@rholshausen
Copy link

values is for use with maps, not arrays. I've never tried to use it with arrays, but I'd imagine it will either be a no-op or act like a type matcher on the array.

wrt question about additional keys/indexes, what do you compare these additional values to?

For instance, given the map {"a":"A","b":"B"}, using the values matcher you are indicating that the keys (a, b) should be ignored, and only the values be compared. I.e., it bascially treats the map as the array ["A", "B"] and compares that.

@tienvx

This comment was marked as off-topic.

@tienvx

This comment was marked as off-topic.

@tienvx
Copy link
Contributor Author

tienvx commented Nov 17, 2023

Look like it's a bug to me. I created a separated issue for it here pact-foundation/pact-reference#337

@tienvx
Copy link
Contributor Author

tienvx commented Nov 17, 2023

Merged as this code has been approved.

@tienvx tienvx merged commit 6378c9b into pact-foundation:ffi Nov 17, 2023
26 checks passed
@tienvx tienvx deleted the revert-values-matching branch November 17, 2023 00:36
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.

None yet

3 participants