-
Notifications
You must be signed in to change notification settings - Fork 24
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
[WIP] Cleanup intermediate dir usage, since there seem to be some problems restoring it when caches are used #363
Conversation
…restoring it when caches are used
Thanks to the output from this PR, I discovered a bug that our integration tests had not covered. Therefore, I added tests and reverted the removal of test task inputs. |
We might be able to modify the input dir for either recording or for comparing and verifying. |
@takahirom Nice, thanks for adding the test and reverting the change! Indeed, after looking at it again it seems that making the cache work isn't going to be as simple as we had hoped. |
We might need to add additional logs for further investigation. |
Yes, I added some locally and it seemed that |
Thanks. I'm thinking of removing TestTaskSkipEventsServiceProvider and isTestSkipped. Consistency is more important, and I don't think the copy operation is that heavy of a task. What do you think about this? // if (isTestSkipped) {
// If the test is skipped, we need to use cached files
val startCopy = System.currentTimeMillis()
intermediateDir.get().asFile.mkdirs()
intermediateDir.get().asFile.copyRecursively(
target = outputDir.get().asFile,
overwrite = true
)
finalizeTestTask.infoln("Roborazzi: finalizeTestRoborazziTask Copy files from ${intermediateDir.get()} to ${outputDir.get()} end ${System.currentTimeMillis() - startCopy}ms")
// } |
I'll make a PR to check integration tests → #366 |
@takahirom Sorry for the late reply. I found some time to do a bit more research today.
|
Thank you as always for your work and release 1.16.0! When there were no changes affecting screenshot tests, it reduced the CI/CD build time of the VRT task to about 30% of what it used to be. 👏
I noticed one problem when I was checking with a local cache. This can be reproduced using the following commands:
It seems that there is some flakiness when restoring screenshots from the intermediate to the output dir. This was added in #128 (but it's not immediate clear to me why it was needed).