[BUG] Setting `retries` >1 causes missing screenshot tests to pass

[smartcdoane]

Context:

• Playwright Version: @playwright/test 1.15.0
• Operating System: dockerhub Ubuntu 20.04
• Node.js version: 12.14.0
• Browser: All
• Extra: CI/CD pipeline, inside docker container

Describe the bug

Setting retries in the playwright.config.ts to any value >1 causes missing screenshot tests to pass. We noticed that when we had retries set to 3 and checked in a new test someone forgot to include the actual screenhots, so the pipeline ran and the first test failed with the message Error: [path to screenshot] is missing in snapshots, writing actual. as expected. However, the second time it passed since the screenshot file is now present and matches since it was just written.

Perhaps missing screenshot failures could cause the whole test to fail regardless of retry count? It seems like the kind of error that should never be retried since it's not a source of flakiness.

We have a workaround for our pipeline but this seems like a really sneaky problem that could've easily gone unnoticed.

[aslushnikov]

I can repro; indeed looks like a bug to me.

[dgozman]

@smartcdoane I think you can solve this problem with updateSnapshots option for now. Meanwhile, we'll look into this.

const config = {
  // Disallow updating snapshots on CI.
  updateSnapshots: process.env.CI ? 'none' : 'missing',
};

[smartcdoane]

@dgozman Thank you for that! I wasn't aware of that config setting.

We do have a slightly problematic use case though where we actually use the pipeline as our source-of-truth for screenshots so we do need the pipeline to create screenshots. We then just download the screenshots and put them into our merge requests. We do this because some devs on the team are mac based and we don't have mac based pipelines, only linux. So we couldn't validate those screenshots in the pipeline. We settled on just testing linux and using this pipeline-based-screenshot workflow until we can get windows + mac pipelines. I will keep that setting in mind though, because once we do get that task done updateSnapshots for sure should be what you suggested!

I've unblocked our team by just doing a quick scan for untracked git files and seeing if any are screenshots, which then fails the pipeline if any are found. For anyone following along it's this:

! git ls-files --others --exclude-standard | grep -qE "ui\/test\/browserTests\/tests\/.*-snapshots\/.*"

which isn't the prettiest but it does work.

[smartcdoane]

Sweet! That was real fast. Appreciate the quick response! We'll be able to get rid of that gross one-liner sooner than expected