Skip to content

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

@TrottTrott commented Mar 8, 2018

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-botnodejs-github-bot added the test Issues and PRs related to the tests. label Mar 8, 2018
@Trott

Trott commented Mar 8, 2018

Copy link
Copy Markdown
MemberAuthor

@nodejs/chakracore

@Trott

Trott commented Mar 8, 2018

Copy link
Copy Markdown
MemberAuthor

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.
@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 8, 2018
@Trott
Trottforce-pushed the do-not-check-message branch from 6661a56 to d7e1eeaCompareMarch 8, 2018 05:57
@Trott

Trott commented Mar 9, 2018

Copy link
Copy Markdown
MemberAuthor

(CI failures are unrelated.)

@Trott

Copy link
Copy Markdown
MemberAuthor

Landed in a910320

@TrottTrott 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>
@targostargos 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>
@MylesBorinsMylesBorins mentioned this pull request Sep 6, 2018
@Trott
Trott deleted the do-not-check-message branch January 13, 2022 22:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@Trott@danbev@jasnell@lpinca@cjihrig@joyeecheung@nodejs-github-bot