Migrate test deps CI job to npm audit #5891
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
npm has informed me that the lockfile has "errors":
npm ERR! code ELOCKVERIFY
npm ERR! Errors were found in your package-lock.json, run npm install to fix them.
npm ERR! Missing: c3@^0.6.7
npm ERR! Invalid: lock file's d3@3.5.17 does not satisfy d3@^5.7.0
Auditing packages when installing here doesn't help anyone as the summary isn't visible and vulnerabilities don't produce a non-zero exit code. We will have `npm audit` as an extra CI job.
Refs MetaMask#4768 Refs MetaMask#5389 This changeset removes the beefy package that: 1. Was last published 2 yrs ago 2. Brought with it 1 moderate and 1 critical vulnerability 3. Was only used in scripts that no longer work
=== npm audit security report === > # Run npm install --save-dev ganache-core@2.3.1 to resolve 1 vulnerability ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ Moderate │ Memory Exposure │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ bl │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ ganache-core [dev] │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ ganache-core > level-sublevel > levelup > bl │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://nodesecurity.io/advisories/596 │ └───────────────┴──────────────────────────────────────────────────────────────┘
From `npm install`: > npm WARN The package css-loader is included as both a dev and production dependency. > npm WARN The package eslint-plugin-react is included as both a dev and production dependency. > npm WARN The package file-loader is included as both a dev and production dependency. > npm WARN The package gulp is included as both a dev and production dependency. It's also worth noting that the Gulp version we were using was inconsistent and there is a published v4 release on GitHub.
brunobar79
reviewed
Dec 7, 2018
| @@ -43,8 +43,6 @@ | |||
| "lint": "eslint .", | |||
| "lint:fix": "eslint . --fix", | |||
| "mozilla-lint": "addons-linter dist/firefox", | |||
| "ui": "npm run test:flat:build:states && beefy development/ui-dev.js:bundle.js --live --open --index=./development/index.html --cwd ./", | |||
| "mock": "beefy development/mock-dev.js:bundle.js --live --open --index=./development/index.html --cwd ./", | |||
There was a problem hiding this comment.
Since you're getting rid of this two commands, What should we do with:
- https://github.com/MetaMask/metamask-extension/blob/develop/docs/ui-mock-mode.md
- https://github.com/MetaMask/metamask-extension/blob/develop/docs/ui-dev-mode.md
- https://github.com/MetaMask/metamask-extension/blob/develop/development/mock-dev.js
- https://github.com/MetaMask/metamask-extension/blob/develop/development/ui-dev.js
There was a problem hiding this comment.
Yup, I'll delete those in an upcoming PR with the rest of the old UI setup 👍
brunobar79
approved these changes
Dec 7, 2018
There was a problem hiding this comment.
Looks good! Thanks for doing this @whymarrh Just got a question about what to do with some leftover files, but if you wanna delete those you can do it on a separate PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4751
This PR updates our test deps CI job to use npm audit instead of nsp.
See also:
npm auditcommandReview:
A lot of the changes are lockfile changes. Outside of that file the commits should be self-explanatory. I've updated the
package.jsonfile as well to de-duplicate dependencies and remove deps that are no longer needed.