[Docs]: custom matchers using extend with locators that retry example has problems

[lukpsaxo]

Page(s)

https://playwright.dev/docs/next/test-assertions#add-custom-matchers-using-expectextend

Description

the example has a few problems..

the problems I came across:

1. isNot - if you use expect.poll and someone uses .not. it does work, but the poller times out and then the command passes, leading to long tests.
   you also see a whole load of red in the ui because its matching x but actually you want not x.

2. if you use strict mode and match more than 1 locator you need to throw that rather than the assertion failure or else the message and reason for the failure is lost.
   I am worried there can be other scenarios similiar to (2) and I was not 100% sure how to solve it. E.g. if the element is not attached?

specifically to fix this I had to do something like

await baseExpect(locator).toHaveAttribute('data-amount', String(expected), options);
// ->
let expected = await baseExpect(locator)
if (this.isNot) {
    expected = expected.not;
}
await expected;

// and later on
    // switch pass back to be the inverse
    if (this.isNot) {
        pass = !pass;
    }

in order to support isNot.

Then in the catch to support other exceptions I had to add something like this:

    } catch (e: any) {
        // if we are testing more than 1 locator we need to show that error message
        if (e.message.includes('strict mode')) {
            throw e;
        }
        matcherResult = e.matcherResult;
    }

here is a full example:

export default async function toContainClass(
    this: ExpectMatcherState,
    locator: Locator,
    expectedClass: string,
) {
    const regex = makeRegex(expectedClass);
    let pass = false;
    let matcherResult: any;
    try {
        let poll = expect.poll(
            async () => {
                const className = await locator.getAttribute('class');
                console.log(className);
                return className;
            },
            {
                timeout: 8_000,
                message: 'toContainClass',
            },
        );
        // we inverse it because otherwise we are polling for a long time
        if (this.isNot) {
            poll = poll.not;
        }
        await poll.toMatch(regex);
        pass = true;
    } catch (e: any) {
        // if we are testing more than 1 locator we need to show that error message
        if (e.message.includes('strict mode')) {
            throw e;
        }
        matcherResult = e.matcherResult;
    }

    // switch pass back to be the inverse
    if (this.isNot) {
        pass = !pass;
    }

    const message = pass
        ? () =>
              this.utils.matcherHint('toContainClass', locator, expectedClass, {
                  isNot: this.isNot,
              }) +
              '\n\n' +
              `Expected: \${this.isNot ? 'not' : ''}\${this.utils.printExpected(expected)}\n` +
              (matcherResult
                  ? `Received: ${this.utils.printReceived(matcherResult.actual)}`
                  : '')
        : () =>
              this.utils.matcherHint('toContainClass', locator, expectedClass) +
              '\n\n' +
              `Expected: ${this.utils.printExpected(expectedClass)}\n` +
              (matcherResult
                  ? `Received: ${this.utils.printReceived(matcherResult.actual)}`
                  : '');

    return {
        pass,
        message,
    };
}

but I have doubts if playwright really thinks this is the best way to do it and if they should provide a utility for adding these types of assertions.

[pavelfeldman]

Can you file these as separate bugs along with your expectations and actual behavior?