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

Fix 'none' severity findings breaking tests #586

Merged
merged 4 commits into from
Nov 5, 2024

Conversation

egibs
Copy link
Member

@egibs egibs commented Nov 5, 2024

Closes #585

Our tests were not applying a consistent level of minimum risk. To avoid showing behaviors with none for their severity, this PR sets MinFileRisk and MinRisk to 1 for all tests. For reference, TestJSON was already using MinRisk: 1.

@egibs egibs requested a review from tstromberg November 5, 2024 19:26
@egibs egibs changed the title Fix 'none' severity findings from breaking tests Fix 'none' severity findings breaking tests Nov 5, 2024
@@ -98,6 +98,8 @@ var dateRe = regexp.MustCompile(`[a-z]{3}\d{1,2}`)
// Map to handle RiskLevel -> RiskScore conversions.
var Levels = map[string]int{
"harmless": 0,
"ignore": 0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to make sure that the API boundary is clear without any unanticipated surprises. The fact that the API returns dropped results would be a surprise to me.

What do you think about assigning the levels to be dropped by default (at least ignore and none) as -1? I'm iffy on harmless, it's probably OK as is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. I like that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated in 323a1e0 (#586).

@egibs egibs requested a review from tstromberg November 5, 2024 19:57
@tstromberg tstromberg merged commit 09c6121 into chainguard-dev:main Nov 5, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestSimple/linux/clean/busybox: failing due to overridden "none" results
2 participants