search

svenkreiss / html5validator

Command line tool to validate HTML5 files. Great for continuous integration.
MIT License
314 stars 34 forks source link

Regression in 0.3.2 with respect to --ignore #64

Closed Morendil closed 4 years ago

Morendil commented 4 years ago

I have yet to properly investigate this but it seems that this latest release might be causing (some?) --ignore directives to be, well, ignored.

See this Circle Build https://circleci.com/gh/betagouv/beta.gouv.fr/4323

Morendil commented 4 years ago

After investigating some more: there is a difference in behaviour between 0.3.1 and 0.3.2 - ignored errors were previously squelched and are now logged.

This isn't necessarily a regression and does not appear to be the cause of the broken build.

Morendil commented 4 years ago

Upon reflection I would consider this a regression: it makes the search for the actual error causing the build to fail a search for a needle in a haystack.

php-coder commented 4 years ago

The same here but with the --ignore-re option.

svenkreiss commented 4 years ago

How about varying the log levels? Messages that are matched to ignore strings go to LOG.debug() versus actual problems go to LOG.info()?

php-coder commented 4 years ago

As a user when I change log level I'd expect to see the same number of violation, not more than I saw before.

What is the case where it's opposite? Why someone would like to see also suppressed messages?

svenkreiss commented 4 years ago

Debug. Besides being marked as debug, we can add an [ignored] label to the message itself.

svenkreiss commented 4 years ago

Just tried to look into this a bit more ...

The link in your first post doesn't work (anymore?). Is the problem that there are too many debug messages? Looked at the code and it looks like all this extra output is in the debug logger. Is that your issue?

Too many debug messages are not an issue. More debug is better. If you need to run "debug" because some important messages are only visible there, then that needs to be addressed.

php-coder commented 4 years ago

Hm, yes, maybe that the issue. So, it means that the default log level has been changed from WARN to DEBUG?

--log LOG log level: DEBUG, INFO or WARNING (default: WARNING)

Perhaps, README.md should be updated then. But I'll try to play with this option tonight.

Thanks!

Morendil commented 4 years ago

The link in your first post doesn't work (anymore?).

That's weird, these CircleCI jobs are public; the link currently brings up the job output for me at present, even in an incognito window. What are you seeing? A 404?

Is the problem that there are too many debug messages?

We're running html5validator as part of a CI test suite, on the output of a Jekyll build which generates many pages. In many of these pages we are outputting an img tag with empty src. Long ago we decided that this was OK and so we used the following command for validation:

html5validator --root _site --ignore 'Element "img" is missing required attribute "src"'`

As of 0.3.1 this resulted in no output to the console at all when all our HTML validated correctly (other than the empty tag which we are explicitly ignoring).

Starting with 0.3.2 we are getting lots of extraneous and unwanted output. This is not problematic when the build is passing, but when the build is failing the actual error which is causing it to fail (by returning an exit code of 1 from the validation command) is buried among hundreds of copies of the error message we specifically requested to ignore. That makes it hard to diagnose the reason for the failing build.

Does that clarify why the behaviour in 0.3.2 seems incorrect?

php-coder commented 4 years ago

... is buried among hundreds of copies of the error message we specifically requested to ignore. That makes it hard to diagnose the reason for the failing build.

I totally agree!

In my case there is around 2600 lines of error messages from checks that I ignore: https://travis-ci.org/php-coder/mystamps/jobs/616310191

php-coder commented 4 years ago

But I'll try to play with this option tonight.

I tried to specify --log option with different levels but it didn't work.