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.

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

Fix test cases of JSON node #4275

Merged
merged 1 commit into from Aug 14, 2023

Conversation

kazuhitoyokoi
Copy link
Member

@kazuhitoyokoi kazuhitoyokoi commented Aug 14, 2023

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

During development, the npm test command always fails in my Node.js v20 environment.

2444 passing (2m)
  59 pending
  2 failing

  1) JSON node
       should log an error if asked to parse an invalid json string:
     AssertionError: expected 'Unexpected token \'o\', "foo" is not valid JSON' to start with 'Unexpected token o'
      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value [as startWith] (node_modules/should/cjs/should.js:356:19)
      at Timeout._onTimeout (test/nodes/core/parsers/70-JSON_spec.js:176:52)
      at listOnTimeout (node:internal/timers:573:17)
      at process.processTimers (node:internal/timers:514:7)

  2) JSON node
       should log an error if asked to parse an invalid json string in a buffer:
     AssertionError: expected 'Unexpected token \'o\', "{"name":foo}" is not valid JSON' to start with 'Unexpected token o'
      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value [as startWith] (node_modules/should/cjs/should.js:356:19)
      at Timeout._onTimeout (test/nodes/core/parsers/70-JSON_spec.js:202:52)
      at listOnTimeout (node:internal/timers:573:17)
      at process.processTimers (node:internal/timers:514:7)

After my investigation, I found that the error message of JSON.parse() has been changed from Node.js v19 as follows.

$ nvm use 18
$ node -e 'JSON.parse("foo")'
-> SyntaxError: Unexpected token o in JSON at position 1

$ nvm use 19
$ node -e 'JSON.parse("foo")'
-> SyntaxError: Unexpected token 'o', "foo" is not valid JSON

$ nvm use 20
$ node -e 'JSON.parse("foo")'
-> SyntaxError: Unexpected token 'o', "foo" is not valid JSON

The current dev branch for v3.1 is not required for this change, but it will be needed in the future release soon.

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

@knolleary knolleary merged commit 04aa489 into node-red:dev Aug 14, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants