Potential new feature - add an optional comments to the interaction

[bethesque]

I had a discussion with Andras and Tim the other day that resulted in this suggestion:

some_provider
  .uponReceiving("a request to do something that is not quite clear from just the request and response data")
   ...
  .will_respond_with(202)
  .comment("This allows me to specify just a bit more information about the interaction")
  .comment("It has no functional impact, but can be displayed in the broker HTML page, and potentially in the test output")
  .comment("It could even contain the name of the running test on the consumer side to help marry the interactions back to the test case")

{
  "interactions": [
    {
      ...
      "comments": ["This allows me to specify just a bit more information about the interaction",
        "It has no functional impact, but can be displayed in the broker HTML page, and potentially in the test output",
        "It could even contain the name of the running test on the consumer side to help marry the interactions back to the test case"]
    }
  ]
}

cc: @mefellows @uglyog @TimothyJones @abubics

[TimothyJones]

in our case, this could be entirely satisfied by exposing test names to the broker. Of course, this is a challenge with some test frameworks.

[bethesque]

Yes, it's easy enough in Ruby, but I don't know how many other languages expose it.

[mefellows]

I'm not sure how I feel about this, albeit I can see the need to want to be able to link in with other tools.

Perhaps some more context of your use case @TimothyJones would help in finding an appropriate solution?

[abubics]

@mefellows: the initial issue surfaces as:

1. I write a nice test suite from the consumer-side,
2. I go to implement the provider side,
3. the pact doesn't have test names (intent descriptions) in it,
4. I have to look at the consumer tests to understand.

I think it's solvable in a few ways, including (but probably not limited to):

1. giving people direction on how to name states and requests,
2. pulling test names through to the pact file.

2 is a bit hard (maybe impossible) to do automatically, and adding this comment facility is a workaround for that, but also maybe useful for other things?

[mefellows]

It's an interesting point.

One thing we've been discussing is the ability to share provider states via the Broker, so that everyone can easily understand the various states of the system (and to avoid duplication in the Provider).

Another feature is creating a PR-like process for consumers to request new features/states/etc. from the Provider, essentially enforcing an approval workflow before the pact is 'accepted'. This mitigates a number of situations, the main one being consumers breaking Provider builds because a new state or expectation has been shoved in the system without prior consultation.

There is opportunity to capture some of this as part of that process too.

I can't quite articulate why this feature doesn't feel right to me belonging in the specification, perhaps because it's specific or not general enough (maybe metadata or similar is more appropriate), but it does feel like a useful thing to have.

[abubics]

I think it feels wrong because it's immaterial.

A mechanism like PRs might work. My approach to that is normally: consumers talk to providers to build the thing before they depend on it with a pact.

It's not a great solution, by any means, but it's the simplest for a declaratice-currency world: it seems unreasonable to declare a dependency before the dependency is available.

Drifting dependencies is a totally separate issue, which Pact addresses nicely.

I guess it depends what specific problems you're trying to solve. Is Pact for contract management, or just verification? Migration or current state? etc.

[uglyog]

I think a metadata section would be better for this, because then you could have keys to provide some context. I.e.

{
  "interactions": [
    {
      ...
      "metadata": {
        "comments": ["This allows me to specify just a bit more information about the interaction",
          "It has no functional impact, but can be displayed in the broker HTML page, and potentially in the test output",
          "It could even contain the name of the running test on the consumer side to help marry the interactions back to the test case"],
        "testname": "example_test.groovy"
       }
    }
  ]
}

A problem is that message interactions already have a metadata section for message metadata (for things like destination channel names and content types), so it would need a different key.

[TimothyJones]

metametadata? 😂

What if the whole section were called comments? That would clearly communicate that it isn't data that's expected to be part of the contract.

{
  "interactions": [
    {
      ...
      "comments": {
        "text": ["This allows me to specify just a bit more information about the interaction",
          "It has no functional impact, but can be displayed in the broker HTML page, and potentially in the test output",
          "It could even contain the name of the running test on the consumer side to help marry the interactions back to the test case"],
        "testname": "example_test.groovy"
       }
    }
  ]
}

As an aside, the testname part is a bit of a challenge in JS - it's hard to (easily/consistently/without user effort) get the test name or currently running spec (and the same may be true in other languages).

[uglyog]

Implemented the following in Pact-JVM:

JUnit DSL in Test:

      .given("good state")
      .comment("This is a comment")
      .uponReceiving("V3 PactProviderTest test interaction")
      .path("/")
      .method("GET")
      .comment("Another comment")
      .willRespondWith()
      .status(200)
      .body("{\"responsetest\": true, \"version\": \"v3\"}")
      .comment("This is also a comment")
      .toPact(V4Pact.class);

Pact file contents:

{
 "interactions": [
    {
      "comments": {
        "testname": "runTest(au.com.dius.pact.consumer.junit.v4.V4HttpPactTest)",
        "text": [
          "This is a comment",
          "Another comment",
          "This is also a comment"
        ]
      },
      "description": "V3 PactProviderTest test interaction"
}

Output when verifying:

Verifying a pact between V4 Client and Activity Service
  [Using au.com.dius.pact.core.model.UnknownPactSource@50cb8e27]
  Given good state
         WARNING: State Change ignored as there is no stateChange URL
  V3 PactProviderTest test interaction

  Test Name: runTest(au.com.dius.pact.consumer.junit.v4.V4HttpPactTest)

  Comments:
    This is a comment
    Another comment
    This is also a comment

    returns a response which
      has status code 200 (OK)
      has a matching body (FAILED)

[bethesque]

Cool!

Given that we're following camel case naming, is it too late to change 'testname' to 'testName'?

[uglyog]

Comments displayed in the Rust verifier

$ ./target/debug/pact_verifier_cli -f pact_matching/tests/v4-http-pact-comments.json

Verifying a pact between test_consumer and test_provider
  test interaction with a binary body

  Test Name: example_test.groovy

  Comments:
    This allows me to specify just a bit more information about the interaction
    It has no functional impact, but can be displayed in the broker HTML page, and potentially in the test output
    It could even contain the name of the running test on the consumer side to help marry the interactions back to the test case

      Request Failed - Invalid response: error sending request for url (http://localhost/): error trying to connect: tcp connect error: Connection refused (os error 111)


Failures:

1) Verifying a pact between test_consumer and test_provider - test interaction with a binary body - Invalid response: error sending request for url (http://localhost/): error trying to connect: tcp connect error: Connection refused (os error 111)


There were 1 pact failures