-
-
Notifications
You must be signed in to change notification settings - Fork 349
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
Use matchers when verifying metadata #745
Use matchers when verifying metadata #745
Comments
Thanks @mduesterhoeft. I just took a quick look at the source and it seems matchers should be applied. Could you please set the log level to trace and share an appropriately redacted log? That will help us diagnose what went wrong. |
Actually, on closer inspection that output is a bit confusing - The core knows how to read in both But it shouldn't actually be generating that. This might be a bug in Pact JS (not sure how yet) or in the version of the FFI we are using. (I'd still love those logs if you could please share) |
Can you tell us which pact framework + version produced the pact file? |
Sorry for the confusion. I played around with the contract a bit - and posted a modified version. The contract the consumer generates has So my snippet above is really this one:
This is the version information from the contract ( I tried with Here are the relevant pieces from the log
So it seems to recognize the |
Is there an update on this? I am using pact-js to generate a
On the provider side this fails with:
|
Hi @NiklasEi, that pact file looks broken - it should not have There have been issues with that in the past with the Ruby core, but please check you're on the latest version and have run a clean test. |
Thank you for the answer. Since this seems to be a different problem, I opened #763. |
@mefellows do you already have an idea on when you can fit this in. If you explain how you envision a solution I would also be happy to give it a try and come up with a PR. |
Thanks @mduesterhoeft, apologies for the delay in my response. I don't have the answer right now I'm afraid. I just had a look through the rust source code and it seems any references to "metaData" have been addressed. With the next release of the beta, I'm hoping this problem should be resolved. cc: @uglyog as he may be able to provide a little more detail. |
In the meantime, i'd remove it from your consumer test until we fix the validation (which I assume you're probably doing!) or you could potentially post-process the pact file to replace |
We tried it with |
👍
…On Tue, 16 Nov 2021, 11:06 pm Niklas Eicker, ***@***.***> wrote:
In the meantime, i'd remove it from your consumer test until we fix the
validation (which I assume you're probably doing!) or you could potentially
post-process the pact file to replace metaData with metadata to see if it
gets picked up properly.
We tried it with metadata, but then the metadata is ignored. At the
moment we just hardcode one value.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#745 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAANFDGBHF3A6PHF6MZXUVTUMJCLBANCNFSM5ERDXNBA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
@mefellows Hi there - do you have any update for me on this topic? I am also totally ready to get my hands dirty to fix this issue with a PR from my side. But I would need an initial pointer, solution idea to get started. |
Thanks @mduesterhoeft. I'm not sure if it has been resolved. You could
|
Thanks @mefellows
I did that before bothering you again. I have the same issue with the latest beta. If the solution would be in the rust core I am afraid I cannot help. I was hoping that this was in the js world still. |
Hi, I have a similar problem, I am using matchers in message consumer metadata like this .withMetadata({
contentType: "application/json", // Working
correlationId: Matchers.string("nCB3FYqJIugLvQZ2b1dV") // Not Working
}) but the generated Pact file contains only the example value {
"consumer": {},
"messages": [
{
"contents": {
// ...
},
"matchingRules": {
"body": {
"$.name": {
"combine": "AND",
"matchers": [
{
"match": "type"
}
]
}
}
},
"metadata": {
"contentType": "application/json",
"correlationId": "nCB3FYqJIugLvQZ2b1dV",
},
"providerStates": [
{
// ...
}
]
}
]
}
This causes the provider tests to fail since the correlationId is a generated value that changes each time, the test output looks like this
Both use PactJS 10.0.1. I have tried MatchersV3 instead but that gives my TypeScript errors since the withMetdata method expects Matchers. Is there anything else I can provide to help with this? |
This probably is a mistake in the pact v10 types - I think it would be more correct to give a V3 matcher there. However, I believe the Update: I think I found the issue: https://github.com/pact-foundation/pact-js/blob/master/src/messageConsumerPact.ts#L178 Could you try patching that line in your node_modules? I'm guessing a bit, but I think it should say:
@mefellows if this does fix it, I think pact-js should just pass the whole thing down to the core, and the core should do the conversion. |
@TimothyJones I tested your line and Pact json now looks like this {
"correlationId": "{\"value\":\"nCB3FYqJIugLvQZ2b1dV\",\"pact:matcher:type\":\"type\"}"
} |
Ah 😅 apologies. I think the next culprit is in the rust core. I’ll leave this for a maintainer to look at. |
@Kampfmoehre That is wrong, there are only matching rules for the body
|
The matching rules for the metadata are not being created in the Pact file. |
Does the rust core support matching rules on the metadata? I think that's the question. |
Can anyone confirm that matching rules are supported for metadata? I'm struggling to get the matching rules for metadata for an asynchronous message (Pact V4) working. The Pact file properly inculdes matching rules for the metadata, e.g.:
But somehow the log states (running it with
The Pact is created with Pact JVM |
Could you please re-test? The latest version should support this now. |
This still seems to be happening for me with version 12.1.1. When I include metadata in the pact like so:
It generates a pact contract that looks like this:
The header matching rules are now included in the pact, which I didn't see happening previously (not sure which version I last tried with, sorry) - but they aren't used by the provider during verification:
It still requires the UUID to match exactly, rather than using the regex specified in the matchers. |
I think this would be resolved via this change: https://github.com/pact-foundation/pact-reference/pull/343/files |
I checked with pact-js 12.1.2 and 12.2.0. For both versions
Any news on this? Any workarounds to be recommended? |
Supports metadata with matching rules Fixes pact-foundation/pact-js#1133 Fixes pact-foundation/pact-js#745
Supports metadata with matching rules Fixes pact-foundation/pact-js#1133 Fixes pact-foundation/pact-js#745
This should be fixed now - thanks all. |
Feature description
Since v10.0.0-beta.45
metaData
present in the consumer contract is verified.It would be great if we could also use matches when verifying metadata.
Something like this should be working.
Use case
The metaData object contains a
traceId
which is change whenever the consumer contract is published. The verification should just make sure that the field exists and carries a string. A match by value is not useful.The text was updated successfully, but these errors were encountered: