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

feat: Include devDependencies in default test from Wizard #26

Closed
wants to merge 1 commit into from
Closed

feat: Include devDependencies in default test from Wizard #26

wants to merge 1 commit into from

Conversation

adam-moss
Copy link

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by @remy (Snyk internal team)

What does this PR do?

(This was previously raised as PR #14 but I... errr... broke it 😊)

Adds --dev to the default snyk test script generated by the Wizard.

Where should the reviewer start?

How should this be manually tested?

  • Create a new package (e.g. npm init)
  • Run snyk wizard and allow it to update the test script
  • Review package.json

Any background context you want to provide?

By default npm install will include devDependencies unless --production or the relevant environment variable is set.

Therefore it would be sensible to reflect this in the implementation of snyk to ensure consistency.

What are the relevant tickets?

Screenshots

Additional questions

  • Test wizard detects existing snyk in scripts.test now fails because of the way it has been implemented. I haven't changed this as I'm unsure if you'd prefer to mock the entire thing.

@adam-moss
Copy link
Author

Fixed broken test 👍

@adam-moss adam-moss changed the title Wizard: Include devDependencies in default test. feat: Include devDependencies in default test from Wizard May 12, 2016
Add --dev to test script generated during the Wizard process
Update internal tests to reflect the same
@Niponkong
Copy link

Niponkong commented Sep 13, 2017 via email

@maban
Copy link
Contributor

maban commented Sep 14, 2017

Hi @adam-moss! We've been thinking about this use case a lot, but we don't want to test dev dependencies by default as they are not usually exploitable in production.

However, enabling the testing of dev dependencies when the user opts-in to do so (eg when running snyk wizard --dev) would be cool. Would you consider changing your code to make testing dev dependencies possible using a flag?

@adam-moss
Copy link
Author

Sure, I can rebase and enable --dev on wizard.

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.

3 participants