-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
repl: always show all error properties #20801
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ const fixtures = require('../common/fixtures'); | |
const assert = require('assert'); | ||
const net = require('net'); | ||
const repl = require('repl'); | ||
const { inspect } = require('util'); | ||
|
||
common.globalCheck = false; | ||
common.crashOnUnhandledRejection(); | ||
|
@@ -62,7 +63,7 @@ async function runReplTests(socket, prompt, tests) { | |
expectedLine = send; | ||
|
||
while (!lineBuffer.includes('\n')) { | ||
lineBuffer += await event(socket, 'data'); | ||
lineBuffer += await event(socket, expect); | ||
|
||
// Cut away the initial prompt | ||
while (lineBuffer.startsWith(prompt)) | ||
|
@@ -523,11 +524,11 @@ const errorTests = [ | |
{ | ||
send: 'require("internal/repl")', | ||
expect: [ | ||
/^Error: Cannot find module 'internal\/repl'/, | ||
/^{ Error: Cannot find module 'internal\/repl'/, | ||
/^ at .*/, | ||
/^ at .*/, | ||
/^ at .*/, | ||
/^ at .*/ | ||
/^ at .* code: 'MODULE_NOT_FOUND' }/ | ||
] | ||
}, | ||
// REPL should handle quotes within regexp literal in multiline mode | ||
|
@@ -836,8 +837,16 @@ function startUnixRepl() { | |
]); | ||
} | ||
|
||
function event(ee, eventName) { | ||
return new Promise((resolve) => { | ||
ee.once(eventName, common.mustCall(resolve)); | ||
function event(ee, expected) { | ||
return new Promise((resolve, reject) => { | ||
const timeout = setTimeout(() => { | ||
reject( | ||
new Error(`Repl did not reply as expected for:\n\n${inspect(expected)}`) | ||
); | ||
}, 500); | ||
ee.once('data', common.mustCall((...args) => { | ||
clearTimeout(timeout); | ||
resolve(...args); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With promises I believe once There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But it would keep the test alive after being done if I am not mistaken. |
||
})); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
expected
always an event name (a string?).If it is I think padding with two newlines is a bit much. It could just go right after the
:
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, expected is the expected output, so either a string or an array with many strings.