review feedback

This commit is contained in:
Max Schmitt 2024-03-22 22:10:03 +01:00
parent e11378043d
commit 1ca1078860
8 changed files with 45 additions and 68 deletions

View file

@ -1382,8 +1382,6 @@ export class Frame extends SdkObject {
// Library mode special case for the expect errors which are return values, not exceptions. // Library mode special case for the expect errors which are return values, not exceptions.
if (result.matches === options.isNot) if (result.matches === options.isNot)
metadata.error = { error: { name: 'Expect', message: 'Expect failed' } }; metadata.error = { error: { name: 'Expect', message: 'Expect failed' } };
if (result.log?.[result.log.length - 1].startsWith('waiting for '))
result.received = '<element(s) not found>';
return result; return result;
} }
@ -1422,7 +1420,7 @@ export class Frame extends SdkObject {
const injected = await context.injectedScript(); const injected = await context.injectedScript();
progress.throwIfAborted(); progress.throwIfAborted();
const { log, matches, received, missingRecevied } = await injected.evaluate(async (injected, { info, options, callId }) => { const { log, matches, received, missingReceived } = await injected.evaluate(async (injected, { info, options, callId }) => {
const elements = info ? injected.querySelectorAll(info.parsed, document) : []; const elements = info ? injected.querySelectorAll(info.parsed, document) : [];
const isArray = options.expression === 'to.have.count' || options.expression.endsWith('.array'); const isArray = options.expression === 'to.have.count' || options.expression.endsWith('.array');
let log = ''; let log = '';
@ -1434,16 +1432,16 @@ export class Frame extends SdkObject {
log = ` locator resolved to ${injected.previewNode(elements[0])}`; log = ` locator resolved to ${injected.previewNode(elements[0])}`;
if (callId) if (callId)
injected.markTargetElements(new Set(elements), callId); injected.markTargetElements(new Set(elements), callId);
return { log, ...(await injected.expect(elements[0], options, elements)) }; return { log, ...await injected.expect(elements[0], options, elements) };
}, { info, options, callId: metadata.id }); }, { info, options, callId: metadata.id });
if (log) if (log)
progress.log(log); progress.log(log);
// Note: missingReceived avoids `unexpected value "undefined"` when element was not found. // Note: missingReceived avoids `unexpected value "undefined"` when element was not found.
if (matches === options.isNot && !missingRecevied) { if (matches === options.isNot) {
lastIntermediateResult.received = received; lastIntermediateResult.received = missingReceived ? '<element(s) not found>' : received;
lastIntermediateResult.isSet = true; lastIntermediateResult.isSet = true;
if (!Array.isArray(received)) if (!missingReceived && !Array.isArray(received))
progress.log(` unexpected value "${renderUnexpectedValue(options.expression, received)}"`); progress.log(` unexpected value "${renderUnexpectedValue(options.expression, received)}"`);
} }
if (!oneShot && matches === options.isNot) { if (!oneShot && matches === options.isNot) {

View file

@ -1098,7 +1098,7 @@ export class InjectedScript {
this.onGlobalListenersRemoved.add(addHitTargetInterceptorListeners); this.onGlobalListenersRemoved.add(addHitTargetInterceptorListeners);
} }
async expect(element: Element | undefined, options: FrameExpectParams, elements: Element[]): Promise<{ matches: boolean, received?: any, missingRecevied?: boolean }> { async expect(element: Element | undefined, options: FrameExpectParams, elements: Element[]): Promise<{ matches: boolean, received?: any, missingReceived?: boolean }> {
const isArray = options.expression === 'to.have.count' || options.expression.endsWith('.array'); const isArray = options.expression === 'to.have.count' || options.expression.endsWith('.array');
if (isArray) if (isArray)
return this.expectArray(elements, options); return this.expectArray(elements, options);
@ -1119,7 +1119,7 @@ export class InjectedScript {
if (options.isNot && options.expression === 'to.be.in.viewport') if (options.isNot && options.expression === 'to.be.in.viewport')
return { matches: false }; return { matches: false };
// When none of the above applies, expect does not match. // When none of the above applies, expect does not match.
return { matches: options.isNot, missingRecevied: true }; return { matches: options.isNot, missingReceived: true };
} }
return await this.expectSingleElement(element, options); return await this.expectSingleElement(element, options);
} }
@ -1166,7 +1166,7 @@ export class InjectedScript {
throw this.createStacklessError('Element is not a checkbox'); throw this.createStacklessError('Element is not a checkbox');
if (elementState === 'error:notconnected') if (elementState === 'error:notconnected')
throw this.createStacklessError('Element is not connected'); throw this.createStacklessError('Element is not connected');
return { matches: elementState }; return { received: elementState, matches: elementState };
} }
} }

View file

@ -20,6 +20,8 @@ import type { Locator } from 'playwright-core';
import type { StackFrame } from '@protocol/channels'; import type { StackFrame } from '@protocol/channels';
import { stringifyStackFrames } from 'playwright-core/lib/utils'; import { stringifyStackFrames } from 'playwright-core/lib/utils';
export const kNoElementsFoundError = '<element(s) not found>';
export function matcherHint(state: ExpectMatcherContext, locator: Locator | undefined, matcherName: string, expression: any, actual: any, matcherOptions: any, timeout?: number) { export function matcherHint(state: ExpectMatcherContext, locator: Locator | undefined, matcherName: string, expression: any, actual: any, matcherOptions: any, timeout?: number) {
let header = state.utils.matcherHint(matcherName, expression, actual, matcherOptions).replace(/ \/\/ deep equality/, '') + '\n\n'; let header = state.utils.matcherHint(matcherName, expression, actual, matcherOptions).replace(/ \/\/ deep equality/, '') + '\n\n';
if (timeout) if (timeout)

View file

@ -15,7 +15,7 @@
*/ */
import { expectTypes, callLogText } from '../util'; import { expectTypes, callLogText } from '../util';
import { matcherHint } from './matcherHint'; import { kNoElementsFoundError, matcherHint } from './matcherHint';
import type { MatcherResult } from './matcherHint'; import type { MatcherResult } from './matcherHint';
import { currentExpectTimeout } from '../common/globals'; import { currentExpectTimeout } from '../common/globals';
import type { ExpectMatcherContext } from './expect'; import type { ExpectMatcherContext } from './expect';
@ -41,12 +41,13 @@ export async function toBeTruthy(
const timeout = currentExpectTimeout(options); const timeout = currentExpectTimeout(options);
const { matches, log, timedOut, received } = await query(!!this.isNot, timeout); const { matches, log, timedOut, received } = await query(!!this.isNot, timeout);
const notFound = received === kNoElementsFoundError ? received : undefined;
const actual = matches ? expected : unexpected; const actual = matches ? expected : unexpected;
const message = () => { const message = () => {
const header = matcherHint(this, receiver, matcherName, 'locator', arg, matcherOptions, timedOut ? timeout : undefined); const header = matcherHint(this, receiver, matcherName, 'locator', arg, matcherOptions, timedOut ? timeout : undefined);
const logText = callLogText(log); const logText = callLogText(log);
return matches ? `${header}Expected: not ${expected}\nReceived: ${received ?? expected}${logText}` : return matches ? `${header}Expected: not ${expected}\nReceived: ${notFound ? kNoElementsFoundError : expected}${logText}` :
`${header}Expected: ${expected}\nReceived: ${received ?? unexpected}${logText}`; `${header}Expected: ${expected}\nReceived: ${notFound ? kNoElementsFoundError : unexpected}${logText}`;
}; };
return { return {
message, message,

View file

@ -23,7 +23,7 @@ import {
printReceivedStringContainExpectedResult, printReceivedStringContainExpectedResult,
printReceivedStringContainExpectedSubstring printReceivedStringContainExpectedSubstring
} from './expect'; } from './expect';
import { matcherHint } from './matcherHint'; import { kNoElementsFoundError, matcherHint } from './matcherHint';
import type { MatcherResult } from './matcherHint'; import type { MatcherResult } from './matcherHint';
import { currentExpectTimeout } from '../common/globals'; import { currentExpectTimeout } from '../common/globals';
import type { Locator } from 'playwright-core'; import type { Locator } from 'playwright-core';
@ -64,39 +64,28 @@ export async function toMatchText(
const { matches: pass, received, log, timedOut } = await query(!!this.isNot, timeout); const { matches: pass, received, log, timedOut } = await query(!!this.isNot, timeout);
const stringSubstring = options.matchSubstring ? 'substring' : 'string'; const stringSubstring = options.matchSubstring ? 'substring' : 'string';
const receivedString = received || ''; const receivedString = received || '';
const message = pass const messagePrefix = matcherHint(this, receiver, matcherName, 'locator', undefined, matcherOptions, timedOut ? timeout : undefined);
? () => const notFound = received === kNoElementsFoundError;
typeof expected === 'string' const message = () => {
? matcherHint(this, receiver, matcherName, 'locator', undefined, matcherOptions, timedOut ? timeout : undefined) + if (pass) {
`Expected ${stringSubstring}: not ${this.utils.printExpected(expected)}\n` + if (typeof expected === 'string') {
`Received string: ${receivedString.indexOf(expected) !== -1 ? printReceivedStringContainExpectedSubstring( if (notFound)
receivedString, return messagePrefix + `Expected ${stringSubstring}: not ${this.utils.printExpected(expected)}\nReceived: ${received}` + callLogText(log);
receivedString.indexOf(expected), const printedReceived = printReceivedStringContainExpectedSubstring(receivedString, receivedString.indexOf(expected), expected.length);
expected.length, return messagePrefix + `Expected ${stringSubstring}: not ${this.utils.printExpected(expected)}\nReceived string: ${printedReceived}` + callLogText(log);
) : '"' + receivedString + '"'}` + callLogText(log) } else {
: matcherHint(this, receiver, matcherName, 'locator', undefined, matcherOptions, timedOut ? timeout : undefined) + if (notFound)
`Expected pattern: not ${this.utils.printExpected(expected)}\n` + return messagePrefix + `Expected pattern: not ${this.utils.printExpected(expected)}\nReceived: ${received}` + callLogText(log);
`Received string: ${printReceivedStringContainExpectedResult( const printedReceived = printReceivedStringContainExpectedResult(receivedString, typeof expected.exec === 'function' ? expected.exec(receivedString) : null);
receivedString, return messagePrefix + `Expected pattern: not ${this.utils.printExpected(expected)}\nReceived string: ${printedReceived}` + callLogText(log);
typeof expected.exec === 'function' }
? expected.exec(receivedString) } else {
: null, const labelExpected = `Expected ${typeof expected === 'string' ? stringSubstring : 'pattern'}`;
)}` + callLogText(log) if (notFound)
: () => { return messagePrefix + `${labelExpected}: ${this.utils.printExpected(expected)}\nReceived: ${received}` + callLogText(log);
const labelExpected = `Expected ${typeof expected === 'string' ? stringSubstring : 'pattern' return messagePrefix + this.utils.printDiffOrStringify(expected, receivedString, labelExpected, 'Received string', this.expand !== false) + callLogText(log);
}`; }
const labelReceived = 'Received string'; };
return (
matcherHint(this, receiver, matcherName, 'locator', undefined, matcherOptions, timedOut ? timeout : undefined) +
this.utils.printDiffOrStringify(
expected,
receivedString,
labelExpected,
labelReceived,
this.expand !== false,
)) + callLogText(log);
};
return { return {
name: matcherName, name: matcherName,

View file

@ -158,7 +158,7 @@ test.describe('not.toHaveText', () => {
await page.setContent('<div>hello</div>'); await page.setContent('<div>hello</div>');
const error = await expect(page.locator('span')).not.toHaveText('hello', { timeout: 1000 }).catch(e => e); const error = await expect(page.locator('span')).not.toHaveText('hello', { timeout: 1000 }).catch(e => e);
expect(stripAnsi(error.message)).toContain('Expected string: not "hello"'); expect(stripAnsi(error.message)).toContain('Expected string: not "hello"');
expect(stripAnsi(error.message)).toContain('Received string: "<element(s) not found>"'); expect(stripAnsi(error.message)).toContain('Received: <element(s) not found>');
expect(stripAnsi(error.message)).toContain('waiting for locator(\'span\')'); expect(stripAnsi(error.message)).toContain('waiting for locator(\'span\')');
}); });
}); });

View file

@ -29,31 +29,18 @@ it('should print no-locator-resolved error when locator matcher did not resolve
const myLocator = page.locator('.nonexisting'); const myLocator = page.locator('.nonexisting');
const expectWithShortLivingTimeout = expect.configure({ timeout: 10 }); const expectWithShortLivingTimeout = expect.configure({ timeout: 10 });
const locatorMatchers = [ const locatorMatchers = [
() => expectWithShortLivingTimeout(myLocator).toBeAttached(), () => expectWithShortLivingTimeout(myLocator).toBeAttached(), // Boolean matcher
() => expectWithShortLivingTimeout(myLocator).toBeChecked(), () => expectWithShortLivingTimeout(myLocator).toHaveJSProperty('abc', 'abc'), // Equal matcher
() => expectWithShortLivingTimeout(myLocator).toBeDisabled(), () => expectWithShortLivingTimeout(myLocator).not.toHaveText('abc'), // Text matcher - pass / string
() => expectWithShortLivingTimeout(myLocator).toBeEditable(), () => expectWithShortLivingTimeout(myLocator).not.toHaveText(/abc/), // Text matcher - pass / RegExp
() => expectWithShortLivingTimeout(myLocator).toBeEmpty(), () => expectWithShortLivingTimeout(myLocator).toContainText('abc'), // Text matcher - fail
() => expectWithShortLivingTimeout(myLocator).toBeEnabled(),
() => expectWithShortLivingTimeout(myLocator).toBeFocused(),
() => expectWithShortLivingTimeout(myLocator).toBeInViewport(),
() => expectWithShortLivingTimeout(myLocator).toBeVisible(),
() => expectWithShortLivingTimeout(myLocator).toContainText('abc'),
() => expectWithShortLivingTimeout(myLocator).toHaveAttribute('abc'),
() => expectWithShortLivingTimeout(myLocator).toHaveClass('abc'),
() => expectWithShortLivingTimeout(myLocator).toHaveCSS('abc', 'abc'),
() => expectWithShortLivingTimeout(myLocator).toHaveId('abc'),
() => expectWithShortLivingTimeout(myLocator).toHaveJSProperty('abc', 'abc'),
() => expectWithShortLivingTimeout(myLocator).toHaveText('abc'),
() => expectWithShortLivingTimeout(myLocator).toHaveValue('abc'),
() => expectWithShortLivingTimeout(myLocator).toHaveValues(['abc']),
]; ];
for (const matcher of locatorMatchers) { for (const matcher of locatorMatchers) {
await it.step(matcher.toString(), async () => { await it.step(matcher.toString(), async () => {
const error = await matcher().catch(e => e); const error = await matcher().catch(e => e);
expect(error).toBeInstanceOf(Error); expect(error).toBeInstanceOf(Error);
expect(error.message).toContain(`waiting for locator('.nonexisting')`); expect(error.message).toContain(`waiting for locator('.nonexisting')`);
expect(stripAnsi(error.message)).toMatch(/Received( string)?: "?<element\(s\) not found>/); expect(stripAnsi(error.message)).toMatch(/Received: ?"?<element\(s\) not found>/);
}); });
} }
}); });

View file

@ -647,7 +647,7 @@ test('should print pending operations for toHaveText', async ({ runInlineTest })
const output = result.output; const output = result.output;
expect(output).toContain(`expect(locator).toHaveText(expected)`); expect(output).toContain(`expect(locator).toHaveText(expected)`);
expect(output).toContain('Expected string: "Text"'); expect(output).toContain('Expected string: "Text"');
expect(output).toContain('Received string: "<element(s) not found>"'); expect(output).toContain('Received: <element(s) not found>');
expect(output).toContain('waiting for locator(\'no-such-thing\')'); expect(output).toContain('waiting for locator(\'no-such-thing\')');
}); });