pact-foundation / pact-go

Golang version of Pact. Pact is a contract testing framework for HTTP APIs and non-HTTP asynchronous messaging systems.
http://pact.io
MIT License
831 stars 104 forks source link

Add support for disabling colored output #340

Open stan-is-hate opened 8 months ago

stan-is-hate commented 8 months ago

Our team is trying to display pact provider verification results in PRs. To do that, we parse the test output and grab the pact provider summary.

However, it contains ANSI color codes by default, which we remove via a hairy regex.

It seems pact ffi already supports disabling colors via pactffi_verifier_set_coloured_output function here - https://github.com/pact-foundation/pact-go/blob/b93338ea05d98e89c365c8c4e44067dc1bf0376c/internal/native/pact.h#L2734 so it looks like it's just a matter of exposing it via VerifyRequest

stan-is-hate commented 8 months ago

I can take a stab at implementing this, since it doesn't look too difficult. The tricky part is what to call the variable. pact lib default value is true, so they have smth like coloured_output bool which defaults to true - and of course we can't default to true in VerifyRequest. So we can either:

I'm leaning towards option 2, to preserve colored output ppl expect.

stan-is-hate commented 8 months ago

Here's my stab at it, but having trouble running tests locally - something something avro something. Maybe it will work better in CI? https://github.com/pact-foundation/pact-go/pull/341

YOU54F commented 8 months ago

Hey thanks for this, option 2 sounds better for me.

You might need the avro plugin downloaded locally, we should document that in a contributing / developing guide.

make download plugins will do the trick

https://github.com/pact-foundation/pact-go/blob/b93338ea05d98e89c365c8c4e44067dc1bf0376c/Makefile#L43-L49

YOU54F commented 8 months ago

contrib guide states run make pact but that job doesn't run make deps which would install the plugin

make ci or make fake_ci would, just dry reading it

stan-is-hate commented 8 months ago

I tried make deps and make download_plugins and that's when the error changed from missing plugin to an error in the avro plugin. But seems to have passed on the PR, is that good enough?

YOU54F commented 8 months ago

yeah thats a complete aside, it's irrelevant for your PR. I'll leave the decision on merging and any code style comments to the maintainer, I've tagged him for review 👍🏾

I assume you've built this locally and tested out that you get the expected behaviour in your use case?

stan-is-hate commented 8 months ago

I have disabled the avro tests and ran make fake_pact. I've also changed the log level from TRACE to INFO on all provider tests, otherwise too much output. And yes the option works as intended, removing color and bold fonts from output:

Screenshot 2023-09-19 at 13 03 50

VS

Screenshot 2023-09-19 at 13 04 11
mefellows commented 8 months ago

Thanks for the PR, will merge once feedback is in :)