Ability to specify formatter

[joshhendo]

This is a proof of concept PR to start the discussion behind this feature. It certainly need agreement and tests added in before it is ready to be merged.

[mboudreau]

So, one is to specify the format, the other is a flag to exclude all outputs coming from pact-node? I didn't even know the pact ruby binary did the format, @bethesque do you want to weigh in on this change?

Thanks for your contribution @joshhendo :)

[joshhendo]

Yep, that's right. Adding the flag to exclude stderr was needed, otherwise the JSON and any INFO (and errors for that matter) will appear in the same string. By excluding it, you can just do JSON.parse on the string and work with it from there. There might be a better way to do it, such as outputting the stdout and strerr to different strings. I used the exclude flag more as a POC and it didn't involve a huge change

(as a side, I noticed when playing around with this that, when outputting JSON, the \n needs to be removed)

The motivation behind this (as I mentioned in the issue I opened) is to be able to have a nicer test output. By having it in JSON, and therefore parsable, it's possible to make tests dynamically based on the output of JSON... for example, here's a test I was able to put together by parsing the JSON:

[mefellows]

I'm totally open to this sort of thing. There is an issue on pact-js about doing this sort of thing (harder, as there are so many test frameworks).

FWIW we do this with pact-go as sub-tests - so we get granular test reporting (and therefore failures) which is really nice when something breaks. Without it, when you look at your CI build at a glance it's just "provider build failed" instead of "got a 404 instead of a 200 for service endpoint /foo/bar/1" type thing.

[bethesque]

@mboudreau yes, this is what the --format option is for. There's a request to add multiple --formats, some of which can output to files. I just haven't had time to pick it up yet.

[bethesque]

@joshhendo would you prefer it if the output went straight to a file, rather than having to scrape it from the stdout?

[joshhendo]

I'd be able to work with either. Grabbing it from stdout is actually easier since I want to parse it in code - if it were written to file, I'd just be reading it back into memory almost as soon as it finished. However, if writing to file makes the implementation cleaner, I'd be fine with doing that.

[bethesque]

Here's the issue. pact-foundation/pact-provider-verifier#21

[bethesque]

I think that suppressing output so that stdout can be scraped is not the most technologically elegant solution. This would be cleaner.

[bethesque]

On a side note, we really don't want the name of the RspecJunitFormatter to leak into the javascript. The value of format should be "json" or nil, and if it is nil, the format option should not be included in the command.

[joshhendo]

Sure, makes sense. Not sure this level of restriction is needed anyway (plus it's only for people using Typescript)... all other params are only doing basic type checks, not enum-like checking

[bethesque]

Sorry, I remember what the RspecJunitFormatter does now. I thought it was the default one, but it's actually the XML formatter. I guess we can put it in there, but given that other options could be used in the future, I feel hesitant about putting the validation into this top level.

[joshhendo]

Yep agreed. And this validation will only be for those using typescript anyway, rather than everyone. I'll remove it

[bethesque]

Ok, there is now an --out option to put the results into a file. https://github.com/pact-foundation/pact-ruby-standalone/releases#pact-provider-verifier

Usage:
  pact-provider-verifier PACT_URL ... -h, --provider-base-url=PROVIDER_BASE_URL

Options:
  -h, --provider-base-url=PROVIDER_BASE_URL                          # Provider host URL
  -c, [--provider-states-setup-url=PROVIDER_STATES_SETUP_URL]        # Base URL to setup the provider states at
  -a, [--provider-app-version=PROVIDER_APP_VERSION]                  # Provider application version, required when publishing verification results
  -r, [--publish-verification-results=PUBLISH_VERIFICATION_RESULTS]  # Publish verification results to the broker
  -n, [--broker-username=BROKER_USERNAME]                            # Pact Broker basic auth username
  -p, [--broker-password=BROKER_PASSWORD]                            # Pact Broker basic auth password
      [--custom-provider-header=CUSTOM_PROVIDER_HEADER]              # Header to add to provider state set up and pact verification requests. eg 'Authorization: Basic cGFjdDpwYWN0'. May be specified multiple times.
  -v, [--verbose=VERBOSE]                                            # Verbose output
  -f, [--format=FORMATTER]                                           # RSpec formatter. Defaults to custom Pact formatter. Other options are json and RspecJunitFormatter (which outputs xml).
  -o, [--out=FILE]                                                   # Write output to a file instead of $stdout.

Verify pact(s) against a provider. Supports local and networked (http-based) files.

It's a bit ugly in that if you verify multiple contracts, they're all in the one file, not separate ones. I'll add that later, but it probably won't be immediately. We should just add the --out option to the JS for now.

[bethesque]

@mboudreau we don't need the "suppress stdout" feature now, as the json or xml output will go directly to a file if specified.

[mboudreau]

@joshhendo can I ask how you're using pact-node? Are you doing it via CLI or via node api?

[mboudreau]

I need to have a bit of a think around this. We want to give the raw output that's JSON parseable without any extra fluff, but it's to do that on top of having logging to the console in case of errors and the like.

[bethesque]

@mboudreau this is a solved problem. The underlying ruby standalone can now write to a file. The only changes that have to be made in the node js are to expose --format and --out. Please update to the latest standalone version (1.38.0) and add these parameters.

[bethesque]

(This PR can be closed or updated with the above requirements)

[joshhendo]

@mboudreau I'm using it through the node api, basically the way this example does it:
https://github.com/pact-foundation/pact-js/blob/master/examples/e2e/test/provider.spec.js#L61

The way I see it is, at the moment 2 streams are merged into 1. For proof of concept I just excluded the error stream to show it worked, but I agree that's not something we'd want people to regularly do. If building from scratch, I'd contemplate designing the interface to be able to return the result and error strings separately, giving the best of both worlds (the stdout can be parsed to JSON, but we don't sacrifice any important error messages from stderr)

Writing to file when used via node-js is a bit clunky and certainly not as clean as streaming the result from stdout (but file is better when using it via CLI). However, given we can do write to file without changing interfaces, it is certainly workable (write to a temp file and make sure they're in .gitignore) and is easier to implement here -- I'll update the PR to implement this.

[mboudreau]

@bethesque I disagree, I don't think this is a fixed issue. We found a workaround, but I believe that @joshhendo has brought up a good point on how he's trying to use pact-node and want to use the output to be able to parse and use it, which would be the same as using linux command line to stream the output.

I'm having a think on how to accomplish this in the best way possible as to not mix logging with the actually output of the binaries in a way that makes sense.

[bethesque]

The solution I have proposed achieves the ultimate goal of providing a clean source of the JSON results that can be parsed by a calling tool. Whether it is read from stdout or from a file is not going to make a massive amount of difference to the calling code, I would have thought.

However, if you would like to achieve the original solution, the underlying ruby library already supports the separation of streams, so that when --json output is used, ONLY the json goes to stdout, and all other log messages to go stderr. You only need to worry about handling the logs written by the javascript code itself.

[bethesque]

Looks ok to me.

[joshhendo]

great. I'm just testing locally and if it all looks good, I'll add in some tests

[bethesque]

My general advice for testing something like this is to only test enough of the implementation to ensure that the interface has been called correctly - avoid trying to test the underlying implementation itself, as that has already been tested in the pact-ruby project.

Funnily enough, this would be the ideal situation for a contract test.

[mboudreau]

@joshhendo hey, I haven't forgotten about you, I've just been swamped with GDPR stuff at work and trying to get checksums working in Pact. I'll get to this as soon as I can. Cheers.

[joshhendo]

hey @mboudreau , sorry was meaning to reply to your message earlier - totally understand, GDPR has a lot of us swamped. Cheers for merging, I'm looking forward to integrating it into my pipeline :)

[mboudreau]

@joshhendo version 6.18.0 going out now with your changes. Cheers.