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

Add "Too many arguments" message to info and improve code coverage #2235

Merged
merged 1 commit into from
Dec 13, 2016

Conversation

skratchdot
Copy link
Contributor

I'm working on some code, and rather than including all my commits in one PR, I'd like to separate a few of the smaller, non-related fixes into their own PR's (for easier review).

Summary

This is a very small change to get the code coverage of the info command up to 100%. The currently included versions of jest and babel-jest still have slightly incorrect coverage reports, but updating them gives 100% on this file. I would like to upgrade all the modules in this repo, but it might make more sense for that to be in a different PR? (I didn't include package.json or yarn.lock updates in this PR)

Test plan

  1. Run yarn test
  2. Confirm __tests__/commands/info.js has no errors
  3. open ./coverage/lcov-report/index.html
  4. Confirm that the coverage for src/commands/info.js has improved

@@ -47,7 +48,7 @@ const expectedKeys = [
];

test.concurrent('without arguments and in directory containing a valid package file', (): Promise<void> => {
return runInfo(['.'], {}, 'local',
return runInfo([], {}, 'local',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: this looks like I'm removing the ['.'] case, but there were actually 2 of the exact same test cases in this file. All I'm doing here is making this test case match the description (and hereby improving the coverage report).

@wyze wyze merged commit b1166d7 into yarnpkg:master Dec 13, 2016
@wyze
Copy link
Member

wyze commented Dec 13, 2016

Thanks!

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.

2 participants