pact-foundation / pact_broker

Enables your consumer driven contracts workflow
http://pactflow.io
MIT License
701 stars 171 forks source link

Provide an option to view last verification result #154

Closed dhoomakethu closed 4 years ago

dhoomakethu commented 6 years ago

At present the only indication of verification is the color coded column Last verified on the pact broker home page, this information just tells if the last verification was a success or not. In case of failure, there is no way to know what exactly went wrong or which particular test failed through UI. An option to view the last verification result would be really good.

bethesque commented 6 years ago

Absolutely, I was just talking to @uglyog and @mefellows about this the other day. Currently, there is a field for putting the build URL in, but it's not really exposed, and the consumer team may not have access to the provder's build box anyway.

The decision we need to make is: do we have a specific JSON structure for reporting, or do we allow the "native" json formats of the ruby/jvm test output. Or, perhaps they are both the same anyway. Let's investigate.

bethesque commented 6 years ago

This is the json output from the RSpec formatter:

{
  "examples": [
    {
      "description": "has status code 200",
      "file_path": "/redacted/.gem/ruby/2.4.1/gems/pact-1.17.0/lib/pact/provider/rspec.rb",
      "full_description": "Verifying a pact between me and they Greeting with GET / returns a response which has status code 200",
      "id": "/redacted/.gem/ruby/2.4.1/gems/pact-1.17.0/lib/pact/provider/rspec.rb[1:1:1:1:1]",
      "line_number": 122,
      "pending_message": null,
      "run_time": 0.111762,
      "status": "passed"
    },
    {
      "description": "has a matching body",
      "file_path": "/redacted/.gem/ruby/2.4.1/gems/pact-1.17.0/lib/pact/provider/rspec.rb",
      "full_description": "Verifying a pact between me and they Greeting with GET / returns a response which has a matching body",
      "id": "/redacted/.gem/ruby/2.4.1/gems/pact-1.17.0/lib/pact/provider/rspec.rb[1:1:1:1:3]",
      "line_number": 139,
      "pending_message": null,
      "run_time": 0.000235,
      "status": "passed"
    },
    {
      "description": "has status code 200",
      "file_path": "/redacted/.gem/ruby/2.4.1/gems/pact-1.17.0/lib/pact/provider/rspec.rb",
      "full_description": "Verifying a pact between me and they Given There is a greeting Provider state success with GET /somestate returns a response which has status code 200",
      "id": "/redacted/.gem/ruby/2.4.1/gems/pact-1.17.0/lib/pact/provider/rspec.rb[1:2:1:1:1:1]",
      "line_number": 122,
      "pending_message": null,
      "run_time": 0.006892,
      "status": "passed"
    },
    {
      "description": "has a matching body",
      "file_path": "/redacted/.gem/ruby/2.4.1/gems/pact-1.17.0/lib/pact/provider/rspec.rb",
      "full_description": "Verifying a pact between me and they Given There is a greeting Provider state success with GET /somestate returns a response which has a matching body",
      "id": "/redacted/.gem/ruby/2.4.1/gems/pact-1.17.0/lib/pact/provider/rspec.rb[1:2:1:1:1:3]",
      "line_number": 139,
      "pending_message": null,
      "run_time": 0.000176,
      "status": "passed"
    }
  ],
  "summary": {
    "duration": 0.123096,
    "errors_outside_of_examples_count": 0,
    "example_count": 4,
    "failure_count": 0,
    "pending_count": 0
  },
  "summary_line": "4 examples, 0 failures",
  "version": "3.7.0"
}

Is there a standard json output for the JVM world @uglyog?

dhoomakethu commented 6 years ago

@bethesque The RSpec JSON output looks pretty neat. I would vouch to have a UI populated based on that. You can choose to present the raw data as it is and provide an option to export it to a html or have the UI provide a tabulated view of tests with individual results. This is a typical output of pytest-html plugin. Which fits to the JSON example by you.
image

mefellows commented 6 years ago

I envisage a "builds" screen (or set of screens) where these build results can be drilled down into, including context back to the CI where it ran, the particular interaction that failed and its line number etc.

Having this accessible via API initially is the first step, then we can web-enable it.

signed commented 6 years ago

Did you consider to include the actual response from the provider in the verification upload? This way you could 'just' run the verification logic on the broker and do not need a separate format for the verification result all implementations need to support.

bethesque commented 6 years ago

I hadn't thought of that actually @signed. It would be significantly more work however. It involve new resources, new client code, and redoing the entire way pacts are verified. The current code is almost done, I just need time to finish of the last bit.

signed commented 6 years ago

It involve new resources, new client code, and redoing the entire way pacts are verified.

Would this be true, even if you had not yet started implementing the verification upload? Just trying to understand the big picture, as I am still new to Pact.

bethesque commented 6 years ago

Yes. To add the test results, we just need to add some JSON to the existing verification results JSON. Pact has always run the verifications and done the matching locally. That's not to say we couldn't do it in the broker in the future, however, as I said, it would involve significant change.

Thadir commented 6 years ago

Yes. To add the test results, we just need to add some JSON to the existing verification results JSON. Pact has always run the verifications and done the matching locally. That's not to say we couldn't do it in the broker in the future, however, as I said, it would involve significant change.

I think it's also important that the consumer/provider system is done locally you don't want your builds to fail by weird fuckups in your network or connectivity.

bethesque commented 6 years ago

True, we are trying to rule out network connectivity issues as a reason for test failures. Pact can operate completely locally without a broker if need be, which I think is a strength.

signed commented 6 years ago

My intention with uploading the actual response to the broker was to avoid defining a new transport format for uploading verification results. To serialize into this format is extra effort for each language provider implementation out there. Just upload the provider response as a json blob. By uploading the response the broker can rerun the verification on demand (or run once and cache) on the broker side providing the validation result for the consumer. As there is already a ruby implementation that does verify a json response against a pact, it seemed easy to me to do. Yes, you would lose line information (and others) from the provider test, but in my opinion they are not of much use for the consumer anyway.

Having the provider response on the broker I can feed it back into the consumers (yes you mentioned you see potential problems with that, but I'm not convinced :).

I 100% agree, that validation in the CI should be done locally without making the pact broker mandatory.

bethesque commented 6 years ago

@uglyog here's a go at a potential format. I was hoping to make the differences array match what you're currently returning in rust/jvm. Could you send me a sample?

{
  "success": false,
  "providerApplicationVersion": "1.2.3",
  "testResults": {
    "tests": [
      {
        "testDescription": "has status code 200",
        "testFullDescription": "Verifying a pact between Foo and Bar A request for something with GET /something returns a response which has status code 200",
        "status": "passed",
        "interactionProviderState": null,
        "interactionDescription": "a request for something",
        "actualStatus": 200
      },
      {
        "testDescription": "has a matching body",
        "testFullDescription": "Verifying a pact between Foo and Bar A request for something with GET /something returns a response which has a matching body",
        "status": "failed",
        "interactionProviderState": null,
        "interactionDescription": "a request for something",
        "exception": {
          "class": "RSpec::Expectations::ExpectationNotMetError",
          "message": "Actual: [{}]\n\nDiff\n--------------------------------------\nKey: - is expected \n     + is actual \nMatching keys and values are not shown\n\n-\"something\"\n+[\n+  {\n+  },\n+]\n\n\nDescription of differences\n--------------------------------------\n* Expected \"something\" but got an Array at $"
        },
        "actualBody": [
          {
          }
        ],
        "differences": [
          {
            "description": "Expected \"something\" but got an Array at $"
          }
        ]
      }
    ],
    "summary": {
      "testCount": 2,
      "failureCount": 1
    }
  }
}
bethesque commented 6 years ago

Some things to consider - the ruby code executes one test for the status, one test for each of the headers, and one for the body. I would expect the JVM/Rust to have a different granulation, so we need to account for that. The key fields are "interactionProviderState" and "interactionDescription". I realise provider state will be an array in v3. Perhaps we should make this forwards compatible with that.

bethesque commented 4 years ago

Done in Pactflow.