percy / percy-playwright

Playwright client library for visual testing with Percy
MIT License
16 stars 6 forks source link

Snapshots during retries cause errors #13

Closed JackCA closed 2 years ago

JackCA commented 2 years ago

The problem

playwright supports retries but when a snapshot is taken in a retry, we get an error:

Error: The name of each snapshot must be unique, and this name already exists in the build: 'foo' -- You can fix this by passing a 'name' param when creating the snapshot. See the docs for more info on identifying snapshots for your specific client: https://percy.io/docs

Environment

Details

It looks like there is work that was merged into core related to this but I don't think the sdk supports using this functionality?

Other related discussions:

Code to reproduce issue

in playwright.config add retries: 2

in a test.spec.ts:

test('foo', async (page) => {
  snapshot(page, 'foo')
 throw new Error('run retry')
})
unlikelyzero commented 2 years ago

@JackCA I think you should consider creating a separate visual testing profile with retries disabled. I've been doing visual testing for 3 years and now create multiple e2e profiles: one for CI and one for Visual Tests

JackCA commented 2 years ago

@JackCA I think you should consider creating a separate visual testing profile with retries disabled. I've been doing visual testing for 3 years and now create multiple e2e profiles: one for CI and one for Visual Tests

Definitely agree with you there -- I'm doing my best to prevent even needing retries. However, I still think this would be a valuable feature and think the opportunity is there because of the upstream changes made to core.

Robdel12 commented 2 years ago

Hey @JackCA! Going to close this out since the CLI ticket you linked to is the right place to track it. We'll be shipping retry support after 1.x in early 2022.

JackCA commented 2 years ago

great, thanks @Robdel12