feat(test runner): make expect.extend immutable (#32366)

Changes `expect.extend` behaviour so that it doesn't mutate the global
instance and behaves closer to what users expect. This is formally a
breaking change, and I had to remove a test that asserts the breaking
behaviour.

TODO:
- [x] decide wether this is a separate method or a flag for
`expect.extend`
- [x] figure out if we need to change docs
This commit is contained in:
Simon Knott 2024-09-12 19:56:38 +02:00 committed by GitHub
parent c9f3eb158e
commit ef4be6afff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 102 additions and 58 deletions

View file

@ -16,6 +16,7 @@
import {
captureRawStack,
createGuid,
isString,
pollAgainstDeadline } from 'playwright-core/lib/utils';
import type { ExpectZone } from 'playwright-core/lib/utils';
@ -104,11 +105,17 @@ export const printReceivedStringContainExpectedResult = (
type ExpectMessage = string | { message?: string };
function createMatchers(actual: unknown, info: ExpectMetaInfo): any {
return new Proxy(expectLibrary(actual), new ExpectMetaInfoProxyHandler(info));
function createMatchers(actual: unknown, info: ExpectMetaInfo, prefix: string[]): any {
return new Proxy(expectLibrary(actual), new ExpectMetaInfoProxyHandler(info, prefix));
}
function createExpect(info: ExpectMetaInfo) {
const getCustomMatchersSymbol = Symbol('get custom matchers');
function qualifiedMatcherName(qualifier: string[], matcherName: string) {
return qualifier.join(':') + '$' + matcherName;
}
function createExpect(info: ExpectMetaInfo, prefix: string[], customMatchers: Record<string, Function>) {
const expectInstance: Expect<{}> = new Proxy(expectLibrary, {
apply: function(target: any, thisArg: any, argumentsList: [unknown, ExpectMessage?]) {
const [actual, messageOrOptions] = argumentsList;
@ -119,18 +126,22 @@ function createExpect(info: ExpectMetaInfo) {
throw new Error('`expect.poll()` accepts only function as a first argument');
newInfo.generator = actual as any;
}
return createMatchers(actual, newInfo);
return createMatchers(actual, newInfo, prefix);
},
get: function(target: any, property: string) {
get: function(target: any, property: string | typeof getCustomMatchersSymbol) {
if (property === 'configure')
return configure;
if (property === 'extend') {
return (matchers: any) => {
const qualifier = [...prefix, createGuid()];
const wrappedMatchers: any = {};
const extendedMatchers: any = { ...customMatchers };
for (const [name, matcher] of Object.entries(matchers)) {
wrappedMatchers[name] = function(...args: any[]) {
const key = qualifiedMatcherName(qualifier, name);
wrappedMatchers[key] = function(...args: any[]) {
const { isNot, promise, utils } = this;
const newThis: ExpectMatcherState = {
isNot,
@ -141,9 +152,12 @@ function createExpect(info: ExpectMetaInfo) {
(newThis as any).equals = throwUnsupportedExpectMatcherError;
return (matcher as any).call(newThis, ...args);
};
Object.defineProperty(wrappedMatchers[key], 'name', { value: name });
extendedMatchers[name] = wrappedMatchers[key];
}
expectLibrary.extend(wrappedMatchers);
return expectInstance;
return createExpect(info, qualifier, extendedMatchers);
};
}
@ -153,6 +167,9 @@ function createExpect(info: ExpectMetaInfo) {
};
}
if (property === getCustomMatchersSymbol)
return customMatchers;
if (property === 'poll') {
return (actual: unknown, messageOrOptions?: ExpectMessage & { timeout?: number, intervals?: number[] }) => {
const poll = isString(messageOrOptions) ? {} : messageOrOptions || {};
@ -178,7 +195,7 @@ function createExpect(info: ExpectMetaInfo) {
newInfo.pollIntervals = configuration._poll.intervals;
}
}
return createExpect(newInfo);
return createExpect(newInfo, prefix, customMatchers);
};
return expectInstance;
@ -241,15 +258,28 @@ type ExpectMetaInfo = {
class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
private _info: ExpectMetaInfo;
private _prefix: string[];
constructor(info: ExpectMetaInfo) {
constructor(info: ExpectMetaInfo, prefix: string[]) {
this._info = { ...info };
this._prefix = prefix;
}
get(target: Object, matcherName: string | symbol, receiver: any): any {
let matcher = Reflect.get(target, matcherName, receiver);
if (typeof matcherName !== 'string')
return matcher;
let resolvedMatcherName = matcherName;
for (let i = this._prefix.length; i > 0; i--) {
const qualifiedName = qualifiedMatcherName(this._prefix.slice(0, i), matcherName);
if (Reflect.has(target, qualifiedName)) {
matcher = Reflect.get(target, qualifiedName, receiver);
resolvedMatcherName = qualifiedName;
break;
}
}
if (matcher === undefined)
throw new Error(`expect: Property '${matcherName}' not found.`);
if (typeof matcher !== 'function') {
@ -260,7 +290,7 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
if (this._info.isPoll) {
if ((customAsyncMatchers as any)[matcherName] || matcherName === 'resolves' || matcherName === 'rejects')
throw new Error(`\`expect.poll()\` does not support "${matcherName}" matcher.`);
matcher = (...args: any[]) => pollMatcher(matcherName, !!this._info.isNot, this._info.pollIntervals, this._info.pollTimeout ?? currentExpectTimeout(), this._info.generator!, ...args);
matcher = (...args: any[]) => pollMatcher(resolvedMatcherName, !!this._info.isNot, this._info.pollIntervals, this._info.pollTimeout ?? currentExpectTimeout(), this._info.generator!, ...args);
}
return (...args: any[]) => {
const testInfo = currentTestInfo();
@ -320,7 +350,7 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
}
}
async function pollMatcher(matcherName: any, isNot: boolean, pollIntervals: number[] | undefined, timeout: number, generator: () => any, ...args: any[]) {
async function pollMatcher(qualifiedMatcherName: any, isNot: boolean, pollIntervals: number[] | undefined, timeout: number, generator: () => any, ...args: any[]) {
const testInfo = currentTestInfo();
const { deadline, timeoutMessage } = testInfo ? testInfo._deadlineForMatcher(timeout) : TestInfoImpl._defaultDeadlineForMatcher(timeout);
@ -333,7 +363,7 @@ async function pollMatcher(matcherName: any, isNot: boolean, pollIntervals: numb
if (isNot)
expectInstance = expectInstance.not;
try {
expectInstance[matcherName].call(expectInstance, ...args);
expectInstance[qualifiedMatcherName].call(expectInstance, ...args);
return { continuePolling: false, result: undefined };
} catch (error) {
return { continuePolling: true, result: error };
@ -375,8 +405,15 @@ function computeArgsSuffix(matcherName: string, args: any[]) {
return value ? `(${value})` : '';
}
export const expect: Expect<{}> = createExpect({}).extend(customMatchers);
export const expect: Expect<{}> = createExpect({}, [], {}).extend(customMatchers);
export function mergeExpects(...expects: any[]) {
return expect;
let merged = expect;
for (const e of expects) {
const internals = e[getCustomMatchersSymbol];
if (!internals) // non-playwright expects mutate the global expect, so we don't need to do anything special
continue;
merged = merged.extend(internals);
}
return merged;
}

View file

@ -18,41 +18,6 @@ import path from 'path';
import { test, expect, parseTestRunnerOutput, stripAnsi } from './playwright-test-fixtures';
const { spawnAsync } = require('../../packages/playwright-core/lib/utils');
test('should be able to call expect.extend in config', async ({ runInlineTest }) => {
const result = await runInlineTest({
'helper.ts': `
import { test as base, expect } from '@playwright/test';
expect.extend({
toBeWithinRange(received, floor, ceiling) {
const pass = received >= floor && received <= ceiling;
if (pass) {
return {
message: () =>
'passed',
pass: true,
};
} else {
return {
message: () => 'failed',
pass: false,
};
}
},
});
export const test = base;
`,
'expect-test.spec.ts': `
import { test } from './helper';
test('numeric ranges', () => {
test.expect(100).toBeWithinRange(90, 110);
test.expect(101).not.toBeWithinRange(0, 100);
});
`
});
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(1);
});
test('should not expand huge arrays', async ({ runInlineTest }) => {
const result = await runInlineTest({
'expect-test.spec.ts': `
@ -1043,8 +1008,8 @@ test('should expose timeout to custom matchers', async ({ runInlineTest, runTSC
test('should throw error when using .equals()', async ({ runInlineTest }) => {
const result = await runInlineTest({
'helper.ts': `
import { test as base, expect } from '@playwright/test';
expect.extend({
import { test as base, expect as baseExpect } from '@playwright/test';
export const expect = baseExpect.extend({
toBeWithinRange(received, floor, ceiling) {
this.equals(1, 2);
},
@ -1052,10 +1017,10 @@ test('should throw error when using .equals()', async ({ runInlineTest }) => {
export const test = base;
`,
'expect-test.spec.ts': `
import { test } from './helper';
import { test, expect } from './helper';
test('numeric ranges', () => {
test.expect(() => {
test.expect(100).toBeWithinRange(90, 110);
expect(() => {
expect(100).toBeWithinRange(90, 110);
}).toThrowError('It looks like you are using custom expect matchers that are not compatible with Playwright. See https://aka.ms/playwright/expect-compatibility');
});
`
@ -1063,3 +1028,44 @@ test('should throw error when using .equals()', async ({ runInlineTest }) => {
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(1);
});
test('expect.extend should be immutable', async ({ runInlineTest }) => {
const result = await runInlineTest({
'expect-test.spec.ts': `
import { test, expect } from '@playwright/test';
const expectFoo = expect.extend({
toFoo() {
console.log('%%foo');
return { pass: true };
}
});
const expectFoo2 = expect.extend({
toFoo() {
console.log('%%foo2');
return { pass: true };
}
});
const expectBar = expectFoo.extend({
toBar() {
console.log('%%bar');
return { pass: true };
}
});
test('logs', () => {
expect(expectFoo).not.toBe(expectFoo2);
expect(expectFoo).not.toBe(expectBar);
expectFoo().toFoo();
expectFoo2().toFoo();
expectBar().toFoo();
expectBar().toBar();
});
`
});
expect(result.outputLines).toEqual([
'foo',
'foo2',
'foo',
'bar',
]);
});

View file

@ -311,7 +311,9 @@ test('should report custom expect steps', async ({ runInlineTest }) => {
};
`,
'a.test.ts': `
expect.extend({
import { test, expect as baseExpect } from '@playwright/test';
const expect = baseExpect.extend({
toBeWithinRange(received, floor, ceiling) {
const pass = received >= floor && received <= ceiling;
if (pass) {
@ -338,7 +340,6 @@ test('should report custom expect steps', async ({ runInlineTest }) => {
},
});
import { test, expect } from '@playwright/test';
test('fail', async ({}) => {
expect(15).toBeWithinRange(10, 20);
await expect(1).toBeFailingAsync(22);
@ -349,8 +350,8 @@ test('should report custom expect steps', async ({ runInlineTest }) => {
expect(result.exitCode).toBe(1);
expect(result.output).toBe(`
hook |Before Hooks
expect |expect.toBeWithinRange @ a.test.ts:31
expect |expect.toBeFailingAsync @ a.test.ts:32
expect |expect.toBeWithinRange @ a.test.ts:32
expect |expect.toBeFailingAsync @ a.test.ts:33
expect | error: Error: It fails!
hook |After Hooks
hook |Worker Cleanup