-
Notifications
You must be signed in to change notification settings - Fork 533
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
feat: unify policy handling & plucking #1223
Conversation
89b58e6
to
197f1ec
Compare
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.
We are having an issue on this test
- test/acceptance/cli-monitor/cli-monitor.acceptance.test.ts
monitor foo:latest --docker
doesnt send policy from cwd empty policy is sent:
Error: empty policy is sent
at Test.test (test/acceptance/cli-monitor/cli-monitor.acceptance.test.ts:1535:5)
9fa342b
to
72154d2
Compare
@@ -466,7 +454,7 @@ async function assembleLocalPayloads( | |||
targetFileRelativePath: `${targetFileRelativePath}`, // Forcing string | |||
projectNameOverride: options.projectName, | |||
originalProjectName, | |||
policy: policy && policy.toString(), | |||
policy: policy ? policy.toString() : undefined, |
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.
make it the same as other instances
@@ -1531,8 +1531,7 @@ test('`monitor foo:latest --docker` doesnt send policy from cwd', async (t) => { | |||
'calls docker plugin with expected arguments', | |||
); | |||
|
|||
const emptyPolicy = await snykPolicy.create(); | |||
t.deepEqual(req.body.policy, emptyPolicy.toString(), 'empty policy is sent'); | |||
t.deepEqual(req.body.policy, undefined, 'no policy is sent'); |
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.
make monitor & test behave the same way (preferring test)
When plucking policies this only works if we created a depenency tree with resolve-deps by traversing node_modules
72154d2
to
9228e50
Compare
Expected release notes (by @lili2311) features: fixes: others (will not be included in Semantic-Release notes):
|
🎉 This PR is included in version 1.346.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this PR do?
Any background context you want to provide?
There is a policy handling but with
--all-project
,gradle sub-projects
and--file
where we look for policy in the root of where the command is run and not next to the manifest, which can be exposed in a test and fixed in a PR after this initial refactor so that the change is made in one place not 4.