install: failing to parse package.json should be error #12406

Closed
wants to merge 2 commits into
from

Projects

None yet

4 participants

@watilde
Contributor
watilde commented Apr 19, 2016 edited

Fixes #12155. We can stop install process when the EJSONPARSE error happened, because the process never finish if npm can't parse package.json.

@othiym23 othiym23 added the review label Apr 19, 2016
@zkat zkat added merge-to-latest and removed review labels Apr 21, 2016
@iarna iarna added this to the next milestone Apr 21, 2016
@iarna iarna added a commit that referenced this pull request Apr 21, 2016
@watilde @iarna watilde + iarna install: failing to parse package.json should be error
Credit: @watilde
Reviewed-By: @iarna
PR-URL: #12406
2010d18
@iarna
Member
iarna commented Apr 21, 2016 edited

I'm merging this as 2010d18. I did two things:

  1. I squashed the test and change commits–I like to see patches in the same commit as the patch they support. Having them together makes later searching through the commit log easier because if git blame leads me to a commit id, git show will show me everything relevant.
  2. I rewrote the test to match our newer house style. We haven't documented this much anywhere, but I encourage you to take a look, the primary changes are using tacks to setup and cleanup our test case and throwing if the err arg from common.npm exists. We do the latter because that is only set if common.npm couldn't exec npm for some reason, usually an OS problem. It's never an error in the software being tested, but a problem with the machine running the tests.
@iarna iarna added a commit that referenced this pull request Apr 21, 2016
@watilde @iarna watilde + iarna install: failing to parse package.json should be error
Credit: @watilde
Reviewed-By: @iarna
PR-URL: #12406
769e620
@watilde
Contributor
watilde commented Apr 23, 2016

Thanks for letting me know them! The newer house style seems neat. Will try to read some the other tests and use it next time.

@iarna
Member
iarna commented Apr 25, 2016

Merged into v3.8.8!

@iarna iarna closed this Apr 25, 2016
@watilde watilde deleted the watilde:patch/fix-error-prefix branch Apr 25, 2016
@segrey segrey added a commit to segrey/npm that referenced this pull request Jun 21, 2016
@watilde @segrey watilde + segrey install: failing to parse package.json should be error
Credit: @watilde
Reviewed-By: @iarna
PR-URL: npm#12406
864532f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment