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

Fixes to PR 6007 #6207

Merged
merged 3 commits into from
Aug 3, 2018
Merged

Fixes to PR 6007 #6207

merged 3 commits into from
Aug 3, 2018

Conversation

clintwood
Copy link
Contributor

Summary

As a result of changes introduced in PR #6007 this PR resolves #6174 and provides a different implementation to #6181.

Test plan

All existing tests pass, PR #6007 added a number of tests but it seems the case of yarn global ... with scoped registries was not tested. I cannot identify exactly how or where these tests should be done. I'd be happy to do them if I'm pointed in the right direction.

@arcanis
Copy link
Member

arcanis commented Aug 2, 2018

Thanks! I think testing this would imply:

  • create a new fixture in __tests__/fixtures/global/ that would contain a yarnrc and an empty pkg.json
  • add a test in __tests__/commands/global.js that would run yarn global add with a scoped package
  • check that it works as expected (not sure exactly how to do this part, I guess you can try setting an invalid registry for the scope and expect for the install to fail?)

@clintwood
Copy link
Contributor Author

@arcanis, I have the basis of a test but cannot figure out how to generate and populate ./__tests__/fixtures/request-cache/GET/test-scope-registry.com/... with the cache responses to my test scoped domain. Some guidance from other contributors on how to do this would be appreciated!

@clintwood
Copy link
Contributor Author

@arcanis - I've added a test for yarn global add @test-scope/scoped-module in
__tests__/commands/global.js with supporting fixtures
__tests__/fixtures/global/add-with-scoped-registry and
__tests__/fixtures/request-cache/GET/test-scope-registry.com.

I think this concludes the PR.

@arcanis arcanis merged commit 1af4c5f into yarnpkg:master Aug 3, 2018
@arcanis
Copy link
Member

arcanis commented Aug 3, 2018

Perfect, thanks!

@clintwood clintwood deleted the fix-pr-6007 branch August 3, 2018 12:44
@clintwood clintwood restored the fix-pr-6007 branch August 3, 2018 13:04
arcanis pushed a commit that referenced this pull request Aug 3, 2018
* fix: ensure enableDefaultRc defaults to true unless exlicitly set to false via --no-default-rc

* fix: ensure enableDefaultRc and extraneousYarnrcFiles are passed to config.init(...)

* add: added test for yarn global add for module using a scoped registry
This was referenced Aug 3, 2018
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.

1.9.2: Scoped Registries are being ignored
2 participants