Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for disabling colored output #340

Closed
stan-is-hate opened this issue Sep 18, 2023 · 8 comments
Closed

Add support for disabling colored output #340

stan-is-hate opened this issue Sep 18, 2023 · 8 comments
Labels
enhancement Indicates new feature requests good first issue Indicates a good issue for first-time contributors help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@stan-is-hate
Copy link
Contributor

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 -

int pactffi_verifier_set_coloured_output(struct VerifierHandle *handle,

so it looks like it's just a matter of exposing it via VerifyRequest

@stan-is-hate
Copy link
Contributor Author

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:

  • default to no color in pact-go - which means our default is different from what main pact lib does
  • call the variable DisableColorOutput or smth - which means the variable is exactly the opposite of what pact lib has.

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

@stan-is-hate
Copy link
Contributor Author

Here's my stab at it, but having trouble running tests locally - something something avro something.
Maybe it will work better in CI?
#341

@YOU54F
Copy link
Member

YOU54F commented Sep 19, 2023

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

pact-go/Makefile

Lines 43 to 49 in b93338e

download_plugins:
@echo "--- 🐿 Installing plugins"; \
./scripts/install-cli.sh
~/.pact/bin/pact-plugin-cli -y install https://github.com/pactflow/pact-protobuf-plugin/releases/tag/v-0.3.4
~/.pact/bin/pact-plugin-cli -y install https://github.com/pact-foundation/pact-plugins/releases/tag/csv-plugin-0.0.1
~/.pact/bin/pact-plugin-cli -y install https://github.com/mefellows/pact-matt-plugin/releases/tag/v0.0.9
~/.pact/bin/pact-plugin-cli -y install https://github.com/austek/pact-avro-plugin/releases/tag/v0.0.3

@YOU54F
Copy link
Member

YOU54F commented Sep 19, 2023

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
Copy link
Contributor Author

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
Copy link
Member

YOU54F commented Sep 19, 2023

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
Copy link
Contributor Author

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 mefellows added help wanted Indicates that a maintainer wants help on an issue or pull request good first issue Indicates a good issue for first-time contributors enhancement Indicates new feature requests labels Sep 20, 2023
@mefellows
Copy link
Member

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

@YOU54F YOU54F closed this as completed Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests good first issue Indicates a good issue for first-time contributors help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
Status: Closed
Development

No branches or pull requests

3 participants