-
Notifications
You must be signed in to change notification settings - Fork 565
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: propagate failed scans with --all-projects #1301
Conversation
87ef9d9
to
c8598d3
Compare
analytics.add('pluginName', inspectResult.plugin.name); | ||
|
||
const postingMonitorSpinnerLabel = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved a little further down unchanged
93e7e73
to
b2954d6
Compare
allProjects: true, | ||
}); | ||
let result; | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these were not previously propagated up if dependencies failed to be detected, but we do fail if one of the paths fails to be scanned so this aligns that behaviour.
This may however start failing some pipelines that previously were skipping the failures. How can this be communicated & is this the desired behaviour right? You want to know something you monitored failed and it did not pass.
af49e17
to
7e9c7ef
Compare
7e9c7ef
to
5cc6eee
Compare
- failed monitor paths generate an error but not failed attempts to get dependencies, these were skipped and shown only with -d output. Propagate full status back to the user. - this may affect the expected exit code for some users where previous failures were skipped in this manner - log failed snyk test scans with --all-projects
5cc6eee
to
7ef59ed
Compare
Expected release notes (by @lili2311) fixes: others (will not be included in Semantic-Release notes):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, tested locally
🎉 This PR is included in version 1.369.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this PR do?
--all-projects
and propagate it higher up.monitor
these are now propagated all the way and will appear in output + json output. This may affect the existing error code as previously we silently skipped failure like these but not failures to get vulnerabilities during monitor or when a path test failed. So it was inconsistent.-d
Where should the reviewer start?
https://github.com/snyk/snyk/compare/fix/throw-erorr-if-all-tests-failed?expand=1#diff-e4ec2ebbae5004d31be7a5b680580a46R42
How should this be manually tested?
You can use
cd snyk/test/acceptance/workspaces && snyk test --all-projects
andcd snyk/test/acceptance/workspaces && snyk monitor --all-projects
to see the output.Any background context you want to provide?
Sometimes users see a blank screen when 100% of
-all-projects
scans fail and we do not flag about other failures as it was too noisy unless you run with-d
Screenshots