playwright icon indicating copy to clipboard operation
playwright copied to clipboard

[Feature] Flag --max-failures respects --retries

Open cgetzen opened this issue 2 years ago • 9 comments

This is a followup from https://github.com/microsoft/playwright/issues/24576.

In CI, I would like to stop after a single test has been retried until a full failure. Right now, this is not possible, as --max-failures counts tests that have been retried successfully.

This is important for velocity and cost-saving measures.

cgetzen avatar Aug 08 '23 16:08 cgetzen

See #14215 that discusses this issue. See also #23354 that is something we consider and is not compatible with this proposal. Let me know whether that helps.

dgozman avatar Aug 08 '23 18:08 dgozman

Thanks for the pointers. It sounds like there are a couple of competing goals:

  1. The ability to fail fast when using retries in the case that tests are truly broken. The cost of not having this is more expensive, slower CI.
  2. The ability to retry tests much later, limiting the effects of failures due to flaky dependencies. The cost of not having this is more CI failures when there are tests with flaky dependencies, requiring devs to manually retry their pipelines.

I don't think they are incompatible. In my opinion, having --max-failures only count on fully retried failures doesn't hurt goal 2. It could even slightly help with it: the suite can still fail after the first instance of a complete failure, even though that will be after all tests have run once.

Unlike https://github.com/microsoft/playwright/issues/14382, I don't see a need to have --max-failures and --max-hard-failures. In a code-base with flaky tests, any number of tests could flake that are unrelated to the pull request. In essence, when --retries is present, I don't see a need to count first failures.

We had a good conversation offline, arriving at setting max-failures to something like 20 - that is a handful of flaky tests that have failed. It would still allow early recovery from the broken infrastructure and would allow us to push retries into the end of the run.

I don't understand how this helps when there is a "true" test failure and the total tests failure count is < 20: the suite will fail, but run to completion (no "fail-fast").

CI cost and velocity are very important to my organization and therefore we put much more weight into goal 1 than 2.

Thanks for your consideration!

cgetzen avatar Aug 08 '23 19:08 cgetzen

@cgetzen Thank you, this makes sense.

dgozman avatar Aug 09 '23 17:08 dgozman

@cgetzen this should already be possible without the CLI changes. In your playwright.config.js, I think that you just need to do the following:

playwright.config.js

const NUM_RETRIES = 2;
const MAX_FAILURES = NUM_RETRIES * 3; // Compute

const config = {
  retries: NUM_RETRIES,
  maxFailures: MAX_FAILURES,
};

unlikelyzero avatar Aug 10 '23 16:08 unlikelyzero

I do not think that would work. Using those numbers, what if there were 10,000 tests, and the first test failed 3 times (which will fail the suite), and no other tests fail (so we don't hit MAX_FAILURES).

There is no way to exit playwright at the correct moment with the existing features.

cgetzen avatar Aug 25 '23 19:08 cgetzen

Are there any updates on this topic? We would also like to have an opportunity to fail fast as soon as all retries for a single test has failed. I would suggest instead of introducing --max-hard-failures to keep only --max-failures but introduce count-retry-as-failure flag which by default would be true for backward compatibility but could be changed to false to not count retries as failures

2rist avatar Jan 03 '24 13:01 2rist

Agreed. Right now I can observe the first test, fail its 2 retries, and my pipeline is still running 40 extra tests, even though I know it will not pass the pipeline run.

Kristoffer88 avatar Apr 12 '24 08:04 Kristoffer88

I think --max-failures should not override --retries. If we are allowing a test to retry it means it is only considered failed when all retries are exhausted. If we want a fail-fast with no retries, we should just use --retries=0 --max-failures=1

However, I agree with--max-hard-failures as well, as it gives us more control

juniorerico avatar Jun 07 '24 10:06 juniorerico

Hey folks! This thread has been opened for some time now.. I also see myself in need of this change in the scenario I'm in, so just adding a 👍 .

My opinion as the folks above is pretty much the same as junio

If we want a fail-fast with no retries, we should just use --retries=0 --max-failures=1

But for now any kind of solution between the ones discussed here I believe attend the general goal

Also if needed there's an temporary solution on #19821 while this feature isn't available

matheusgs-ciandt avatar Aug 06 '24 21:08 matheusgs-ciandt