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

Ci action performance #2200

Merged
merged 3 commits into from
Jan 21, 2020
Merged

Ci action performance #2200

merged 3 commits into from
Jan 21, 2020

Conversation

mantoni
Copy link
Member

@mantoni mantoni commented Jan 21, 2020

Purpose (TL;DR) - mandatory

Improve build performance by configuring npm dependency caching and skipping the unused Chromium download.

Background (Problem in detail) - optional

The build is currently downloading all dependencies and an unused Chromium binary on every execution. This PR should save some resources and speed up the build.

How to verify - mandatory

Inspect the GitHub actions log for this branch at https://github.com/sinonjs/sinon/runs/400526914 and find these messages in the logs:

  • In Cache modules: Cache Size: ~14 MB (14936242 B) and Cache restored from key: Linux-node-39f0627 ...
  • In npm ci: **INFO** Skipping Chromium download. "PUPPETEER_SKIP_CHROMIUM_DOWNLOAD" environment variable was found.

Checklist for author

  • npm run lint passes
  • References to standard library functions are cached.

@mantoni mantoni force-pushed the ci-action-performance branch 3 times, most recently from 1dd91b4 to 1b983e7 Compare January 21, 2020 12:54
@mantoni
Copy link
Member Author

mantoni commented Jan 21, 2020

Hmmm, the last build step fails with

(node:3093) UnhandledPromiseRejectionWarning: Error: Chromium revision is not downloaded. Run "npm install" or "yarn install"
82
    at Launcher.launch (/home/runner/work/sinon/sinon/node_modules/puppeteer/lib/Launcher.js:119:15)

Apparently I'm doing something wrong with setting SINON_CHROME_BIN to the chrome executable. Please help!

@fatso83
Copy link
Contributor

fatso83 commented Jan 21, 2020

Apparently I'm doing something wrong with setting SINON_CHROME_BIN to the chrome executable. Please help!

I am not totally sure what is going wrong, but I do know you are trying to set the env variable in the wrong place, as it's not that npm script which is failing.

Looking at the build log, something (pretest-webworker?)is triggering the build (npm run build) and it's the postbuild that is triggering the test-esm-bundle.

@fatso83
Copy link
Contributor

fatso83 commented Jan 21, 2020

Yup. Pretty sure I found it. It's test-webworker that breaks. Just set the environment variable before stuff that triggers the build:

        export SINON_CHROME_BIN=$(which google-chrome-stable)
        npm run test-headless -- --chrome $SINON_CHROME_BIN --allow-chrome-as-root
        npm run test-webworker -- --chrome $SINON_CHROME_BIN --allow-chrome-as-root
        npm run test-esm-bundle

@mantoni
Copy link
Member Author

mantoni commented Jan 21, 2020

Awesome! That fixed it 👍 🙏

@fatso83 fatso83 merged commit 9f59015 into master Jan 21, 2020
@fatso83 fatso83 deleted the ci-action-performance branch January 21, 2020 14:20
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.

None yet

2 participants