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 tests for global command #3238

Merged
merged 7 commits into from
May 2, 2017

Conversation

voxsim
Copy link
Contributor

@voxsim voxsim commented Apr 23, 2017

This pull request adds:

cc @bestander @arcanis

@voxsim
Copy link
Contributor Author

voxsim commented Apr 24, 2017

IMO we should remove reporter and install from checkInstalled function because they were never used. I will try to open another pull-request to see if I am right O.o

@voxsim
Copy link
Contributor Author

voxsim commented May 1, 2017

@bestander if you don't have time to review this PR right now, I can open a mini PR that fixes for #3142. yarn global ls does not work because we pass the flag skipIntegrity instead of skipIntegrityCheck.

@voxsim voxsim force-pushed the add-tests-for-global-command branch from 0d0f3ed to 2040243 Compare May 1, 2017 16:29
@arcanis
Copy link
Member

arcanis commented May 1, 2017

@voxsim There's a test that fails on Windows, can you give it a look?

 FAIL  __tests__\commands\global.js (16.563s)
  ● bin
    Error: expect(string).toContain(value)
    
    Expected string:
      "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\yarn-global-0.40320568132368995
    "
    To contain value:
      "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\yarn-global-0.40320568132368995\\bin"
      
      at runGlobal (__tests__\commands\global.js:80:25)
      at __tests__\commands\_helpers.js:135:15
      at next (native)
      at step (node_modules\babel-runtime\helpers\asyncToGenerator.js:17:30)
      at node_modules\babel-runtime\helpers\asyncToGenerator.js:28:13
      Console output:
       C:\Users\appveyor\AppData\Local\Temp\1\yarn-global-0.40320568132368995
      
      at __tests__\commands\_helpers.js:138:13
      at next (native)
      at step (node_modules\babel-runtime\helpers\asyncToGenerator.js:17:30)
      at node_modules\babel-runtime\helpers\asyncToGenerator.js:28:13

@TimvdLippe
Copy link
Contributor

I assume this also closes #2725?

@voxsim
Copy link
Contributor Author

voxsim commented May 1, 2017

@arcanis ok I will look this tomorrow ;)

@TimvdLippe yes it should close that pull request, but please check it, ok?

@TimvdLippe
Copy link
Contributor

@voxsim Sure, will do tomorrow 😄

@voxsim voxsim force-pushed the add-tests-for-global-command branch from 2040243 to e66c593 Compare May 2, 2017 06:48
@voxsim
Copy link
Contributor Author

voxsim commented May 2, 2017

@arcanis now should work ;)

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

Can confirm it actually runs on my machine now, finally a green build! Not sure if isCI is the nicest solution, but at least it works for now. Could you add Closes #2725 to the pull request description?

@voxsim
Copy link
Contributor Author

voxsim commented May 2, 2017

@TimvdLippe done ;)

@voxsim
Copy link
Contributor Author

voxsim commented May 2, 2017

@TimvdLippe I didn't delete that tests because I have no idea how to prove that tests are useful or not. For what I understand we can delete without worries.

@arcanis
Copy link
Member

arcanis commented May 2, 2017

That's great, thanks! Let's merge it! :)

@arcanis arcanis merged commit 4731641 into yarnpkg:master May 2, 2017
@DanBuild DanBuild mentioned this pull request May 2, 2017
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.

Global packages unavailable
3 participants