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.

Sign up for GitHub

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

Jump to bottom

build: Update (internal) node version to 16.x #6236

Merged
merged 3 commits into from Nov 21, 2022
Merged

build: Update (internal) node version to 16.x #6236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9d2e65d
build: bump internal node version from 14.x to 16.x
mydea Nov 18, 2022
e0d0293
ci: Use node 16 to build on CI
mydea Nov 18, 2022
a98d932
ci: Make CI use volta to install dependencies
mydea Nov 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 15 additions & 35 deletions .github/workflows/build.yml
View file
Open in desktop
Expand Up @@ -18,9 +18,6 @@ concurrency:
cancel-in-progress: true

env:
# We pin the exact version to enforce reproducable builds with node + npm.
DEFAULT_NODE_VERSION: '16.15.1'

HEAD_COMMIT: ${{ github.event.inputs.commit || github.sha }}

CACHED_DEPENDENCY_PATHS: |
Expand Down Expand Up @@ -132,9 +129,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
uses: volta-cli/action@v4
# we use a hash of yarn.lock as our cache key, because if it hasn't changed, our dependencies haven't changed,
# so no need to reinstall them
- name: Compute dependency cache key
Expand Down Expand Up @@ -163,10 +158,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
# ember won't build under node 16, at least not with the versions of the ember build tools we use
node-version: '14'
uses: volta-cli/action@v4
- name: Check dependency cache
uses: actions/cache@v3
with:
Expand Down Expand Up @@ -202,9 +194,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
uses: volta-cli/action@v4
- name: Check dependency cache
uses: actions/cache@v3
with:
Expand Down Expand Up @@ -282,9 +272,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
uses: volta-cli/action@v4
- name: Check dependency cache
uses: actions/cache@v3
with:
Expand All @@ -309,9 +297,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
uses: volta-cli/action@v4
- name: Check dependency cache
uses: actions/cache@v3
with:
Expand All @@ -337,9 +323,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
uses: volta-cli/action@v4
- name: Check dependency cache
uses: actions/cache@v3
with:
Expand Down Expand Up @@ -509,9 +493,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
uses: volta-cli/action@v4
- name: Check dependency cache
uses: actions/cache@v3
with:
Expand Down Expand Up @@ -550,9 +532,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
uses: volta-cli/action@v4
- name: Check dependency cache
uses: actions/cache@v3
with:
Expand Down Expand Up @@ -582,9 +562,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
uses: volta-cli/action@v4
- name: Check dependency cache
uses: actions/cache@v3
with:
Expand Down Expand Up @@ -689,9 +667,7 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
uses: volta-cli/action@v4
- name: Check dependency cache
uses: actions/cache@v3
with:
Expand All @@ -702,9 +678,13 @@ jobs:
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}
- name: Get node version
id: versions
run: |
echo "echo node=$(jq -r '.volta.node' package.json)" >> $GITHUB_OUTPUT
- name: Run E2E tests
env:
E2E_TEST_PUBLISH_SCRIPT_NODE_VERSION: ${{ env.DEFAULT_NODE_VERSION }}
E2E_TEST_PUBLISH_SCRIPT_NODE_VERSION: ${{ steps.versions.outputs.node }}
E2E_TEST_AUTH_TOKEN: ${{ secrets.E2E_TEST_AUTH_TOKEN }}
E2E_TEST_DSN: ${{ secrets.E2E_TEST_DSN }}
run: |
Expand Down
11 changes: 6 additions & 5 deletions .github/workflows/canary.yml
View file
Open in desktop
Expand Up @@ -27,17 +27,18 @@ jobs:
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v3
with:
# ember won't build under node 16, at least not with the versions of the ember build tools we use
node-version: 14
uses: volta-cli/action@v4
- name: Install dependencies
run: yarn install --ignore-engines --frozen-lockfile
- name: Build packages
run: yarn build
- name: Get node version
id: versions
run: |
echo "echo node=$(jq -r '.volta.node' package.json)" >> $GITHUB_OUTPUT
- name: Run Canary Tests
env:
E2E_TEST_PUBLISH_SCRIPT_NODE_VERSION: ${{ env.DEFAULT_NODE_VERSION }}
E2E_TEST_PUBLISH_SCRIPT_NODE_VERSION: ${{ steps.versions.outputs.node }}
Copy link
Member Author

Choose a reason for hiding this comment

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

@lforst Do we actually need to set this, when it is the default installed node version? 🤔 Just wondering, because in the canary tests it was actually also setting it to env.DEFAULT_NODE_VERSION, which isn't actually set there 😅 (same applies then to the non-canary tests I guess)

Copy link
Member

Choose a reason for hiding this comment

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

I can't speak to the end-to-end tests, but as for the others, GH recommends always being explicit, so I think it's not a bad thing to do.

Copy link
Member

Choose a reason for hiding this comment

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

We use this env variable to set what node version we use to fake publish our packages in the E2E tests. The version should always align with the version we actually publish our packages with. If in the future we want to do our E2E tests with multiple node versions this will be necessary. Here is how this came to be: #5806 (comment) Looking back at it now, I probably wouldn't do it again this way. Maybe we can simplify.

As for the canary tests: Looks like I forgot to copy the env var over. (Which wasn't too bad since right now we only tested with the default node version anyways but good that you caught it.)

Copy link
Member Author

Choose a reason for hiding this comment

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

makes sense. So we can leave this in (this parses the node version defined for volta, so it is still consistent, but we can change it later if needed)!

E2E_TEST_AUTH_TOKEN: ${{ secrets.E2E_TEST_AUTH_TOKEN }}
E2E_TEST_DSN: ${{ secrets.E2E_TEST_DSN }}
CANARY_E2E_TEST: 'yes'
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Expand Up @@ -28,7 +28,7 @@
"test-ci": "ts-node ./scripts/test.ts"
},
"volta": {
"node": "14.17.0",
"node": "16.18.1",
"yarn": "1.22.19"
},
"workspaces": [
Expand Down