fix(inspector): stop on all snapshottable actions (#8990)

This commit is contained in:
Pavel Feldman 2021-09-17 15:24:15 -07:00 committed by GitHub
parent e7a7a0cfc1
commit 63ff405e6e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 134 additions and 14 deletions

View file

@ -3668,6 +3668,7 @@ export const commandsWithTracingSnapshots = new Set([
'Frame.addStyleTag', 'Frame.addStyleTag',
'Frame.check', 'Frame.check',
'Frame.click', 'Frame.click',
'Frame.dragAndDrop',
'Frame.dblclick', 'Frame.dblclick',
'Frame.dispatchEvent', 'Frame.dispatchEvent',
'Frame.evaluateExpression', 'Frame.evaluateExpression',
@ -3683,6 +3684,8 @@ export const commandsWithTracingSnapshots = new Set([
'Frame.isChecked', 'Frame.isChecked',
'Frame.isDisabled', 'Frame.isDisabled',
'Frame.isEnabled', 'Frame.isEnabled',
'Frame.isHidden',
'Frame.isVisible',
'Frame.isEditable', 'Frame.isEditable',
'Frame.press', 'Frame.press',
'Frame.selectOption', 'Frame.selectOption',
@ -3706,14 +3709,50 @@ export const commandsWithTracingSnapshots = new Set([
'ElementHandle.dispatchEvent', 'ElementHandle.dispatchEvent',
'ElementHandle.fill', 'ElementHandle.fill',
'ElementHandle.hover', 'ElementHandle.hover',
'ElementHandle.innerHTML',
'ElementHandle.innerText',
'ElementHandle.inputValue',
'ElementHandle.isChecked',
'ElementHandle.isDisabled',
'ElementHandle.isEditable',
'ElementHandle.isEnabled',
'ElementHandle.isHidden',
'ElementHandle.isVisible',
'ElementHandle.press', 'ElementHandle.press',
'ElementHandle.scrollIntoViewIfNeeded', 'ElementHandle.scrollIntoViewIfNeeded',
'ElementHandle.selectOption', 'ElementHandle.selectOption',
'ElementHandle.selectText', 'ElementHandle.selectText',
'ElementHandle.setInputFiles', 'ElementHandle.setInputFiles',
'ElementHandle.tap', 'ElementHandle.tap',
'ElementHandle.textContent',
'ElementHandle.type', 'ElementHandle.type',
'ElementHandle.uncheck', 'ElementHandle.uncheck',
'ElementHandle.waitForElementState', 'ElementHandle.waitForElementState',
'ElementHandle.waitForSelector' 'ElementHandle.waitForSelector'
]);
export const pausesBeforeInputActions = new Set([
'Frame.check',
'Frame.click',
'Frame.dragAndDrop',
'Frame.dblclick',
'Frame.fill',
'Frame.hover',
'Frame.press',
'Frame.selectOption',
'Frame.setInputFiles',
'Frame.tap',
'Frame.type',
'Frame.uncheck',
'ElementHandle.check',
'ElementHandle.click',
'ElementHandle.dblclick',
'ElementHandle.fill',
'ElementHandle.hover',
'ElementHandle.press',
'ElementHandle.selectOption',
'ElementHandle.setInputFiles',
'ElementHandle.tap',
'ElementHandle.type',
'ElementHandle.uncheck'
]); ]);

View file

@ -1273,6 +1273,7 @@ Frame:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
click: click:
parameters: parameters:
@ -1302,6 +1303,7 @@ Frame:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
content: content:
returns: returns:
@ -1317,6 +1319,9 @@ Frame:
trial: boolean? trial: boolean?
sourcePosition: Point? sourcePosition: Point?
targetPosition: Point? targetPosition: Point?
tracing:
snapshot: true
pausesBeforeInput: true
dblclick: dblclick:
parameters: parameters:
@ -1345,6 +1350,7 @@ Frame:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
dispatchEvent: dispatchEvent:
parameters: parameters:
@ -1386,6 +1392,7 @@ Frame:
noWaitAfter: boolean? noWaitAfter: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
focus: focus:
parameters: parameters:
@ -1445,6 +1452,7 @@ Frame:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
innerHTML: innerHTML:
parameters: parameters:
@ -1512,6 +1520,8 @@ Frame:
strict: boolean? strict: boolean?
returns: returns:
value: boolean value: boolean
tracing:
snapshot: true
isVisible: isVisible:
parameters: parameters:
@ -1519,6 +1529,8 @@ Frame:
strict: boolean? strict: boolean?
returns: returns:
value: boolean value: boolean
tracing:
snapshot: true
isEditable: isEditable:
parameters: parameters:
@ -1540,6 +1552,7 @@ Frame:
timeout: number? timeout: number?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
querySelector: querySelector:
parameters: parameters:
@ -1580,6 +1593,7 @@ Frame:
items: string items: string
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
setContent: setContent:
parameters: parameters:
@ -1610,6 +1624,7 @@ Frame:
noWaitAfter: boolean? noWaitAfter: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
tap: tap:
parameters: parameters:
@ -1631,6 +1646,7 @@ Frame:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
textContent: textContent:
parameters: parameters:
@ -1656,6 +1672,7 @@ Frame:
timeout: number? timeout: number?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
uncheck: uncheck:
parameters: parameters:
@ -1668,6 +1685,7 @@ Frame:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
waitForFunction: waitForFunction:
parameters: parameters:
@ -1858,6 +1876,7 @@ ElementHandle:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
click: click:
parameters: parameters:
@ -1885,6 +1904,7 @@ ElementHandle:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
contentFrame: contentFrame:
returns: returns:
@ -1915,6 +1935,7 @@ ElementHandle:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
dispatchEvent: dispatchEvent:
parameters: parameters:
@ -1931,6 +1952,7 @@ ElementHandle:
noWaitAfter: boolean? noWaitAfter: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
focus: focus:
@ -1957,42 +1979,61 @@ ElementHandle:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
innerHTML: innerHTML:
returns: returns:
value: string value: string
tracing:
snapshot: true
innerText: innerText:
returns: returns:
value: string value: string
tracing:
snapshot: true
inputValue: inputValue:
returns: returns:
value: string value: string
tracing:
snapshot: true
isChecked: isChecked:
returns: returns:
value: boolean value: boolean
tracing:
snapshot: true
isDisabled: isDisabled:
returns: returns:
value: boolean value: boolean
tracing:
snapshot: true
isEditable: isEditable:
returns: returns:
value: boolean value: boolean
tracing:
snapshot: true
isEnabled: isEnabled:
returns: returns:
value: boolean value: boolean
tracing:
snapshot: true
isHidden: isHidden:
returns: returns:
value: boolean value: boolean
tracing:
snapshot: true
isVisible: isVisible:
returns: returns:
value: boolean value: boolean
tracing:
snapshot: true
ownerFrame: ownerFrame:
returns: returns:
@ -2006,6 +2047,7 @@ ElementHandle:
noWaitAfter: boolean? noWaitAfter: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
querySelector: querySelector:
parameters: parameters:
@ -2063,6 +2105,7 @@ ElementHandle:
items: string items: string
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
selectText: selectText:
parameters: parameters:
@ -2085,6 +2128,7 @@ ElementHandle:
noWaitAfter: boolean? noWaitAfter: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
tap: tap:
parameters: parameters:
@ -2104,10 +2148,13 @@ ElementHandle:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
textContent: textContent:
returns: returns:
value: string? value: string?
tracing:
snapshot: true
type: type:
parameters: parameters:
@ -2117,6 +2164,7 @@ ElementHandle:
timeout: number? timeout: number?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
uncheck: uncheck:
parameters: parameters:
@ -2127,6 +2175,7 @@ ElementHandle:
trial: boolean? trial: boolean?
tracing: tracing:
snapshot: true snapshot: true
pausesBeforeInput: true
waitForElementState: waitForElementState:
parameters: parameters:

View file

@ -989,6 +989,7 @@ export function textContentTask(selector: SelectorInfo): SchedulableTask<string
if (!element) if (!element)
return continuePolling; return continuePolling;
progress.log(` selector resolved to ${injected.previewNode(element)}`); progress.log(` selector resolved to ${injected.previewNode(element)}`);
progress.log(` retrieving textContent`);
return element.textContent; return element.textContent;
}); });
}, { parsed: selector.parsed, strict: selector.strict }); }, { parsed: selector.parsed, strict: selector.strict });

View file

@ -1040,7 +1040,7 @@ export class Frame extends SdkObject {
const info = this._page.parseSelector(selector, options); const info = this._page.parseSelector(selector, options);
const task = dom.textContentTask(info); const task = dom.textContentTask(info);
return controller.run(async progress => { return controller.run(async progress => {
progress.log(` retrieving textContent from "${selector}"`); progress.log(` waiting for selector "${selector}"\u2026`);
return this._scheduleRerunnableTask(progress, info.world, task); return this._scheduleRerunnableTask(progress, info.world, task);
}, this._page._timeoutSettings.timeout(options)); }, this._page._timeoutSettings.timeout(options));
} }

View file

@ -19,6 +19,7 @@ import { debugMode, isUnderTest, monotonicTime } from '../../utils/utils';
import { BrowserContext } from '../browserContext'; import { BrowserContext } from '../browserContext';
import { CallMetadata, InstrumentationListener, SdkObject } from '../instrumentation'; import { CallMetadata, InstrumentationListener, SdkObject } from '../instrumentation';
import { debugLogger } from '../../utils/debugLogger'; import { debugLogger } from '../../utils/debugLogger';
import { commandsWithTracingSnapshots, pausesBeforeInputActions } from '../../protocol/channels';
const symbol = Symbol('Debugger'); const symbol = Symbol('Debugger');
@ -55,7 +56,7 @@ export class Debugger extends EventEmitter implements InstrumentationListener {
async onBeforeCall(sdkObject: SdkObject, metadata: CallMetadata): Promise<void> { async onBeforeCall(sdkObject: SdkObject, metadata: CallMetadata): Promise<void> {
if (this._muted) if (this._muted)
return; return;
if (shouldPauseOnCall(sdkObject, metadata) || (this._pauseOnNextStatement && shouldPauseOnNonInputStep(sdkObject, metadata))) if (shouldPauseOnCall(sdkObject, metadata) || (this._pauseOnNextStatement && shouldPauseBeforeStep(metadata)))
await this.pause(sdkObject, metadata); await this.pause(sdkObject, metadata);
} }
@ -117,8 +118,14 @@ function shouldPauseOnCall(sdkObject: SdkObject, metadata: CallMetadata): boolea
return metadata.method === 'pause'; return metadata.method === 'pause';
} }
const nonInputActionsToStep = new Set(['close', 'evaluate', 'evaluateHandle', 'goto', 'setContent']); function shouldPauseBeforeStep(metadata: CallMetadata): boolean {
// Always stop on 'close'
function shouldPauseOnNonInputStep(sdkObject: SdkObject, metadata: CallMetadata): boolean { if (metadata.method === 'close')
return nonInputActionsToStep.has(metadata.method); return true;
if (metadata.method === 'waitForSelector' || metadata.method === 'waitForEventInfo')
return false; // Never stop on those, primarily for the test harness.
const step = metadata.type + '.' + metadata.method;
// Stop before everything that generates snapshot. But don't stop before those marked as pausesBeforeInputActions
// since we stop in them on a separate instrumentation signal.
return commandsWithTracingSnapshots.has(step) && !pausesBeforeInputActions.has(metadata.type + '.' + metadata.method);
} }

View file

@ -21,6 +21,7 @@ export function metadataToCallLog(metadata: CallMetadata, status: CallLogStatus)
let title = metadata.apiName || metadata.method; let title = metadata.apiName || metadata.method;
if (metadata.method === 'waitForEventInfo') if (metadata.method === 'waitForEventInfo')
title += `(${metadata.params.info.event})`; title += `(${metadata.params.info.event})`;
title = title.replace('object.expect', 'expect');
if (metadata.error) if (metadata.error)
status = 'error'; status = 'error';
const params = { const params = {

View file

@ -68,7 +68,7 @@ const customMatchers = {
}; };
function wrap(matcherName: string, matcher: any) { function wrap(matcherName: string, matcher: any) {
return function(this: any, ...args: any[]) { const result = function(this: any, ...args: any[]) {
const testInfo = currentTestInfo(); const testInfo = currentTestInfo();
if (!testInfo) if (!testInfo)
return matcher.call(this, ...args); return matcher.call(this, ...args);
@ -107,6 +107,8 @@ function wrap(matcherName: string, matcher: any) {
reportStepError(e); reportStepError(e);
} }
}; };
result.displayName = 'expect.' + matcherName;
return result;
} }
const wrappedMatchers: any = {}; const wrappedMatchers: any = {};

View file

@ -33,6 +33,8 @@ export function rewriteErrorMessage<E extends Error>(e: E, newMessage: string):
const ROOT_DIR = path.resolve(__dirname, '..', '..'); const ROOT_DIR = path.resolve(__dirname, '..', '..');
const CLIENT_LIB = path.join(ROOT_DIR, 'lib', 'client'); const CLIENT_LIB = path.join(ROOT_DIR, 'lib', 'client');
const CLIENT_SRC = path.join(ROOT_DIR, 'src', 'client'); const CLIENT_SRC = path.join(ROOT_DIR, 'src', 'client');
const TEST_LIB = path.join(ROOT_DIR, 'lib', 'test');
const TEST_SRC = path.join(ROOT_DIR, 'src', 'test');
export type ParsedStackTrace = { export type ParsedStackTrace = {
allFrames: StackFrame[]; allFrames: StackFrame[];
@ -60,9 +62,18 @@ export function captureStackTrace(): ParsedStackTrace {
return null; return null;
if (frame.file.startsWith('internal')) if (frame.file.startsWith('internal'))
return null; return null;
if (frame.file.includes(path.join('node_modules', 'expect')))
return null;
const fileName = path.resolve(process.cwd(), frame.file); const fileName = path.resolve(process.cwd(), frame.file);
if (isTesting && fileName.includes(path.join('playwright', 'tests', 'config', 'coverage.js'))) if (isTesting && fileName.includes(path.join('playwright', 'tests', 'config', 'coverage.js')))
return null; return null;
const inClient =
// Allow fixtures in the reported stacks.
(!fileName.includes('test/index') && !fileName.includes('test\\index')) && (
fileName.startsWith(CLIENT_LIB)
|| fileName.startsWith(CLIENT_SRC)
|| fileName.startsWith(TEST_LIB)
|| fileName.startsWith(TEST_SRC));
const parsed: ParsedFrame = { const parsed: ParsedFrame = {
frame: { frame: {
file: fileName, file: fileName,
@ -71,10 +82,10 @@ export function captureStackTrace(): ParsedStackTrace {
function: frame.function, function: frame.function,
}, },
frameText: line, frameText: line,
inClient: fileName.startsWith(CLIENT_LIB) || fileName.startsWith(CLIENT_SRC), inClient
}; };
return parsed; return parsed;
}).filter(frame => !!frame) as ParsedFrame[]; }).filter(Boolean) as ParsedFrame[];
let apiName = ''; let apiName = '';
// Deepest transition between non-client code calling into client code // Deepest transition between non-client code calling into client code

View file

@ -79,7 +79,7 @@
.call-log-selector { .call-log-selector {
color: var(--orange); color: var(--orange);
white-space: normal; white-space: nowrap;
} }
.call-log-time { .call-log-time {

View file

@ -268,7 +268,7 @@ test('should report error and pending operations on timeout', async ({ runInline
expect(result.output).toContain('Pending operations:'); expect(result.output).toContain('Pending operations:');
expect(result.output).toContain('- page.click at a.test.ts:9:16'); expect(result.output).toContain('- page.click at a.test.ts:9:16');
expect(result.output).toContain('- page.textContent at a.test.ts:10:16'); expect(result.output).toContain('- page.textContent at a.test.ts:10:16');
expect(result.output).toContain('retrieving textContent from "text=More missing"'); expect(result.output).toContain('waiting for selector');
expect(stripAscii(result.output)).toContain(`10 | page.textContent('text=More missing'),`); expect(stripAscii(result.output)).toContain(`10 | page.textContent('text=More missing'),`);
}); });

View file

@ -229,9 +229,9 @@ test('should report expect steps', async ({ runInlineTest }) => {
`%% end {\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}`,
`%% end {\"title\":\"Before Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}]}`, `%% end {\"title\":\"Before Hooks\",\"category\":\"hook\",\"steps\":[{\"title\":\"browserContext.newPage\",\"category\":\"pw:api\"}]}`,
`%% begin {\"title\":\"expect.not.toHaveTitle\",\"category\":\"expect\"}`, `%% begin {\"title\":\"expect.not.toHaveTitle\",\"category\":\"expect\"}`,
`%% begin {\"title\":\"page.title\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"object.expect.toHaveTitle\",\"category\":\"pw:api\"}`,
`%% end {\"title\":\"page.title\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"object.expect.toHaveTitle\",\"category\":\"pw:api\"}`,
`%% end {\"title\":\"expect.not.toHaveTitle\",\"category\":\"expect\",\"steps\":[{\"title\":\"page.title\",\"category\":\"pw:api\"}]}`, `%% end {\"title\":\"expect.not.toHaveTitle\",\"category\":\"expect\",\"steps\":[{\"title\":\"object.expect.toHaveTitle\",\"category\":\"pw:api\"}]}`,
`%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`, `%% begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`,
`%% begin {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`, `%% begin {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`,
`%% end {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`, `%% end {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`,

View file

@ -170,6 +170,7 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
`]; `];
const tracingSnapshots = []; const tracingSnapshots = [];
const pausesBeforeInputActions = [];
const yml = fs.readFileSync(path.join(__dirname, '..', 'src', 'protocol', 'protocol.yml'), 'utf-8'); const yml = fs.readFileSync(path.join(__dirname, '..', 'src', 'protocol', 'protocol.yml'), 'utf-8');
const protocol = yaml.parse(yml); const protocol = yaml.parse(yml);
@ -232,6 +233,11 @@ for (const [name, item] of Object.entries(protocol)) {
for (const derived of derivedClasses.get(name) || []) for (const derived of derivedClasses.get(name) || [])
tracingSnapshots.push(derived + '.' + methodName); tracingSnapshots.push(derived + '.' + methodName);
} }
if (method.tracing && method.tracing.pausesBeforeInput) {
pausesBeforeInputActions.push(name + '.' + methodName);
for (const derived of derivedClasses.get(name) || [])
pausesBeforeInputActions.push(derived + '.' + methodName);
}
const parameters = objectType(method.parameters || {}, ''); const parameters = objectType(method.parameters || {}, '');
const paramsName = `${channelName}${titleCase(methodName)}Params`; const paramsName = `${channelName}${titleCase(methodName)}Params`;
const optionsName = `${channelName}${titleCase(methodName)}Options`; const optionsName = `${channelName}${titleCase(methodName)}Options`;
@ -271,6 +277,10 @@ for (const [name, item] of Object.entries(protocol)) {
channels_ts.push(`export const commandsWithTracingSnapshots = new Set([ channels_ts.push(`export const commandsWithTracingSnapshots = new Set([
'${tracingSnapshots.join(`',\n '`)}' '${tracingSnapshots.join(`',\n '`)}'
]);`); ]);`);
channels_ts.push('');
channels_ts.push(`export const pausesBeforeInputActions = new Set([
'${pausesBeforeInputActions.join(`',\n '`)}'
]);`);
validator_ts.push(` validator_ts.push(`
return scheme; return scheme;