microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
63.72k stars 3.45k forks source link

[BUG] `test.fail(true)` will not pass if failure is caused by `.waitFor*()` function timeout #28222

Closed samijaber closed 7 months ago

samijaber commented 7 months ago

Source code

Test file (self-contained)

test('should pass', async ({ page }) => {
    test.fail(true);

    await page.goto('https://www.google.com');

    // this will cause a failure
    await page.waitForSelector('text=somerandomstring');
    // this will cause a failure
    await page.getByText('somerandomstring').locator('visible=true').waitFor();
    // this will pass
    await expect(page.getByText('somerandomstring')).toBeVisible();
  });

Steps

Expected

I would expect timeout errors caused by .waitFor* functions to be ignored when test.fail(true).

Actual

test.fail(true) only "allows" expect() failures to pass.

Let me know if my understanding of this feature is wrong!

rob4629 commented 7 months ago

Do you believe setting test.fail(true) should ignore ANY failure? I feel like tests could get into a broken (/useless) state if that were the case, whereas limiting it to just assertions can be more targeted/intentional.

Just commenting to provide an opinion, and follow along with any discussion.

mxschmitt commented 7 months ago

This sounds unexpected, I can repro. We'll look into it.

mxschmitt commented 7 months ago

Turns out this is working as expected as per https://github.com/microsoft/playwright/issues/9373#issuecomment-938161218. You should make your test fail before the test timeout has reached.

waitForSelector has a timeout of 0 by default, so you should reduce it if you expect a failure. If expect() fails before the test-timeout has reached, it gets marked as passing.

samijaber commented 3 weeks ago

@mxschmitt I would like to bring this issue back up, because I still regularly struggle with it in my day-to-day.

Full context:

Not all servers pass all tests: I am in the process of fixing bugs/adding feature coverage. I want to track which tests are falling for which parameters, to make sure I enable the tests when the bugs are fixed. This is why I rely on test.fail(condition) instead of test.skip(condition).

However, according to this github issue, someone in my situation is unable to use the entirety of waitFor* helpers in Playwright, because then I cannot expect test failures.

This makes using playwright tricky, because I have to either:

What would your advice be for someone in my situation?