-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
test: make debugp collect IO #1485
Conversation
test/playwright.spec.js
Outdated
state.browser._setDebugFunction(onLine); | ||
} | ||
|
||
let rl; | ||
if (state.browserServer.process().stderr) { |
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.
this already happens here. I guess you want stdout as well, you can add it there.
test/playwright.spec.js
Outdated
@@ -113,8 +113,11 @@ module.exports.describe = ({testRunner, product, playwrightPath}) => { | |||
|
|||
beforeEach(async(state, test) => { | |||
const onLine = (line) => test.output += line + '\n'; | |||
if (dumpProtocolOnFailure) | |||
if (dumpProtocolOnFailure) { | |||
state.browserServer.process().stdout.on('data', onLine); |
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.
stdout might be null on the process is somebody set dumpio=true.
test/browsercontext.spec.js
Outdated
@@ -422,7 +422,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF | |||
await context.close(); | |||
}); | |||
// flaky: https://github.com/microsoft/playwright/pull/1301/checks?check_run_id=496478707 | |||
it.fail(FFOX && LINUX)('should fail if wrong credentials', async({browser, server}) => { | |||
fit('should fail if wrong credentials', async({browser, server}) => { |
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.
fit('should fail if wrong credentials', async({browser, server}) => { | |
it.fail(FFOX && LINUX)('should fail if wrong credentials', async({browser, server}) => { |
a3f9025
to
b91f5eb
Compare
@@ -207,7 +207,8 @@ class Reporter { | |||
console.log(`${prefix} ${colors.red(`[TIMEOUT ${test.timeout}ms]`)} ${test.fullName} (${formatLocation(test.location)})`); | |||
if (test.output) { | |||
console.log(' Output:'); | |||
console.log(padLines(test.output, 4)); | |||
for (const line of test.output) |
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.
This will indent poorly if a line of output is multiline.
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.
I'd like to be consistent with requre('debug')
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.
previously I could do test.output += JSON.stringify(obj, undefined, 2)
. Now I can't without breaking the indent.
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.
We were never putting anything from stdio there at the first place, let's see how it looks first
@@ -207,7 +207,8 @@ class Reporter { | |||
console.log(`${prefix} ${colors.red(`[TIMEOUT ${test.timeout}ms]`)} ${test.fullName} (${formatLocation(test.location)})`); | |||
if (test.output) { | |||
console.log(' Output:'); | |||
console.log(padLines(test.output, 4)); | |||
for (const line of test.output) | |||
console.log(' ' + line); |
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.
console.log(' ' + line); | |
console.log(padLines(line, 4)); |
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.
These are different things
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.
If it works, sure.
5965e27
to
d762557
Compare
This reverts commit b1bebda.
This reverts commit b1bebda.
No description provided.