FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: do not check text for engine-generated error by Trott · Pull Request #19215 · nodejs/node · GitHub

/ node Public

test: do not check text for engine-generated error - #19215

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:do-not-check-message
Closed

test: do not check text for engine-generated error#19215
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:do-not-check-message

Conversation

Trott commented Mar 8, 2018
edited
Loading

Copy link
Copy Markdown
Member

In test-tls-wrap-event-emitter, the text of a TypeError is checked as
part of the test. However, this text is generated by the JavaScript
engine (V8) and not Node.js. Thus, we should not check the text as JS
engine error messages can change without it being considered a breaking
change for Node.js.

A side effect is that node-chakracore will no longer need to patch this
test to pass.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 8, 2018

Trott commented Mar 8, 2018

Copy link
Copy Markdown
Member Author

@nodejs/chakracore

Trott commented Mar 8, 2018

Copy link
Copy Markdown
Member Author

In test-tls-wrap-event-emitter, the text of a TypeError is checked as
part of the test. However, this text is generated by the JavaScript
engine (V8) and not Node.js. Thus, we should not check the text as JS
engine error messages can change without it being considered a breaking
change for Node.js.

A side effect is that node-chakracore will no longer need to patch this
test to pass.
Trott added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 8, 2018
Trott force-pushed the do-not-check-message branch from 6661a56 to d7e1eea Compare March 8, 2018 05:57

Trott commented Mar 9, 2018

Copy link
Copy Markdown
Member Author

(CI failures are unrelated.)

Trott commented Mar 10, 2018

Copy link
Copy Markdown
Member Author

Landed in a910320

Trott closed this Mar 10, 2018
Trott added a commit that referenced this pull request Mar 10, 2018
In test-tls-wrap-event-emitter, the text of a TypeError is checked as
part of the test. However, this text is generated by the JavaScript
engine (V8) and not Node.js. Thus, we should not check the text as JS
engine error messages can change without it being considered a breaking
change for Node.js.

A side effect is that node-chakracore will no longer need to patch this
test to pass.

PR-URL: #19215
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Mar 17, 2018
In test-tls-wrap-event-emitter, the text of a TypeError is checked as
part of the test. However, this text is generated by the JavaScript
engine (V8) and not Node.js. Thus, we should not check the text as JS
engine error messages can change without it being considered a breaking
change for Node.js.

A side effect is that node-chakracore will no longer need to patch this
test to pass.

PR-URL: #19215
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos mentioned this pull request Mar 18, 2018
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
In test-tls-wrap-event-emitter, the text of a TypeError is checked as
part of the test. However, this text is generated by the JavaScript
engine (V8) and not Node.js. Thus, we should not check the text as JS
engine error messages can change without it being considered a breaking
change for Node.js.

A side effect is that node-chakracore will no longer need to patch this
test to pass.

PR-URL: #19215
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
In test-tls-wrap-event-emitter, the text of a TypeError is checked as
part of the test. However, this text is generated by the JavaScript
engine (V8) and not Node.js. Thus, we should not check the text as JS
engine error messages can change without it being considered a breaking
change for Node.js.

A side effect is that node-chakracore will no longer need to patch this
test to pass.

PR-URL: nodejs#19215
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit to jasnell/node that referenced this pull request Aug 17, 2018
In test-tls-wrap-event-emitter, the text of a TypeError is checked as
part of the test. However, this text is generated by the JavaScript
engine (V8) and not Node.js. Thus, we should not check the text as JS
engine error messages can change without it being considered a breaking
change for Node.js.

A side effect is that node-chakracore will no longer need to patch this
test to pass.

PR-URL: nodejs#19215
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 6, 2018
In test-tls-wrap-event-emitter, the text of a TypeError is checked as
part of the test. However, this text is generated by the JavaScript
engine (V8) and not Node.js. Thus, we should not check the text as JS
engine error messages can change without it being considered a breaking
change for Node.js.

A side effect is that node-chakracore will no longer need to patch this
test to pass.

Backport-PR-URL: #22380
PR-URL: #19215
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins mentioned this pull request Sep 6, 2018
Trott deleted the do-not-check-message branch January 13, 2022 22:48
This file contains hidden or 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL