-
Notifications
You must be signed in to change notification settings - Fork 6
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: make sure that name is always passed #25
Conversation
f62976d
to
e259652
Compare
test/inspect.test.js
Outdated
t.test('make sure project name is no-name', function (t) { | ||
t.deepEqual( | ||
result.package.name, | ||
'snyk-php-plugin' |
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.
how does this pass? shouldn't it be 'no_project_name' ?
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.
the cwd is the directory where the process is ran from (we run tap from the main project dir)
lib/composer.js
Outdated
@@ -100,6 +99,10 @@ function generateJsonReport( | |||
return data; | |||
} | |||
|
|||
function getDefaultProjectName() { | |||
return process.cwd().split(path.sep).pop(); |
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.
this should use also uses the root
parameter passed to inspect()
, as it's relative to the cwd
e259652
to
939ef49
Compare
lib/composer.js
Outdated
@@ -100,6 +101,10 @@ function generateJsonReport( | |||
return data; | |||
} | |||
|
|||
function getDefaultProjectName(manifestPath) { | |||
return path.dirname(manifestPath).split(path.sep).pop(); |
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.
you should use path.resolve
(here or above) so that you get the full path, otherwise if user runs snyk test
inside the php
project, this code will result in the name '.'
lib/composer.js
Outdated
@@ -75,10 +75,11 @@ function buildDependencies( | |||
} | |||
|
|||
function generateJsonReport( | |||
fileName, composerJsonObj, composerLockObj, systemVersions) { | |||
fileName, composerJsonObj, composerLockObj, systemVersions, basePath) { |
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.
would pass the basePath
as first param, just before fileName
939ef49
to
10ad625
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.
🍪
When we can't get a project name from the lock file we pass the directory where the project file is as the project name.