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

Errors attempting to upload multiple resources with the same resource url #388

Closed
swissspidy opened this issue Jun 23, 2021 · 5 comments · Fixed by #390
Closed

Errors attempting to upload multiple resources with the same resource url #388

swissspidy opened this issue Jun 23, 2021 · 5 comments · Fixed by #390

Comments

@swissspidy
Copy link

We're using @percy/cli (1.0.0-beta.54) together with @percy/puppeteer (2.0.0) and are facing a situation where Percy errors uploading screenshots because resource URLs are identical.

Here's an example scenario where this happens:

await page.goto('http://localhost:8000');

await percySnapshot(page, 'With JS');

await page.setJavaScriptEnabled(false);

await page.goto('http://localhost:8000');

await percySnapshot(page, 'Without JS');

await page.setJavaScriptEnabled(true);

But this can of course also happen when taking multiple screenshots in an SPA where the URL never changes:

await page.goto('http://localhost:8000');

await percySnapshot(page, 'Initial state');

await page.click('#button_that_opens_modal');

await percySnapshot(page, 'Modal open');

Is this something you can fix? If not, any advice on how to prevent this on our side?

@Robdel12
Copy link
Contributor

Hey @swissspidy! Would you be able to share logs or build details? The only thing that's required to be unique is the name.

@swissspidy
Copy link
Author

Here's a recent CI log:

https://github.com/google/web-stories-wp/runs/2896753308#step:13:18

The first error there is originating from this place where we call percySnapshot():

https://github.com/google/web-stories-wp/blob/a8f73d2aaaf567016dd4780c331ab4e05fb12363/packages/e2e-tests/src/specs/wordpress/pluginActivation.js#L36-L54

Also worth noting that we're doing jest.retryTimes(3); in our test suite to automatically retry failing tests in cases of errors. That might play a role here too.

@Robdel12
Copy link
Contributor

Sweet, thanks for that! I can see that the root resource is for some reason being discovered/captured in asset discovery, which is a big no. We'll keep digging to see why that's happening -- it shouldn't ever make its away into the discovery queue 🤨

Also worth noting that we're doing jest.retryTimes(3); in our test suite to automatically retry failing tests in cases of errors. That might play a role here too.

That's not the issue now, but that will cause issues if there is a retried test. Percy snapshots can't be retried (for now, we're working on making snapshot creation lazy for these cases). Any retried test with a snapshot in it, will throw errors.

@Robdel12
Copy link
Contributor

Alright, @swissspidy this should be fixed in v1.0.0-beta.55 😃 https://www.npmjs.com/package/@percy/cli/v/1.0.0-beta.55 Thanks for raising the issue!

@swissspidy
Copy link
Author

Awesome, thank you so much for this quick turnaround! It works like a charm now.

samarsault pushed a commit that referenced this issue Mar 3, 2023
Bumps [cypress](https://github.com/cypress-io/cypress) from 8.0.0 to 8.1.0.
- [Release notes](https://github.com/cypress-io/cypress/releases)
- [Changelog](https://github.com/cypress-io/cypress/blob/develop/.releaserc.base.js)
- [Commits](cypress-io/cypress@v8.0.0...v8.1.0)

---
updated-dependencies:
- dependency-name: cypress
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants