Skip to content

Commit

Permalink
Merge pull request #5012 from kobotoolbox/node-20-upgrade-CI-testing
Browse files Browse the repository at this point in the history
Use Node 20, and add it to CI
  • Loading branch information
jnm authored Aug 30, 2024
2 parents f9d55ba + f587a1e commit d235ce1
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 48 deletions.
57 changes: 44 additions & 13 deletions .github/workflows/npm-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,38 +8,69 @@ on:
jobs:
build:
runs-on: ubuntu-20.04

strategy:

matrix:
node-version:
- '16.15.0' # prior pinned Node version supported by kpi
- '20.17.0' # version pinned for kpi release
- '20' # latest available v20
- '22' # latest available v22
fail-fast: false # Let each job finish

steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
# We need this particular version, as npm 8.5.5 is the last version
# that works with our package.json :sadface:.
node-version: '16.15.0'
node-version: ${{ matrix.node-version }}
check-latest: true # download newer semver match if available
cache: 'npm'

- name: Identify resolved node version
id: resolved-node-version
run: echo "NODE_VERSION=$(node --version)" >> "$GITHUB_OUTPUT"
- name: Add "Node ${{ steps.resolved-node-version.outputs.NODE_VERSION }}" to summary
run: echo "${{ matrix.node-version }} → **${{ steps.resolved-node-version.outputs.NODE_VERSION }}**" >> "$GITHUB_STEP_SUMMARY"

# Set up Chrome, for the unit tests
- uses: browser-actions/setup-chrome@latest
- run: chrome --version
- name: Check for cached node_modules

# Cache node_modules, keyed on os, node version, package-lock, and patches
- uses: actions/cache@v4
name: Check for cached node_modules
id: cache-nodemodules
uses: actions/cache@v3
env:
cache-name: cache-node-modules
with:
path: node_modules
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json', 'patches/**/*.patch') }}
- name: Install JavaScript dependencies (npm install)
if: steps.cache-nodemodules.outputs.cache-hit != 'true'
run: npm install
- name: Run copy-fonts (if using cached node_modules)
if: steps.cache-nodemodules.outputs.cache-hit == 'true'
key: ${{ runner.os }}-build-${{ env.cache-name }}-node-v${{ steps.resolved-node-version.outputs.NODE_VERSION }}-${{ hashFiles('**/package-lock.json', 'patches/**/*.patch') }}

# Cache hit: node_modules is copied from a previous run. Run copy-fonts
- if: steps.cache-nodemodules.outputs.cache-hit == 'true'
name: Run copy-fonts (if using cached node_modules)
run: npm run copy-fonts

# Cache miss: Run npm install, which does copy-fonts as post-install step
- if: steps.cache-nodemodules.outputs.cache-hit != 'true'
name: Install JavaScript dependencies (npm install)
run: npm install

# Build the app!
- name: Build Prod
run: SKIP_TS_CHECK=true npm run build

# Run TypeScript Checks and ESLint
- name: Check TypeScript # Separated for visibility
run: npm run check-types
- name: Check ESLint, errors only
run: npm run lint -- --quiet

# Unit Tests
- name: Build Tests
run: npx webpack --config webpack/test.config.js

- name: Run Tests, with mocha-chrome
run: npx mocha-chrome test/tests.html --chrome-launcher.connectionPollInterval=5000
# This step takes less than 1 minute if it succeeds, but will hang for
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pytest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ jobs:
ports:
- 6379:6379
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Install pip-tools
Expand Down
2 changes: 1 addition & 1 deletion .node-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v16.15.0
v20.17.0
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v16.15.0
v20.17.0
14 changes: 5 additions & 9 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,8 @@ RUN mkdir -p "${NGINX_STATIC_DIR}" && \
# jnm (or the current on-call sysadmin). Thanks.

RUN apt-get -qq update && \
apt-get -qq -y install ca-certificates curl gnupg && \
mkdir -p /etc/apt/keyrings && \
curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key \
| gpg --dearmor -o /etc/apt/keyrings/nodesource.gpg && \
echo "deb [signed-by=/etc/apt/keyrings/nodesource.gpg] https://deb.nodesource.com/node_16.x nodistro main" \
| tee /etc/apt/sources.list.d/nodesource.list && \
apt-get -qq update && \
apt-get -qq -y install curl && \
curl -fsSL https://deb.nodesource.com/setup_20.x | bash - && \
apt-get -qq -y install openjdk-17-jre && \
apt-get -qq -y install --no-install-recommends \
ffmpeg \
Expand All @@ -65,7 +60,8 @@ RUN apt-get -qq update && \
less \
libproj-dev \
locales \
nodejs=$(apt-cache show nodejs | grep -F 'Version: 16.15.0' | cut -f 2 -d ' ') \
# pin an exact Node version for stability. update this regularly.
nodejs=$(apt-cache show nodejs | grep -F 'Version: 20.17.0' | cut -f 2 -d ' ') \
postgresql-client \
procps \
rsync \
Expand Down Expand Up @@ -109,7 +105,7 @@ WORKDIR ${KPI_SRC_DIR}/
RUN rm -rf ${KPI_NODE_PATH} && \
mkdir -p "${TMP_DIR}/.npm" && \
npm config set cache "${TMP_DIR}/.npm" --global && \
npm install -g npm@8.5.5 && \
# to pin an exact npm version, see 'git blame' here
npm install -g check-dependencies@1 && \
rm -rf "${KPI_SRC_DIR}/jsapp/fonts" && \
rm -rf "${KPI_SRC_DIR}/jsapp/compiled" && \
Expand Down
3 changes: 1 addition & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
"version": "2.0.0",
"description": "KoboToolbox frontend interface.",
"engines": {
"node": "16.15.0",
"npm": "8.5.5"
"node": "^20.17.0 || ^22.4.1"
},
"dependencies": {
"@fontsource/roboto": "^4.4.2",
Expand Down
13 changes: 13 additions & 0 deletions patches/mocha-chrome+2.2.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/node_modules/mocha-chrome/lib/client.js b/node_modules/mocha-chrome/lib/client.js
index 7629126..cd3d163 100644
--- a/node_modules/mocha-chrome/lib/client.js
+++ b/node_modules/mocha-chrome/lib/client.js
@@ -9,7 +9,7 @@ module.exports = async function connectClient(instance, log, options) {
return fs.readFileSync(filePath, 'utf-8');
}

- const client = await CDP({ port: instance.port });
+ const client = await CDP({ port: instance.port, host: '127.0.0.1' });
const { DOM, DOMStorage, Console, Network, Page, Runtime } = client;
const mochaOptions = `window.mochaOptions = ${JSON.stringify(options.mocha)}`;

38 changes: 20 additions & 18 deletions scripts/hints.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,18 @@ if (process.env.SKIP_TS_CHECK && tsCheckAffects.includes(hintName)) {
/*
NPM VERSION WARNING
Issue a warning if not running with npm 8.5.5, since the errors
that you get if you run a different version are not very obvious.
Issue a warning if there's a version mismatch between expected and user's
version of Node or npm, since it could cause strange difficulties.
- Help contributors who switched their Node version inadvertently.
- Support new contributors by printing more context / steps to remedy.
- Give the benefit of the doubt to developers who run a different node/npm
version on purpose. So, don't process.exit(1), even on npm install.
It is a bit redundant with the EBADENGINE warning issued from package.json,
but this script (originally written for a bumpy >=16.15.1 upgrade) provides
a bit of context.
Show on preinstall. Since it's easy to miss there, also show it on other
run scripts such as 'watch'.
*/
const ok_node = 'v16.15.0';
const ok_npm = '8.5.5';
const ok_node = 'v20.17.0';
const ok_npm = '10.8.2';

if (process.version !== ok_node) {
const blu = '\u001b[94m'; // bright blue
Expand All @@ -79,10 +78,10 @@ if (process.version !== ok_node) {
console.warn(`${blu}
--------------------------------------------------------------`);

console.warn(`${yel}
Are you running the supported version of Node and npm?
console.warn(`${nrm}
Are you running a supported version of Node and npm?
${nrm}
node ${ok_node}, npm@${ok_npm} supported`);
node ${yel}${ok_node}${nrm}, ${yel}npm@${ok_npm}${nrm} supported`);

// Let's be more helpful by running `npm --version` instead of making
// you do it.
Expand Down Expand Up @@ -110,21 +109,24 @@ if (process.version !== ok_node) {
// what causes the most problems.
if (wrongNpm) {
console.warn(`
Switch to a supported Node / npm version:
${blu}(This is probably OK, but it's helpful to
run the same version we're using in release.)${nrm}
Use Node v16.15.0, which comes with [email protected]
To switch to a supported Node / npm version:
Use Node ${ok_node}, which comes with npm@${ok_npm}
\`nvm use\` or \`fnm use\`
or \`npm install -g npm@8.5.5\`
or \`npm install -g npm@${ok_npm}\`
to change npm for your current Node`);

console.warn(`${red}
If you've run \`npm install\` with an unsupported npm version,${nrm}
there may be changes in node_modules and package-lock.json
console.warn(`${yel}
If you've run \`npm install\` with a different npm version,${nrm}
there could be changes in node_modules and package-lock.json
(1) Don't commit these changes to package-lock.json
(2) You may want to reset these changes and run
\`npm install\` again with 8.5.5
\`npm install\` again with ${ok_npm}
`);

// If you switch between Node projects and see this message often,
Expand Down

0 comments on commit d235ce1

Please sign in to comment.