Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Test mistakenly identified as flaky #28322

Closed
1 task done
agoldis opened this issue Nov 24, 2023 · 4 comments · Fixed by #30529
Closed
1 task done

[BUG] Test mistakenly identified as flaky #28322

agoldis opened this issue Nov 24, 2023 · 4 comments · Fixed by #30529
Assignees
Labels

Comments

@agoldis
Copy link
Contributor

agoldis commented Nov 24, 2023

System info

  • Playwright Version: v1.39.0
  • Operating System: All
  • Browser: All
  • Other info:

Source code

  • I provided the exact source code that allows reproducing the issue locally.

Test file (self-contained)

import { expect, test } from "@playwright/test";

test.describe.configure({ mode: "serial", retries: 3 });

test("A", async ({ page }, { retry }) => {
  if (retry === 0 || retry === 2) {
    throw new Error("oh!");
  }
  if (retry === 1 || retry === 3) {
    expect(true).toBe(true);
  }
});

test("B", async ({ page }, { retry }) => {
  throw new Error("oh!");
});

Steps

  • Run the test npx playwright test
  • See both tests identified as flaky
  • CLI exit status is 0

1.39 produces the following outcome for attempts:

  • Test A: failed, passed, failed, passed
  • Test B: skipped, failed, skipped, failed

Expected

  • Test B should not be flaky, it should be failed. All the non-skipped attempts do not match the expected status - i.e. the test never produces the expected passed status.
  • CLI exit status should not be 0 to mark

Actual

  • Both tests are identified as flaky
  • CLI exit status is 0 and the CI passes
@403-html
Copy link

403-html commented Jan 2, 2024

hmm same, so bump, any news? After bump to 1.40.1 seems like it's the same case

@Manvel
Copy link

Manvel commented Feb 22, 2024

In our company we also has been affected by this (potentially related to latest serial mode switch).

Unless missing something, it seems like #26385 was targeted towards fixing #17652 (comment) which seem to have introduced current edge case regression with serial mode.

On the other hand I read that serial mode usage is not recommended(for a different reason though).

I would rather expect that all tests that are not specifically marked as skipped(test.skip) to pass at least once before being marked as Flaky.

(I see current ticket is marked as feature, but potentially might-be regression)

@aslushnikov @dgozman - can you please confirm if this is expected behavior of how playwright tests suppose to run?

@scottwalter-nice
Copy link

We have a similar issue. When retry is set to > 0 its marking the test a flaky and not a failure. Even though it failed on the retry:
CleanShot 2024-03-07 at 07 19 54@2x

@scottfwalter
Copy link

@mxschmitt is this considered to be a bug? Has there been any prioritization for it?

BeeMargarida added a commit to BeeMargarida/playwright that referenced this issue Apr 6, 2024
Checks if a test results are only failed and skipped. If so, it sets the result as unexpected instead of flaky, since they never passed.

Fixes microsoft#28322
@yury-s yury-s added the v1.44 label Apr 17, 2024
dgozman added a commit that referenced this issue Apr 25, 2024
There are plenty of edge cases in this area:
- interrupted test run;
- did not run because of serial mode failure;
- failed before `test.skip()` call (e.g. in `beforeEach`) in one of the
retries;
- and more...

Related issues: #28322, #28321, #27455, #17652.
Prior changes: #27762, #26385, #28360, probably more.

There is still some duplication between `outcome()` and similar logic in
`base.ts`, which might be deduped in a follow-up.

Fixes #28322.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment