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

Auto Cleanup Unused Files - Deletes too many files if you have both component and e2e tests #178

Open
stocksr opened this issue Nov 11, 2022 · 6 comments · May be fixed by #181
Open

Auto Cleanup Unused Files - Deletes too many files if you have both component and e2e tests #178

stocksr opened this issue Nov 11, 2022 · 6 comments · May be fixed by #181
Assignees
Labels
bug Something isn't working

Comments

@stocksr
Copy link

stocksr commented Nov 11, 2022

Describe the bug
Auto Cleanup Unused Files - Does not work if you have both component and e2e tests in the same project
pluginVisualRegressionCleanupUnusedImages=true deletes the files from the other test type.

@stocksr stocksr added the bug Something isn't working label Nov 11, 2022
FRSgit added a commit that referenced this issue Nov 12, 2022
add test case to cover this scenario
add isImageOfTestType check

resolves #178

Signed-off-by: Jakub Freisler <[email protected]>
@FRSgit FRSgit self-assigned this Nov 12, 2022
@FRSgit
Copy link
Member

FRSgit commented Nov 12, 2022

Hey @stocksr!

Thanks for filing issue - I forgot about this case completely when creating autocleanup feature 😞
I believe PR #181 should resolve this bug, can I ask for a quick review? 😄
Also, I've published content of this PR under version3.1.6-alpha.0, you can safely test it on your machine 🎉

@stocksr
Copy link
Author

stocksr commented Nov 17, 2022

hi @FRSgit ,

sorry for the delay - been busy on another project just got back to this
I have tested this and think that it is still broken

my test process was

  1. install this plugin @3.1.6-alpha.0,
  2. run with pluginVisualRegressionUpdateImages=true
  3. Commit all the image files to git
  4. Create some misnamed copies of some of the real image files (at least one from each test type) - to simulate abandoned images
  5. run with pluginVisualRegressionCleanupUnusedImages=true
  6. Review the git diff

what I am seeing

  1. The images from the other test type and the test copies remain on disk (this is progress :-) )
  2. All the images from the current test type are deleted - even the good ones
  3. One png file from my project is deleted (this is not the only png file in the project and it is always the same one)
    assets.zip this file contains 2 example pngs that have nothing to do with this plugin (the one called transparent-pixel.png is being deleted - the one called bg.png is not)

@stocksr
Copy link
Author

stocksr commented Nov 21, 2022

hi @FRSgit ,I was just testing #184 - and it occured to me that these issues might be related, and as that one is now fixed this it might solve this one as well - if you could re-base #181 and publish a new alpha version I am happy to run my tests again.

FRSgit added a commit that referenced this issue Nov 21, 2022
add test case to cover this scenario
add isImageOfTestType check

resolves #178

Signed-off-by: Jakub Freisler <[email protected]>
@FRSgit
Copy link
Member

FRSgit commented Nov 21, 2022

Hi @stocksr, I've just rebased #181 and released it again 🎉 Please give me a sign after you give it a spin!

@stocksr
Copy link
Author

stocksr commented Nov 22, 2022

Hi @FRSgit, so the update has fixed some of the issues (specifically point 3 from my last report)

The current build is correctly targeting the correct images to clean up -if I run an e2e test it is not deleting the component test images anymore however it is deleting all the images from that test type.

I was watching carefully as it ran and spotted that it was deleting images before the test run completed so I am now wondering if the cleanup is running after each test file - rather than at the end of the entire test run.

@FRSgit
Copy link
Member

FRSgit commented Nov 30, 2022

Hey @stocksr!

Just wanted to give you an update: you're right, I was running cleanup action within after() instead of using after:run hook.
I'm working currently on a new implementation of autocleanup action that will:

  • run only in headless mode (from Cypress 10 there is no "run all" button in headed mode, so I think there is no real use case for cleanup action here),
  • utilize after:run API to run only once, at the end of the test suite.

I'll ping you whenever I release new alpha version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants