Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Speed up integration / namespace tests #148

Merged
merged 0 commits into from
May 24, 2017
Merged

Speed up integration / namespace tests #148

merged 0 commits into from
May 24, 2017

Conversation

ransombriggs
Copy link
Contributor

@dickeyxxx could you review? This brings the test suite runtime down to about 20 seconds rather than about 40 seconds and make it possible to run offline. In order to get this done I made the following changes

  1. add a yarn-offline-mirror "./npm-packages-offline-cache" to .yarnrc and add three local dependencies, one for each version of heroku-debug that the tests install. This way, it can download the versions to npm-packages-offline-cache without having to hit the network
  2. change latest, alpha and cowabunga references in tests to be the versions that they currently resolve to so that it does not have to check dist tags and can just use the caches
  3. hack --prefer-offline in yarn.js during tests so that it checks local cache first
  4. isolate tests using tmp dirs for cache and data dirs which allows us to turn off the test locking and run in band without having issues of yarn writing to the same directories

@codecov
Copy link

codecov bot commented May 22, 2017

Codecov Report

Merging #148 into master will increase coverage by 0.76%.
The diff coverage is 95.23%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #148      +/-   ##
=========================================
+ Coverage   76.74%   77.5%   +0.76%     
=========================================
  Files          30      30              
  Lines         890     907      +17     
  Branches      183     184       +1     
=========================================
+ Hits          683     703      +20     
+ Misses        184     182       -2     
+ Partials       23      22       -1
Impacted Files Coverage Δ
src/plugins/yarn.js 93.93% <100%> (ø) ⬆️
test/helpers.js 95% <95%> (-5%) ⬇️
src/namespaces.js 91.89% <0%> (+2.7%) ⬆️
src/plugins/cache.js 97.87% <0%> (+6.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cdbb37...a74137c. Read the comment docs.

@jdx
Copy link
Contributor

jdx commented May 22, 2017

👏

package.json Outdated
@@ -78,7 +81,7 @@
"download": "node ./bin/download-yarn",
"prepare": "npm run clean && npm run build && npm run download",
"release": "np",
"test": "npm run download && jest -i && flow && standard",
"test": "npm run download && jest && flow && standard",
Copy link
Contributor

Choose a reason for hiding this comment

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

they still should be run in series though right? If the tests are doing things like installing and uninstalling plugins we can't be running things in parallel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dickeyxxx I have not run into any issues with this because it is installing to different directories, though there may be some issues with the yarn cache apparently. I could add --mutex to the yarn options to isolate this at the yarn level. Thoughts?

@@ -2,5 +2,6 @@ coverage
lib
/node_modules
/test/links/test-foo/node_modules
/npm-packages-offline-cache
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add this to the circle cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dickeyxxx added in a639907

@jdx
Copy link
Contributor

jdx commented May 22, 2017

seems appveyor is failing

@ransombriggs
Copy link
Contributor Author

@dickeyxxx as always, windows eats out faces because of file locking, not sure if there is a good way around this as the code in the tmp dir is being requireed by the tests so that might be what is making them appear as still in use. Any ideas?

Should I just file a different PR that only has the local cache changes for now?

@ransombriggs
Copy link
Contributor Author

@dickeyxxx could you look at this again? I pushed up daa37a3 to fix appveyor

@ransombriggs ransombriggs merged commit 555bb2d into master May 24, 2017
@ransombriggs ransombriggs deleted the speed-up-tests branch May 24, 2017 20:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants