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

Suppress version constraint token scanner errors #247

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Mar 15, 2021

The text/scanner is used for constraint expression tokenization. If no error handler is provided to the scanner then errors are reported to stdout by default. This PR adds a handler which ignores all scanner tokenization errors, which is safe under the current context (see the added comments for details).

Closes #246

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman wagoodman self-assigned this Mar 15, 2021
@wagoodman wagoodman added the bug Something isn't working label Mar 15, 2021
@wagoodman wagoodman requested a review from a team March 15, 2021 19:11
Copy link
Contributor

@luhring luhring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird af!

@alfredodeza
Copy link
Contributor

I'm guessing there is no easy way to test this one out right? Otherwise, I would love to catch this with a test, brownie points if it can catch other unexpected output as well as the one this fixes.

@wagoodman
Copy link
Contributor Author

@alfredodeza I thought about what kind of test I would write and how to write it. It can't easily be done in unit testing without affecting other tests, so that's not an option. Adding a new CLI-level test to account for it was something I considered, but felt it to be a little heavy handed for testing if the stdlib scanner configuration was correct (make certain this specific error text does not appear).

I think we can add a more generic test like you described last (check that output matches the given output, fail if anything else is given), but can be added in a follow up PR as a CLI test to verify grype scan output for multiple test-fixtures. That test would add a lot of value outside of this specific behavior being fixed on this PR.

@wagoodman wagoodman merged commit ec1f11f into main Mar 15, 2021
@wagoodman wagoodman deleted the suppress-token-error branch March 15, 2021 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reporting "exponent has no digits" on go case
3 participants