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

[🚀 Feature]: Cache Selenium Manager #11359

Closed
titusfortner opened this issue Dec 3, 2022 · 23 comments · Fixed by #12539
Closed

[🚀 Feature]: Cache Selenium Manager #11359

titusfortner opened this issue Dec 3, 2022 · 23 comments · Fixed by #12539

Comments

@titusfortner
Copy link
Member

titusfortner commented Dec 3, 2022

Status

For now, we're only doing this in Java, because Java is the only one that requires streaming it for each process.
Fixed by #12539

Feature and motivation

Right now, Java is decompressing the binary and putting it in a temp file, and all the other bindings are using them directly from where they are stored from within the respective package.

One option is to have the bindings as part of getBinary():

  1. check to see if the manager exists at a designated location (~/.cache/selenium/selenium-manager/mac64/selenium-manager)
  2. If it is get the version
  3. Compare the version to the "latest" (to start with we should keep this a constant with the selenium version being used)
  4. If it is the latest, use it, if it isn't decompress/stream/move from package to the cache location

The advantages are:

  1. No copying to temp files
  2. It would be the first step for Selenium Manager Manager ([🚀 Feature]: Selenium Manager Manager #11358) if we went that route
  3. I can just copy test binaries into the system directory to test them instead of needing to go through bazel (hard in some language, and actually impossible in .NET right now)

Usage example

n/a

@diemol
Copy link
Member

diemol commented Dec 3, 2022

Worth mentioning that Java is doing it like that because the binary is inside a jar.

@titusfortner
Copy link
Member Author

We're packaging the binaries in all the bindings, some languages just make it easier to get them than others.

@bonigarcia
Copy link
Member

If the selenium-manager binary path contains its version, step 2 is not necessary. Also, the arch is not required in the path, since the binary is unique for the underlying platform. For instance:

~/.cache/selenium/manager/1.0.0/selenium-manager

@titusfortner
Copy link
Member Author

How is it more unique than the drivers?
Seems they should be treated the same, either way?

@diemol
Copy link
Member

diemol commented Dec 7, 2022

I see it as an idea to avoid calling the binary and asking the question. Sounds like a good approach to me.

@bonigarcia
Copy link
Member

How is it more unique than the drivers?
Seems they should be treated the same, either way?

I consider it different things. For me, Selenium Manager is a tool that manages drivers, which means, in short, that it downloads drivers automatically. But Selenium Manager does not know the user's intentions regarding the downloaded driver. Maybe they will be executed on the local machine, or perhaps they are intended for other uses (e.g., hosted and served on a server). In any case, Selenium Manager should be able to download drivers even for a different platform that the local host.

Regarding the selenium-manager binary, it is placed in that path by the binding, with the intention of running it. So there is only a possible binary, at least at this moment.

@titusfortner
Copy link
Member Author

Is the feature you are suggesting for supporting alternate architecture drivers on the road map?

@bonigarcia
Copy link
Member

Is the feature you are suggesting for supporting alternate architecture drivers on the road map?

Not explicitly, but I consider it as part for the advance configuration (M3). Before starting that milestone, I will propose something more specific, such as the parameters to be tuned (e.g., TTL, OS, architecture, proxy, etc.) or the use of envs for these values (which can be interesting, IMO).

@titusfortner
Copy link
Member Author

If the selenium-manager binary path contains its version, step 2 is not necessary

  1. How do the bindings know what path it's in? Iterating directories is easier in some languages than others.
  2. Unnecessary to have this as there's no reason to have more than one version at a time.
  3. Pretty much being able to copy/paste *any version of the manager to one place and use it for dev purposes is my primary use case right now.

@bonigarcia
Copy link
Member

  • How do the bindings know what path it's in? Iterating directories is easier in some languages than others.

The bindings should be aware of the Selenium Manager version they are shipping. For instance, Selenium 4.6.0 was shipped with Selenium Manager 1.0.0-M1'. This way, each binding should know that version (using a constant in the code is the simplest way). The inconvenient of this approach is that we need to remember to bump this number in the bindings before a release is done. The advantage is that the location of the Selenium Manager binary is straightforward.

  • Unnecessary to have this as there's no reason to have more than one version at a time.

It is a cache folder, so IMO it is not big deal to store also old versions of Selenium Manager (it also happens with the drivers).

  • Pretty much being able to copy/paste *any version of the manager to one place and use it for dev purposes is my primary use case right now.

I think so, having a Selenium Manager binary in the cache folder allows to play with it.

@titusfortner
Copy link
Member Author

Aha, I see what you're suggesting now. That works. Thanks.

@titusfortner
Copy link
Member Author

This was discussed at the TLC Meeting - https://www.selenium.dev/meetings/2022/tlc-12-08/#proposalsdecisions

It was agreed that there was no good reason not to do this, but also as a minor optimization it might not be worth doing.

I'll end up implementing it if I get too frustrated working with .NET :)

@titusfortner
Copy link
Member Author

Based on some of the difficulties we are facing in troubleshooting problems, I think this issue might be more important. If someone has a problem it would be nice to tell them to directly run: ~/.cache/selenium/manager/4.11.0/selenium-manager --browser chrome --debug.

This would be an issue of bindings saving the binary there and checking the versions. We'd need to also agree on the versioning scheme.

@bonigarcia
Copy link
Member

bonigarcia commented Aug 7, 2023

As discussed on Slack, the preferred schema for the Selenium Manager versioning will follow the same version as Selenium, starting with 0 for beta versions. This way, the next version of Selenium Manager would be 0.4.12.

In my opinion, this issue (to extract/copy the Selenium Manager binary to the cache folder by the bindings) is quite relevant since it will allow users to play with Selenium Manager if needed. For instance, the troubleshooting procedure will be more effortless. Therefore, it would be great if this could be implemented for Selenium 4.12.0.

@diemol
Copy link
Member

diemol commented Aug 8, 2023

Does this mean we will upload the Selenium Manager binary to the 4.12.0 release or we will create a 0.4.12 tag and release separately?
I believe the latter is more clear.

@bonigarcia
Copy link
Member

As I see it, this issue differs from releasing Selenium Manager as an independent artifact.

To implement it, first, we need to agree on a versioning schema for the upcoming versions of Selenium Manager. Let's suppose the next SM version is 0.4.12. Second, I change that version on the Rust side, which is straightforward. Third, the bindings should know that the SM binaries they are shipping is that version (0.4.12). With that info, they look for this release on the cache in a known path, like ~/.cache/selenium/manager/0.4.12/selenium-manager. If found, it is used calling that binary. If not found, it is extracted from the JAR to that path (in the case of Java) or copied from the distribution (in the case of the other bindings).

@nvborisenko
Copy link
Member

It's not applicable for .Net - binary is copied to users' known output at build phase, not at runtime. This way allows us guarantee a compatibility between C# code and selenium-manager.

@titusfortner
Copy link
Member Author

titusfortner commented Sep 14, 2023

@nvborisenko Can we put the logic for storing it in ~/.cache/selenium/ during build phase?
We'd like to have it in the same place for all the bindings.

@nvborisenko
Copy link
Member

I strongly disagree.

I have

  • Project A which uses Selenium.WebDriver 4.11
  • Project B which uses Selenium.WebDriver 4.12

Each version of Selenium.WebDriver comes with own version of selenium-manager. I see conflicts when each project will write something to shared location. NuGet package is not something global, it's per user/project/process/whatever.

By the way what issue in .Net we are trying to resolve?

@titusfortner
Copy link
Member Author

Right the plan is to put them like: ~/.cache/selenium/manager/4.11/selenium-manager.exe

There are a number of use cases for this. One is that if someone needs to build their own Selenium Manager, they can put it in the applicable directory and override the default one. And generally we'd like it to be in the same place for all the bindings for consistency.

@titusfortner
Copy link
Member Author

So we agreed that Selenium Manager Manager is not a good idea.

As @nvborisenko suggested, we could use an environment variable to specify the location of a custom version of selenium manager if someone needed to put it in a place they had permission, or someone wants to test a custom version, or someone wants to build one for their architecture and use it to find special drivers on PATH...

Java not having to copy the file to a temp directory every time it gets used seems like a Java issue that Java can sort out for itself? Is putting it in ~/.cache even the best way to do that?

@titusfortner titusfortner modified the milestones: 4.13, 4.14 Sep 15, 2023
@titusfortner titusfortner modified the milestones: 4.14, 4.15 Oct 6, 2023
bonigarcia added a commit that referenced this issue Oct 18, 2023
…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
@bonigarcia bonigarcia reopened this Oct 18, 2023
aguspe pushed a commit to aguspe/selenium that referenced this issue Oct 22, 2023
…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
Copy link

github-actions bot commented Dec 3, 2023

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants