fix tests and other tweaks related to node 20 #2353
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The failing test was testing a failure caused by trying to parse an
object as if it were a JSON string.
It’s failing because the exception thrown by a JSON parse error
changed between Node versions.
Node 16.18.0
============
```
==> node
Welcome to Node.js v16.18.0.
Type ".help" for more information.
> JSON.parse("{")
Uncaught SyntaxError: Unexpected end of JSON input
> JSON.parse({})
Uncaught SyntaxError: Unexpected token o in JSON at position 1
```
Node 20.11.1
============
```
==> nvm use 20.11.1
Now using node v20.11.1 (npm v10.2.4)
==> node
Welcome to Node.js v20.11.1.
Type ".help" for more information.
> JSON.parse({})
Uncaught SyntaxError: "[object Object]" is not valid JSON
```
You can get the integer from a javascript date with `.getTime()`
lperson
commented
Mar 24, 2024
| @@ -632,30 +636,19 @@ describe("graphql test suite", () => { | |||
| typeof copiedCampaign.due_by === "number" || | |||
| typeof copiedCampaign.due_by === "string" | |||
| ) { | |||
| let parsedDate = new Date(copiedCampaign.due_by); | |||
| const parsedDate = new Date(copiedCampaign.due_by); | |||
There was a problem hiding this comment.
To silence the linter.
lperson
commented
Mar 24, 2024
| if ( | ||
| typeof copiedCampaign.features === "object" && | ||
| copiedCampaign.features | ||
| ) { | ||
| let jsonString = JSON.stringify(copiedCampaign.features); | ||
| const jsonString = JSON.stringify(copiedCampaign.features); |
There was a problem hiding this comment.
To silence the linter.
…20 specified in package.json
lperson
changed the title
lperson
force-pushed
the
new-node-20-branch
branch
from
0164840
to
8f4791e
Compare
lperson
changed the title
|
@mau11 I think it's worth reviewing this and merging it if you approve. It gets the tests a lot further than before. I'll continue to work on the redis tests failing and make another PR. |
|
This looks great! |
mau11
merged commit e986194
into
StateVoicesNational:new-node-20-branch
6 of 8 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a couple of failing tests.
Additional context in a slack thread.
Description
ERR_REQUIRE_ESM errors
Pinned
strip-ansito 6.0.1.Failing actions
actions/checkout@v3toactions/checkout@v4so it doesn't complain about node 20.actions/cache@v3toactions/cache@v4so it doesn't complain about node 20.Failure in
__test__/backend.test.jsSpecify the campaign due date as milliseconds since the epoch when testing with SQLite.
I did a quick smoke test of copying a campaign in the UI when using SQLite and no additional special handling was necessary.
Failure in
__test__/extensions/action-handlers/ngpvan-action.test.jsThe failing test was testing a failure caused by trying to parse an object as if it were a JSON string.
It’s failing because the exception thrown by a JSON parse error changed between Node versions.
Node 16.18.0
Node 20.11.1
Checklist: