search

codespell-project / actions-codespell

MIT License
74 stars 19 forks source link

WIP: Add support for quiet level #22

Open GuillaumeFavelier opened 3 years ago

GuillaumeFavelier commented 3 years ago

On mne-python, we would like to have access to the --quiet-level parameter to suppress some messages.

This PR adds support for the quiet level.

It's still work in progress, I am not familiar with actions design and I need help to write useful tests.

GuillaumeFavelier commented 3 years ago

The failures seem similar to those on master (f53408792b98a46c92075dfc0112858bc97be05d)

peternewman commented 3 years ago

Hi @GuillaumeFavelier ,

The failure running against git master within the test was because we had some breaking changes which were released in Codespell 2.0. The other bats failures are seemingly something that slipped through the net with how we're simulating part of our Docker run, I think I'm getting there now.

You're certainly welcome to add this option to the action, there is also another option now Codespell 2.0 has been released, you can just put everything in a config file ( https://github.com/codespell-project/codespell/#using-a-config-file ), which might be a lot easier, it depends on your preferences. Although I appreciate you might want it quiet for your users and verbose for your CI or vice versa.

peternewman commented 3 years ago

I've put this into draft as well for now @GuillaumeFavelier feel free to switch it back as appropriate, let me know if you need a hand with the BATS tests (once I've wrangled them into behaving again!).

peternewman commented 3 years ago

@GuillaumeFavelier if you update then the existing tests should come good and you can see some working examples to copy/paste in test/test.bats to add a test for this.

GuillaumeFavelier commented 3 years ago

I tried 8687724 but I have to admit, I still do not grasp exactly how it works

GuillaumeFavelier commented 3 years ago

Sorry for the huge delay :pray: I updated the branch and tried 19753a4 to "disable the bats diagnostics": Testing / Diagnose bats are green but not Testing / Run tests though

peternewman commented 3 years ago

Sorry for the huge delay :pray:

No worries @GuillaumeFavelier

I updated the branch and tried 19753a4 to "disable the bats diagnostics":

Sorry, I explained that really badly/not at all. I actually meant remove that line. Unfortunately #24 doesn't make it as simple as I'd hoped.

Testing / Diagnose bats are green but not Testing / Run tests though

I've now changed diagnose bats to mimic your test run (one of the things I hoped to be able to automate but it hasn't quite happened yet).

So the dummy bats diagnostics run, shows what bats will be trying to check: https://github.com/codespell-project/actions-codespell/pull/22/checks?check_run_id=1889049192#step:5:28

Which as you can see returns 0 errors: https://github.com/codespell-project/actions-codespell/pull/22/checks?check_run_id=1889049192#step:5:40

But in the bats test you're expecting errorCount=$QUIET_LEVEL_MISSPELLING_COUNT with QUIET_LEVEL_MISSPELLING_COUNT=1 https://github.com/codespell-project/actions-codespell/pull/22/files#diff-e98dd325e580e64931383363be3819d3e40938c4ad0fb8a868056d4c58aa80f8R88

So as per the previous comment you'll need to make those two match up for starters.

GuillaumeFavelier commented 3 years ago

So as per the previous comment you'll need to make those two match up for starters.

Oh! Then what I want to test is not the number of errors but the number of warnings. Is there a way to do that? Or the opposite let's say turn warnings into errors?

peternewman commented 3 years ago

So as per the previous comment you'll need to make those two match up for starters.

Oh! Then what I want to test is not the number of errors but the number of warnings. Is there a way to do that?

Probably not based on the count, but you could also do line matching as some of the other examples show.

If you had a warning sandwich:

ERROR
WARNING
ERROR

Then you could check the line contents of all three, and then disable the warning and check the two still matched maybe.

In theory you could just count all the lines returned, but as you'll see with the add/remove matcher lines that can get a bit complicated if we can't fake them up correctly.

Or the opposite let's say turn warnings into errors?

I'm not sure off-hand if there's a way to do that in Codespell currently, although maybe we should generally add one.