Skip to content

Commit

Permalink
fix: Delete obsolete platform-compatibility-test (#2419)
Browse files Browse the repository at this point in the history
Closes: #2418 
Refs: #2417 #1308 

## Description

Adapted from
#2419 (review)
below

The platform compatibility test specifically validates that SES works on
Node.js 12 and can be deleted since it has vanished into history.
Node.js 12 required special consideration because of its experimental
ESM support. Delete the `test:platform-compatibility` script in the ses
`package.json`, as well as the `test/package` fixture in ci.yml

Immediate motivation explained in #2418 , #2417 broken the
platform-compatibility-test tests because it depends on the platform
providing either `Array.prototype.transfer` or `structuredClone`. Node
supports `transfer` starting with Node 22, but supports
`structuredClone` starting in Node 18. That should be fine, since that
is our declared support floor. This PR changes our one known remaining
dependence on Node 12.

### Security Considerations

none
### Scaling Considerations

none
### Documentation Considerations

none beyond the need to explain our platform requirements, which this PR
does not change.
### Testing Considerations

The point. This PR only affects tests, not production code.

Reviewers, should this PR be labeled `test:` instead of `fix:`?

### Compatibility and Upgrade Considerations

After this PR, we will no longer notice further breakage on Node < 18.
That fits with our declared support floor.
  • Loading branch information
erights authored Aug 27, 2024
1 parent 0517f2c commit 61660da
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 53 deletions.
51 changes: 0 additions & 51 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -231,57 +231,6 @@ jobs:
- name: Run yarn test262
run: exit 0 # TODO remove test262 from required tests for CI

platform-compatibility-test:
name: platform-compatibility-test

# begin macro

runs-on: ${{ matrix.platform }}
strategy:
fail-fast: false
matrix:
node-version: [18.x, 20.x]
platform: [ubuntu-latest]

steps:
- name: Checkout
uses: actions/checkout@v3

# without this, setup-node errors on mismatched yarn versions
- run: corepack enable

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
cache: yarn

- name: Echo node version
run: node --version

- name: Install dependencies
run: yarn install --immutable

# end macro

- name: 'build'
run: yarn run build

# fails under Node v12
- run: corepack disable

- name: 'switch to node v12'
uses: actions/setup-node@v3
with:
node-version: '12.x'

- name: Echo node version
run: node --version

- name: Run test:platform-compatibility
# npm b/c Yarn 4 doesn't work in Node 12
run: cd packages/ses && npm run test:platform-compatibility

viable-release:
name: viable-release

Expand Down
3 changes: 1 addition & 2 deletions packages/ses/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@
"lint:types": "tsc",
"prepare": "npm run clean && npm run build",
"qt": "ava",
"test": "tsd && ava",
"test:platform-compatibility": "node test/package/test.cjs"
"test": "tsd && ava"
},
"dependencies": {
"@endo/env-options": "workspace:^"
Expand Down

0 comments on commit 61660da

Please sign in to comment.