Skip to content

Commit

Permalink
feat(test runner): make expect.extend immutable (#32366)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@Skn0tt
Skn0tt committed Sep 12, 2024
1 parent c9f3eb1 commit ef4be6a
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 58 deletions.
65 changes: 51 additions & 14 deletions packages/playwright/src/matchers/expect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import {
captureRawStack,
createGuid,
isString,
pollAgainstDeadline } from 'playwright-core/lib/utils';
import type { ExpectZone } from 'playwright-core/lib/utils';
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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);
};
}

Expand All @@ -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 || {};
Expand All @@ -178,7 +195,7 @@ function createExpect(info: ExpectMetaInfo) {
newInfo.pollIntervals = configuration._poll.intervals;
}
}
return createExpect(newInfo);
return createExpect(newInfo, prefix, customMatchers);
};

return expectInstance;
Expand Down Expand Up @@ -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') {
Expand All @@ -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();
Expand Down Expand Up @@ -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);

Expand All @@ -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 };
Expand Down Expand Up @@ -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;
}
86 changes: 46 additions & 40 deletions tests/playwright-test/expect.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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': `
Expand Down Expand Up @@ -1043,23 +1008,64 @@ 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);
},
});
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');
});
`
});
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',
]);
});
9 changes: 5 additions & 4 deletions tests/playwright-test/test-step.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand All @@ -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
Expand Down

0 comments on commit ef4be6a

Please sign in to comment.