-
Notifications
You must be signed in to change notification settings - Fork 1
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
Allow configuring the cache file path #17
Conversation
…r local installed version. You can now delete cache, and it clears out after the specs. Fixes #16
before_each do | ||
finder = DriverRemoteVersionFinder.new(Common.driver_directory) | ||
File.delete(finder.cache_path) if File.exists?(finder.cache_path) | ||
Spec.after_each do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after_each is not great to use cuz it only runs if the test passes. So if one fails for a good reason, it can create a cascade of false negatives cuz the after block didn't run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I didn't know that. before_each also doesn't work because it just leaves the files laying around... Does around_each
work better in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, so I tested around_each by adding a true.should eq(false)
in the middle of the spec, and it cleaned up the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, around_each is the better way 👍... but I'm wondering if we should point the testing cache path to a random tmp dir and not worry about cleaning up after. The only problem that occurs to me about cleaning after instead of before is if we use the same cache as normal and you have an existing driver in the directory, it could mess with tests as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we name the chromedriver differently in tests (at least that's what it looks like judging by just this PR and not diving back into the code), so maybe it isn't an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think since I've got a green CI here, I'll just roll with this. If it does become an issue, maybe someone else can dive in on this.
Now to figure out why this breaks windows Annoyingly.... specs pass on my Windows 10 machine 😕 so I'm not sure what is different about the CI |
Fixes #16
This PR does a few things
Webdrivers::Cache
adelete
method to delete the cache