search

pactflow / pact-cypress-adapter

Cypress Pact Plugin
MIT License
25 stars 13 forks source link

Contract with matchers #19

Open mmadariaga opened 2 years ago

mmadariaga commented 2 years ago

Added usePactIntercept comand that allows to intercept network calls and record response and matching rules into a pact file

YOU54F commented 2 years ago

Oooooh this looks epic!

Can't wait to get some time to check this out.

Just had a quick glance, if we support v2 or v3 matchers, we will probably want to ensure the user only provides one set, and that the output pact specification value matches up with the used matchers

Kicked off the build in the mean time

YOU54F commented 2 years ago

Could we actually use matchers from pact-js to setup the response body, and use the reify method to strip the matchers from the response body used in cy.intercept

mefellows commented 2 years ago

Thanks @mmadariaga! I think it's a good suggestion, but it should be used with caution. Because the matching rules are being setup automatically, we'll have to make sensible suggestions in how they are applied.

In the simplest form, you just need a:

"$.body": {
  "match": "type"
},

And this will recursively apply the type matcher to the body. It gets tricky with arrays and things where you might want to actually apply an eachLike matcher (an array of len >= 1, with all objects conforming to the shape of the supplied object), but this is probably what most people expect.

So I don't think you need a whole extra method to catch this, and could just make it a serialisation option (or a request by request parameter).

mmadariaga commented 2 years ago

Hi @mefellows , This is my first time using both, cypress and pact, and I think I lack the knowledge to properly understand your points. I hope at least that this PR will encourage someone to implement the feature in a better way.

dbro04 commented 2 years ago

I think it would be better to add a set of matching rules to the PactConfigtype and use these rules in the response. In costructInteraction you can check if a rule exist for the property of the response.body. Therefor a rule consists of

There is no further need for nested types, because they could be handled in the parent type.

YOU54F commented 1 year ago

Hey @dbro04 would you be up for collabbing on the PR?

I still think using the PactJS matchers, and reifying the payload before its serialised to a pact might be a sensible idea. We can probably lift parts of the code rather than pulling in the project as a dep

Adding some help wanted labels to try and attract extra eyes

ishaqibrahimbot commented 1 year ago

Hey @YOU54F! I just started using the pact-cypress-adapter for a work project and realized the adapter doesn't currently support matchers. And then I found this PR.

It's quite important for my use case to make use of matchers - I want to use type-based matching and use stuff like eachLike for arrays and so on.

I'm open to collaborating with you on this PR. Here is my understanding of the approach you're suggesting:

Let me know if I've understood this correctly. If so, I'll start working on this via a fresh fork + PR.

YOU54F commented 1 year ago

👋🏾 Howdy!

Something like that, I think my main concern would be allowing the user to use pact-js style matchers, rather than having to express json path notation. A pact-js user wouldn't look inside the pact file regularly, so their experience would be with the v2/v3 matchers interface. We should take should of serialising them correctly to the pact file.

We would need to consider the pact specification that gets written to the file, currently it assumes a v2 spec. if we cater for matching rules for both spec versions, then we want to ensure only one format is used, and the correct specification is also serialised.

There are typed schemas for each of the pact specs that may be useful for validation.

https://github.com/pactflow/pact-schemas

whats the use case where you need to use matchers with this adapter. Are you unable to use pact-js in your project? Our general advise if is you are not comparing these contracts against an openapi specification whereby the schema matching rules are applied by the tool (swagger-mock-validator), is to generate the contracts at the lowest layer (your api client) and reuse those pact contracts, as stubs for higher level tests (including those run with cypress).

We use that pattern within PactFlow between its UI/backend app.

ishaqibrahimbot commented 1 year ago

Thanks for the quick reply @YOU54F!

Let me describe my use case: I'm working on a nextjs project for which we're using cypress to write all the component, integration, and e2e tests. As part of the project's scope, we are also required to set up contract tests via pact and pactflow to ensure no breaking changes are published by the provider (our backend).

Our solution architect suggested we use the cypress-pact plugin to reduce the maintenance burden since the idea was that generating contracts and using them would become a lot more seamless through the plugin. Recently while researching into this I found that the plugin was archived long ago and this adapter was published to work with cypress.

So I've configured the adapter in my project and have written a sample test using it that generates a pact file. My only issue now is that because of the lack of support of matchers, the broker checks the type and the value of a given property inside the response body instead of just making sure the property is of the right type. Only when I manually add matchingRules to the generated pact file does it work as I want.

So either you can give me some advice on what I should do for my use case, or we can somehow work on adding the matchers feature to the adapter.

Let me know if you need more context.

mefellows commented 1 year ago

https://github.com/pactflow/pact-cypress-adapter/pull/19#issuecomment-1810115070

This comment sums it up well. You will probably also want to implement provider states, because these are crucial to data preparation on the provider side.

Yousaf's comments are worth heeding also. One of the problems with using this adapter for pure Pact testing (and not BDCT for which it is intended) is that because you're testing at a higher level, you may end up with more tests than strictly necessary, further burdening the provider. See https://pactflow.io/blog/a-disastrous-tale-of-ui-testing-with-pact/ for more on that topic.

ishaqibrahimbot commented 1 year ago

Thanks @mefellows, I'll start working on this feature over the weekend.

Regarding Yousaf's comments, I did previously read the article you've linked and understand the pitfalls of overloading the provider with extra and to-some-extent duplicate requests. We're going to restrict the number of pacts we generate in our tests and try to keep them as different as possible. Thanks for the heads up!

mefellows commented 1 year ago

No worries - thanks and good luck!