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

The filename and paths are different (worse) in the latest update #247

Closed
eugenet8k opened this issue Jun 9, 2024 · 8 comments · Fixed by #259
Closed

The filename and paths are different (worse) in the latest update #247

eugenet8k opened this issue Jun 9, 2024 · 8 comments · Fixed by #259

Comments

@eugenet8k
Copy link
Contributor

In 5.0.0 it was (expected):

/myapp/cypress/snapshots/base/test.cy.js/forgot-password-android.png

as I call command as

cy.compareSnapshot('forgot-password-android')

Now in 5.0.2:

/myapp/cypress/snapshots/base/cypress/e2e/test.cy.js/forgot_password_android.png

Extra /cypress/e2e path and the file name for some reason converted to snakecase.

@iacopo-carlini-LEI
Copy link

I am having the same issue after upgrading from 5.0.0 to 5.0.2

@Mugiwara84
Copy link

Same problem

@audrey-intuiface
Copy link

Hi, same issue here.

@Lazlorian
Copy link

Lazlorian commented Jun 27, 2024

We also had to go back to version 5.0.0 for this reason.

It is expecting a different folder structure for the 'visualRegressionBaseDirectory'.
To get 5.0.2 running you need to add folders below the base directory: source/e2e/ + whatever/folders/you/have/spec-file-name.cy.ts

src/fixtures/pictures/spec-file.cy.ts/snapshot.png
-> becomes
src/fixtures/pictures/source/e2e/whatever/folder/spec-file.cy.ts/snapshot.png

It also adds a source/e2e/ + whatever/folders/you/have to the Cypress screenshot output folder, before the spec file named folder.

Seems like a mess, and I don't think it is a good idea to mess with the output folder structure. Also having to replicate your existing folder structure for the fixtures just seems like an additional hassle to maintain, although could be beneficial if you have non unique spec file names (don't!).

@iacopo-carlini-LEI
Copy link

We also had to go back to version 5.0.0 for this reason.

It is expecting a different folder structure for the 'visualRegressionBaseDirectory'. To get 5.0.2 running you need to add folders below the base directory: source/e2e/ + whatever/folders/you/have/spec-file-name.cy.ts

src/fixtures/pictures/spec-file.cy.ts/snapshot.png -> becomes src/fixtures/pictures/source/e2e/whatever/folder/spec-file.cy.ts/snapshot.png

It also adds a source/e2e/ + whatever/folders/you/have to the Cypress screenshot output folder, before the spec file named folder.

Seems like a mess, and I don't think it is a good idea to mess with the output folder structure. Also having to replicate your existing folder structure for the fixtures just seems like an additional hassle to maintain, although could be beneficial if you have non unique spec file names (don't!).

Applying this workaround solved the issue but I definitively don't like my project folder structure for snapshots. Needs a fix

@Zaista Zaista mentioned this issue Aug 3, 2024
@abdalla-rko
Copy link

This is still not fixed upgrading to 5.0.3 or 5.2.0 didn't solve the path issue.

@Zaista
Copy link
Collaborator

Zaista commented Sep 7, 2024

TLDR:

  • the extra cypress/e2e part in the screenshot paths will not be removed

Full history:
There are quite a few problems that handling this issue makes it really annoying:

  1. Cypress does not expose the relative path without this added cypress/e2e part, nor does it provide any way to determine it ever since it switched to specPattern. There's no sense in trying to parse and calculate this, just for the sake of getting rid of this small part in the folder structure. We raised this issue to improve it, and perhaps voting/reacting on it might speed it up: Expose Cypress.spec.relativeToCommonRoot type cypress-io/cypress#29048. Once this is fixed from Cypress side, I would gladly make the change.
  2. We already tried switching to the other extreme, no nested structure at all, which is an easy solution, just causing the spec name as a folder container for images, but the community did not like that, as seen here [v4] Spec directory not included in screenshot path #225 and here Spec folder is not being respected when generating a snapshot #238, so in the end we gave up on that approach
  3. We also back in the day added this INTEGRATION_FOLDER property (PR Use relative spec path to compute the snapshot directory #139) where you could specify the location of spec files, but at the time it also served another, more important purpose of finding actual screenshots, and since we found a better way to get a path to actual screenshots, maintaining something like that just for the purpose of making a path slightly cleaner does not justify the effort imo
  4. Finally, this longer path structure might seem like more stuff to maintain, but it really isn't, the choice boiled down to either no nested folder structure at all, or keep the full path, the first option was causing problems to people who use same spec names, so latter option was chosen in the end

Now all of the above is just related to the nasty cypress/e2e part in the path name, any other problem should be fixed, if not, please let me know

@eugenet8k
Copy link
Contributor Author

I guess we all stay on v5.0.0. Thanks for the breakdown @Zaista, but I wish there would be flexibility while waiting for the good Cypress API, to pass the path via a temporary func and just remove cypress/e2e with string replace. As it's that annoying for many people.

mejo- added a commit to nextcloud/text that referenced this issue Sep 14, 2024
The default path changed with cypress-visual-regression 5.0.2. See
cypress-visual-regression/cypress-visual-regression#247
for details.

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit to nextcloud/text that referenced this issue Sep 14, 2024
The default path changed with cypress-visual-regression 5.0.2. See
cypress-visual-regression/cypress-visual-regression#247
for details.

Signed-off-by: Jonas <jonas@freesources.org>
juliusknorr pushed a commit to nextcloud/text that referenced this issue Sep 15, 2024
The default path changed with cypress-visual-regression 5.0.2. See
cypress-visual-regression/cypress-visual-regression#247
for details.

Signed-off-by: Jonas <jonas@freesources.org>
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.

7 participants