pact-foundation / pact-reference

Reference implementations for the pact specifications
https://pact.io
MIT License
91 stars 46 forks source link

Interactions with same request and state names, but different parameter values are treated as the same #467

Open timvahlbrock opened 4 days ago

timvahlbrock commented 4 days ago

I'm not sure if this is by design, but I haven't found anything on this so far. If two interactions have the same request and state description, but use different state parameter values, they are still treated the same and only one of them is added to the contract file. If this is by design, it may be helpful to get some sort of warning (if the architecture even allows to detect this), because I'm currently working on an adaption of contract testing in an existing project and just realized that two contract tests weren't saved to the contract file, as they had only differed in state parameter values.

timvahlbrock commented 3 days ago

The following pact-js example produces a single interaction in the contract.

import { LogLevel, PactV4, SpecificationVersion } from "@pact-foundation/pact";
import { HTTPMethods } from "@pact-foundation/pact/src/common/request";

describe("check if it generates two interactions if requests are same, but state is different", () => {
    it("check if it generates two interactions if requests are same, but state is different", () => {
        const pact = new PactV4({
            consumer: "Consumer",
            provider: "Provider",
            spec: SpecificationVersion.SPECIFICATION_VERSION_V4,
            logLevel: (process.env.LOG_LEVEL as LogLevel) || "warn",
        });

        pact.addInteraction()
            .given("one state", {
                value: "the first value"
            })
            .uponReceiving("a request")
            .withRequest(HTTPMethods.GET, "/test")
            .willRespondWith(200)
            .executeTest(async (mockServer) => {
                await fetch(mockServer.url + "/test")
            })

        pact.addInteraction()
            .given("one state", {
                value: "the second value"
            })
            .uponReceiving("a request")
            .withRequest(HTTPMethods.GET, "/test")
            .willRespondWith(200)
            .executeTest(async (mockServer) => {
                await fetch(mockServer.url + "/test")
            })
    })
})
YOU54F commented 3 days ago

parameters haven't been considered, I mirrored pact-ruby behaviour where it doesn't consider state values with params. I think its a fair addition

https://github.com/pact-foundation/pact-reference/pull/437

PR would be great, or as a starting point, generating some example test cases (the existing ones in the above PR should give you a start)

timvahlbrock commented 3 days ago

PR would be great

I would be happy to provide a PR, but 'm not sure whether I am able to provide a PR any time soon, especially because I have very little experience with Rust. Might check with my employer. One thing that came to my attention: this change would probably encourage modifications in the OSS pact broker, which currently lists interactions by request and state description (without params).