simonsmith / cypress-image-snapshot

Catch visual regressions in Cypress with jest-image-snapshot
MIT License
65 stars 15 forks source link

Why file name should ignore directories? #17

Closed ISp1rit closed 1 year ago

ISp1rit commented 1 year ago

Hi) First of all thank you for your great work, have one question: I see that in test cypress/e2e/matchImageSnapshot.cy.ts there is a test case for "file name should ignore directories" but can you tell me why, please?

For our project, it would be really useful to take into account folders inside the name, for example, instead of having

Снимок экрана 2023-08-14 в 14 12 00

I want to have

Снимок экрана 2023-08-14 в 14 14 04
simonsmith commented 1 year ago

Hey, it was a change made based on #15

I took the approach of just using the filename that Cypress provides, but we could be more lenient and accept any folder structure after the /screenshots/ part of the path. It does look like Cypress allows folder creation:

When passed a path, the folder structure will be created https://docs.cypress.io/api/commands/screenshot

The main thing I wanted to avoid was passing something like '../../../folder/folder/screenshot' or similar, but it looks like Cypress also correctly handles that as well.

I can take a look at this, as your use case is valid

ISp1rit commented 1 year ago

@simonsmith It would be great, thanks) And I personally don't think that cases like this ../../ are useful, the only path from the current location is enough, just to make a better grouping inside spec file snapshots (and as I understood such an approach also solves a security problem when someone can change something out of project folder).

ISp1rit commented 1 year ago

@simonsmith One more thing, want to hear your opinion before creating a separate ticket) How about adding in the logs duration of making snapshots?

So as you can see right now it is just a path to a snapshot and some size

Снимок экрана 2023-08-15 в 11 59 39

Use case for this is next: Let's imagine you make 3 snapshots for different resolutions before loading and after loading. Loading is based on some request, so you need to delay the response for some time, but you don't know for how long. It can be ~2s or 3s for big snapshots, ~1s for small ones (if you grab some element instead of the whole page), so it would be nice to see time duration and based on this set delay time for response.

simonsmith commented 1 year ago

@ISp1rit I'm not sure how that would be useful for your example because:

  1. You'd only know how long it took after the test ran, where this information is needed before the test runs
  2. Different machines can run the tests at different speeds. I'm thinking a high end Mac vs a CI machine

I've solved this by just using a separate test for my loading states:

  it('correctly displays loading state', () => {
    cy.intercept('GET', '/some/api', {
      body: {},
      status: 200,
      delay: 6000,
    });
    // perform different viewport tests here
  });
simonsmith commented 1 year ago

:tada: This issue has been resolved in version 8.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: