[Feature]: Add a timeout message option to locator.waitFor() method

[marcusNumminen]

🚀 Feature Request

When locator.waitFor() do not fulfill the wait and fail we get a fairly generic error message. The error messages usually looks something like this:

 Error: locator.waitFor: Target page, context or browser has been closed
    Call log:
      - waiting for locator('#my_fancy_locator') to be visible
      ...

or

    TimeoutError: locator.waitFor: Timeout 10000ms exceeded.
    Call log:
      - waiting for locator('#my_fancy_locator') to be visible
      ...

Add a message option to waitFor option where we could provide a custom error message.

Example

// some code
// ...
page.locator('#my_fancy_locator').waitFor({ state: 'visible', timeout: 10000, message: 'Fancy Locator never displayed' });

This could produce this failing msg:

    TimeoutError: Fancy Locator never displayed (locator.waitFor: Timeout 10000ms exceeded).
    Call log:
      - waiting for locator('.modal__title-row') to be visible

or we could get a similar error message structure as expects:

TimeoutError: Fancy Locator never displayed

    locator.waitFor() failed

    Locator: locator('#my_fancy_locator')
    Waiting state: visible
    Timeout: 10000ms
    TimeoutError: element(s) not found

    Call log:
      - Fancy Locator never displayed with timeout 10000ms
      - waiting for .locator('#my_fancy_locator')

Motivation

This simple change would improve the error messages and make it easier to see what has failed.

[agg23]

Does locator.describe('Custom message') serve your use case? The describe message will show in any error messages involving that locator.

[marcusNumminen]

locator.describe could be used for this. But this feels like a workaround. We usually define the locators in a POM and if we would like to use describe there we would probably add a more human readable description of the element and that would for sure increase the readability of the fail message. But perhaps we would like to use different error messages depending in the situation and to be able to do that with describe we would need to use a "locally defined" locator with a describe for that particular situation.

What we have started to do is to replace locatror.waitFor to expect.toPass due to this but goes a bit against the "Page objects themselves should never make verifications or assertions"

I think it would have been great with custom error messages for all "waits" to increase readability of the failures

[agg23]

We are of the opinion that APIs like waitFor() should be used very sparingly; you're implicitly waiting on the visibility of a DOM node, but that's exactly what Playwright's auto-awaiting assertions already do. So no matter if your next expression after your locator('foo').waitFor() is

await locator.click();
await expect(locator).toHaveText("hello");

etc, each of those expressions will first validate that locator is visible and actionable. If this doesn't apply to you, your waitFor() is likely the last expression in your test (i.e. you don't have any following auto-awaiting assertions).

What are you using waitFor specifically?

[marcusNumminen]

We have situations were we need to wait for a toast to appear before we can proceed. For example we have to wait for the toast to appear and then make an api call that would fail otherwise. These test cases could of course be re-written without waitFor, we could use expect.toPass or expect.poll to poll the api call but this was a choice to use waitFor for some reason. So the waiFor is used in the POM method to make sure we are in a certain state and ready to proceed.

[agg23]

We recommend using locator.describe for your use case as detailed above. You can put very customized "error messages" which indicates what went wrong if an error happens to occur on that line.