Run lint and unit tests on Buildkite #17
Conversation
mokagio
force-pushed
the
mokagio/migrate-gh-action-to-buildkite
branch
8 times, most recently
from
2b6c499
to
81a258b
Compare
| LOCAL_NPM_CACHE=./vendor/npm | ||
| mkdir -p $LOCAL_NPM_CACHE | ||
| echo "--- :npm: Set npm to use $LOCAL_NPM_CACHE for cache" | ||
| npm set cache $LOCAL_NPM_CACHE | ||
| echo "npm cache set to $(npm get cache)" |
There was a problem hiding this comment.
In setting this up, it seems like using a local cache works better than the default ($HOME/.npm).
I haven't gone back and did a fair comparison, but I got the impression that the _cacache subfolder was not extracted when using the local cache.
There was a problem hiding this comment.
I am no npm expert so I don't know all the available commands nor how it handles cache, nor what npm ci does that npm install wouldn't, but couldn't our install_npm_dependencies helper from ci-toolkit be of help here (instead of reinventing the wheel)?
And if not, could we maybe extend a8c-ci-toolkit to bring the things from this script that are not yet supported by our helper to be supported (either by the existing install_npm_packages helper, or by a new one)? 🤔
mokagio
changed the title
| LOCAL_NPM_CACHE=./vendor/npm | ||
| mkdir -p $LOCAL_NPM_CACHE | ||
| echo "--- :npm: Set npm to use $LOCAL_NPM_CACHE for cache" | ||
| npm set cache $LOCAL_NPM_CACHE | ||
| echo "npm cache set to $(npm get cache)" |
There was a problem hiding this comment.
I am no npm expert so I don't know all the available commands nor how it handles cache, nor what npm ci does that npm install wouldn't, but couldn't our install_npm_dependencies helper from ci-toolkit be of help here (instead of reinventing the wheel)?
And if not, could we maybe extend a8c-ci-toolkit to bring the things from this script that are not yet supported by our helper to be supported (either by the existing install_npm_packages helper, or by a new one)? 🤔
| npm ci \ | ||
| --unsafe-perm \ | ||
| --prefer-offline \ | ||
| --no-audit \ | ||
| --no-progress \ | ||
| --maxsockets "$MAX_SOCKETS" \ | ||
| "$@" | ||
|
|
||
| echo "--- :npm: Save cache if necessary" | ||
| # Notice that we don't cache the local node_modules. | ||
| # Those get regenerated by npm ci as per Node reccomendations. | ||
| # What we are caching is the root npm folder, which stores pacakge downloads so they are available if the package.json resolution demands them. |
There was a problem hiding this comment.
This is very interesting information. I wonder why we didn't go that route in https://github.com/Automattic/a8c-ci-toolkit-buildkite-plugin/blob/trunk/bin/install_npm_packages; might be worth updating that helper to make other apps using npm benefit from this?
mokagio
force-pushed
the
mokagio/migrate-gh-action-to-buildkite
branch
from
ec49b5b
to
497e700
Compare
Proposed Changes
As discussed internally, we should have a single CI system. Given we need Buildkite to code sign builds, Buildkite will be that system.
This PR duplicates the lint and unit tests steps to Buildkite. I didn't remove the GitHub actions one because it would be interesting to run the in tandem for a couple of days to see if there is any difference in behavior.
Testing Instructions
See https://buildkite.com/automattic/studio/builds/81 and notice it's green 😄
Pre-merge Checklist