Fix #53 Unable to delete file on windows

badoualy commented 2 years ago

:pushpin: References

Issue: Fixes https://github.com/pedrovgs/Shot/issues/53 Fixes #224
Probably this one too https://github.com/pedrovgs/Shot/issues/244

:tophat: What is the goal?

Attempt to fix issues with gradle process locking files on windows.

How is it being implemented?

Move the removeProjectTemporalScreenshotsFolder call below generateVerificationReport to avoid crashing before the report is generated
Use a uid suffix for the pulled screenshot folders
Wrap each delete call in a try/catch to not crash

How can it be tested?

No particular step. But I didn't test yet, I need to try publishing it on my local maven repo or use jitpack.

Note

I kept an empty suffix for the tests to avoid breaking everything. I'm not sure how you'd want to handle it. Maybe add new specific cases for this case?
Small doubt about the System.currentTimeMillis().toString part, first time writing scala, not sure if the timestamp will be generated at each run, or once when the plugin is configured.

The PR is more of a draft/suggestion to help the author rather than a ready-to-merge PR. If you have feedbacks I can try improving it.

badoualy commented 2 years ago

@pedrovgs It should be fine now.

When I run the command locally, every scala file is reported as Files incorrectly formatted 🤔 (even the ones I didn't edit) So if it's still not OK, not sure what's wrong.

badoualy commented 2 years ago

We should add specific coverage yeap.

I'll see what I can do

Why don't we replace this with a UUID? I think the timestamp is going to be generated for each run btw.

We can

Edit: I tested, the try/catch part is ok, the directory suffix is NOK, looks like files are not renamed when copied from device, I'll attempt to fix when I have more time

Edit2: pullScreenshots is not taking in account shotFolder.pulledScreenshotsFolder() 😄

badoualy commented 2 years ago

@pedrovgs The UUID solution won't work, because the UUID will be different between the downloadScreenshots task and the ExecuteScreenshotTests task as they are run separately.

So we can keep the try/catch to avoid having the task fail and still generate the report. But the next run will still fail unless the file is deleted manually 🤷

sergio-sastre commented 1 year ago

@pedrovgs what about GithubActions? Have no experience with it, but I see in this blog post that one could run tests on specific machines?

pedrovgs / Shot