enhancement: YAMLLINT using --strict causes error and incorrect error count

[TommyE123]

Fixes

When running YAMLLINT with --strict to return non-zero exit code on warnings as well as errors, you will see an error message displayed. This happens when only finding a warning in the file its processed but unable to use the regex to record it. The count is also not reflective of the issues found.

Proposed Changes

Regex corrected to also select warnings across all for format options.
This code change fixes the error appearing and also counts the warnings when --strict is used and either warnings are found in a file or mixed errors and warnings. Please note with neither --strict or --no-warnings specified, warnings only appear in the list if there’s an error as well. So you won’t see warnings without at least 1 error in a file. If warnings don’t want to be counted and appear in this scenario then use --no-warnings to suppress them.

Test evidence

Github (Tested on Gitlab using --format github --strict)

Github

Parsable

Standard

Colored

Regex Testing

https://regex101.com/r/WaroYf/3

Readiness Checklist

Author/Contributor

• Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)

Reviewing Maintainer

• Label as either automation, bug, documentation, enhancement, infrastructure, or performance

[echoix]

I remember taking a look at the state of yamllint, see this comment and the ones below (the title of the issue isn't representative of the problem, the errors were shown before the output for a tool) #3446 (comment)

So there was more than a way to change the output to make it more parsable.

[TommyE123]

Apologies, I wasn’t aware that other conversation existed. I now see what you mean!

It seems there can be four possible output formats parsable, standard, standard_color, github depending on the environment where the linter is executed?

I've been testing in an Azure DevOps pipeline. I’m thinking as you mentioned the regex needs to cover other outputs.

Leave this with me and I’ll investigate further and see if there’s anything I can come up with.

Tom

[echoix]

Apologies, I wasn’t aware that other conversation existed. I now see what you mean!

It seems there can be four possible output formats parsable, standard, standard_color, github depending on the environment where the linter is executed?

I've been testing in an Azure DevOps pipeline. I’m thinking as you mentioned the regex needs to cover other outputs.

Leave this with me and I’ll investigate further and see if there’s anything I can come up with.

Tom

No worries, I had to search a little, as it was an ongoing conversation with multiple problems in the same issue.

I think that since we are a tool making the call, we could set the output format to parsable and parse it with that format. If I understand their docs correctly, the goal of this format is to be stable and to be used by tools, just like us.
So it's safe to assume that if a user wants to change the output format, it will break the regex count.
If the GitHub format isn't too different, it might be fun to have it working too, as it is a format dedicated to be parsable that lets the GitHub interface link errors to the line in the code. If it isn't possible, only keep the parsable format, it's the best bet.

It might be easier to look at their source code (updated), to understand what is what in their format.

[TommyE123]

Hi @echoix,

I've analysed all 4 different format options and updated the regex to handle them all. 😊

When working with log files uploaded to GitHub, I noticed that GitHub parses and formats the output for better display. Therefore, I applied the regex to the behind-the-scenes output for those cases.

Additionally, I've updated the good and bad example test files with the ones I used during testing.

I hope this is now okay.

Best regards,
Tom

[echoix]

Great! I will take a look later tonight, but great to see you managed to go above and beyond to support all formats!

[echoix]

Looks fine! I’ve resolved the merge conflict in the changelog, to allow CI to run one last time.

Thanks again for all the details, screenshots, I was spoiled as a reviewer for this one!

[echoix]

Hmm, do you have the same test failures for yaml prettier too? I've already launched a rerun and failed too

[TommyE123]

I've probably broken this by adding my example files at a late stage to the PR and not noticing there were other YAML linters using them to do test fixes.

I can take a look and hopefully sort this.

Tom

[echoix]

There are other linters that use specific files just for them, instead of generic ones that apply to other linters of the same language, if you'd rather keep your files and let the original yaml files.

We don't need to test the linter itself, only our integration for it, and that is calling it works correctly + reporting.

[TommyE123]

@echoix,

Hoping this is ok to merge now. 😊

Tom

[echoix]

That commit worked great !