KO-22267 upgrade to nodejs24 and modernise the build pipeline and tests#36
KO-22267 upgrade to nodejs24 and modernise the build pipeline and tests#36ls-simon-he wants to merge 3 commits intomasterfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| language: node_js | ||
| node_js: | ||
| - 0.12 | ||
| - 24 |
There was a problem hiding this comment.
we can remove this config file, since we're not using travis anymore
There was a problem hiding this comment.
possibly .eslintrc as well? It was removed on devDeps/GruntFile
There was a problem hiding this comment.
Good spot, done.
| @@ -1,106 +1,71 @@ | |||
| // Karma configuration | |||
| // Generated on Fri Feb 26 2016 11:48:37 GMT+1100 (AUS Eastern Summer Time) | |||
| process.env.CHROME_BIN = process.env.CHROME_BIN || require('puppeteer').executablePath(); | |||
There was a problem hiding this comment.
I think we can do it below.
if (!process.env.CHROME_BIN) {
try {
process.env.CHROME_BIN = require('puppeteer').executablePath();
} catch (err) {
console.warn('[karma] Puppeteer Chrome not available, falling back to system Chrome:', err.message);
// leave CHROME_BIN unset; karma-chrome-launcher will try `google-chrome`/`chromium` on PATH
}
}
2 potential issues:
- with the current statement, it short-circuits only on truthy CHROME_BIN, not on "Puppeteer is unusable". The || only skips the Puppeteer call when the env var is already set. If CHROME_BIN is empty, Puppeteer is required regardless of whether its Chrome download exists.
- executablePath() throws when the download was skipped. Puppeteer 21+ uses @puppeteer/browsers and downloads a pinned Chrome for Testing build to ~/.cache/puppeteer during npm install's postinstall.
| @@ -8,23 +8,25 @@ jobs: | |||
| build: | |||
There was a problem hiding this comment.
we might need to add a PR-time test run as well, for additional checks?
Add a pull_request: trigger that runs npm ci && npm test (CI=true) before this merges. Everything below is untested until that happens.
There was a problem hiding this comment.
Done, see the check: Node.js Package / build (pull_request)
|
one more thing - the .nvmrc is still set to 5.7.0, which is inconsistent with the update. We might need to bump it to 24 |
Remove .travis.yml and .eslintrc (both replaced/dropped in this upgrade), bump .nvmrc to 24 to match engines, harden the Puppeteer fallback in karma.conf.js so a missing bundled Chrome degrades to system Chrome instead of throwing, and add a pull_request trigger to the workflow so build+test runs on PRs (publish stays release-only). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Done |
3 participants
KO-22267
Notes: this doesn't change the source code which is the only file src/db.js