fix(cli): fail when yarn adding Github package with no version (#5292)
#5318
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
|
@bestander please review at your convenience. It looks like most of the testing for these helper libraries is done through issuing actual cli commands, so hopefully this test coverage is sufficient. Thanks for all your help! |
mhchen
commented
Feb 5, 2018
| @@ -473,7 +473,7 @@ export default class Git implements GitRefResolvingInterface { | |||
| }); | |||
| if (!resolvedResult) { | |||
| throw new MessageError( | |||
| this.reporter.lang('couldntFindMatch', version, Object.keys(refs).join(','), this.gitUrl.repository), | |||
| this.reporter.lang('couldntFindMatch', version, Array.from(refs.keys()).join(','), this.gitUrl.repository), | |||
There was a problem hiding this comment.
refs is a Map which always returns an empty array with Object.keys()
mhchen
commented
Feb 5, 2018
| } catch (err) { | ||
| message = err.message; | ||
| } | ||
| expect(message).toEqual(expect.stringContaining('invalid package version')); |
There was a problem hiding this comment.
Would be preferable to use .rejects and assert on the error message using toThrow or the like, but it looks like this is still problematic: jestjs/jest#3601
|
Thank you @mhchen ! |
|
@mhchen great work! Thank you for following it through and doing tests. |
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.
Summary
Running
yarn addto install from a Github repo that has a package.json with no version succeeds, but then all furtheryarnoperations fail due to failing checks forversions in dependencies. As well, per npm, the install method should not succeed at all. This PR causesyarn addto fail ifversionis not properly formatted as a semver.Fixes #5292
Test plan
Running:
Produces the following output: