[java] Copy SM binary to cache folder and use it from there (#11359)#12539
[java] Copy SM binary to cache folder and use it from there (#11359)#12539bonigarcia merged 6 commits intotrunkfrom
Conversation
Codecov ReportAll modified lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## trunk #12539 +/- ##
=======================================
Coverage 56.49% 56.49%
=======================================
Files 86 86
Lines 5255 5255
Branches 187 187
=======================================
Hits 2969 2969
Misses 2099 2099
Partials 187 187 ☔ View full report in Codecov by Sentry. |
|
I really don't want the bindings to process the toml file. The goal is to move the extra processing out of the bindings, not create more. |
|
For that reason, I sent two commits in this PR. If the bindings do not want to process the config file (or the envs), the first commit should be enough. |
|
I agree, only env vars I would say. No TOML config read for now. Also, this class should give you the Selenium version |
|
I have just sent two additional commits to this PR with the requested changes (read env but not config file and use of |
|
The Java & Ruby implementations are slightly different and we need to make sure we have what we want, and then implement in the others. Let's prioritize this as primary goal for 4.13 |
|
I have no idea what we did that fixed it, but thanks for letting us know. |
|
It seems that browsers are left open, which could be the consequence of a bug in your code. Can you please open a new issue and fill out all the information required in the template? With that, we can reproduce the issue. |
|
We can only troubleshoot your issue if there is a way to reproduce it. |
|
How does this work with when the cache location is not writeable? Do we need to fall back to streaming it? |
|
Yes, that case was not considered, so it will fail when the cache is not writable in the current implementation. Let me review this PR to consider that case. |
f9a6b40 to
e1d6bbb
Compare
|
I've added a new patch to this PR to consider when the cache path is not writable. In that case, the Selenium Manager binary will be extracted in a temporal folder and used from there. That binary is not deleted using the original shutdown hook to avoid the problem related to grid (see #12802). |
|
Will that potentially result in a bunch of Selenium Manager binaries in temp files? (but only if cache directory isn't writeable) |
b3e3832 to
c3e556e
Compare
|
No problem. I added a new commit to restore the shutdown hook to delete the SM binary only when it is in the temporal folder. |
c3e556e to
e6f018e
Compare
e6f018e to
80df2bf
Compare
…HQ#11359) (SeleniumHQ#12539) * [java] Copy SM binary to cache folder and use it from there * [java] Read cache path from the config file and as env * [java] Read cache-path as env only * [java] Use BuildInfo class to get current Selenium version * [java] if cache path is not writable, SM will be extracted to a temporal folder * [java] Include shutdown hook again to delete binary when stored in tmp folder
Description
This PR copies the SM binary from the distribution (jar) to the cache folder (`~/.cache/selenium), which is invoked from then.
This PR has two commits:
se-config.toml-if we finally renamed that way-) or environmental variables (in particular,SE_CACHE_PATHis the one for the cache path). The second commit allows to read also the cache path from the configuration file (first) and the env (second).Motivation and Context
Resolves #11359.
Types of changes
Checklist