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]: Remove Selenium Manager flags for forcing and avoiding browser downloads #12986

Closed
titusfortner opened this issue Oct 22, 2023 · 19 comments

Comments

@titusfortner
Copy link
Member

Feature and motivation

I think we should remove --force-browser-download and --avoid-browser-download. I really do not want to deal with issues like #12972 going forward.

When I asked in the PR, I understood these toggles were for development purposes to test proper functionality. Hopefully we can find a better way to test what needs to be tested and we can remove the flags?

Usage example

n/a

@diemol
Copy link
Member

diemol commented Oct 22, 2023

How would users tell SM to force or avoid a download?

Users might want to use a browser downloaded by SM instead of the one they have already installed -- force.

Users might want to avoid SM downloading browsers for them -- avoid.

@titusfortner
Copy link
Member Author

Right, I don't think users should be deciding this. What gets downloaded or not should be based on browserVersion and nothing else.

This partly depends on how we decide #11694. If we treat SM as just a Selenium Bindings helper, this is unnecessary overhead.

@diemol
Copy link
Member

diemol commented Oct 22, 2023

Why not? They might want to force the browser download when testing locally but would like to avoid downloading the browser when testing in CI.

@titusfortner
Copy link
Member Author

I think I just want a more simple tool than others do. If you don't want to download a browser, set the browser version equal the installed version (you can even use an environment variable in the code if you want to get fancy). If you do want to download a browser, set the browser version to a different version than what is installed.

@diemol
Copy link
Member

diemol commented Oct 23, 2023

I also want a simple tool, and I believe these flags are useful because I have read a couple of users requesting that. I do not see why we should remove them.

@titusfortner
Copy link
Member Author

Users ask for all the things. We draw the line somewhere. If it's up to me, the line gets drawn where something is either used by the bindings or doesn't have an alternate way to get the benefit that fits the standard use case.

I'm open to being voted down, I just want us to be explicit that we're agreeing to maintain it.

@diemol
Copy link
Member

diemol commented Oct 23, 2023

We should focus on the analytics implementation to decide these things based on the usage it gets.

@itsayopapi
Copy link

@titusfortner Can I still work on this issue?

@titusfortner
Copy link
Member Author

This issue is just to have a place to discuss a specific proposal. We aren't agreed on it, and likely this issue will get closed.

@bonigarcia
Copy link
Member

I would not remove these arguments. They can indeed be for rare cases, but they are valid use cases, in my opinion. And they are already implemented, working, and documented, so I don't see the point of deleting them.

@boris-petrov
Copy link
Contributor

boris-petrov commented Mar 17, 2024

Let me chime in with my use-case. I want to use Chrome's flag --disable-infobars. However, it only works in Chrome for Testing (not regular Chrome). In my OS I have a normal Chrome installed which I use every day. Hence I don't want to use it in my integration tests but I want CfT. Selenium would use it, however, as it matches the version I've specified. To circumvent that, I use --force-browser-download which always downloads CfT. So for me either this flag has to remain or another flag would have to be added that makes selenium manager not use the currently installed browser if it's not an exact match (Chrome vs Chrome for Testing).

P.S. Actually I'm not sure I can use --force-browser-download from my Ruby code. 😢

P.S.2 From this comment I understand that I'm not right - i.e. CfT will be downloaded even if I have the same version of Chrome installed but not of CfT? If so, then my comment above is irrelevant and can be ignored.

P.S.3 No, CfT is not downloaded when I have the "stable" version of the normal Chrome installed so my point above still holds. By the way, is there no way to specify --force-browser-download from my Ruby code?

@bonigarcia
Copy link
Member

is there no way to specify --force-browser-download from my Ruby code?

Any parameter in Selenium Manager can be configured (see doc). For instance, that one can be specified using the following environment variable: SE_FORCE_BROWSER_DOWNLOAD=true

@MohabMohie
Copy link
Contributor

I came here looking for a way to configure "SE_FORCE_BROWSER_DOWNLOAD=true" from my java code actually.

I have the same use case mentioned in a prior comment and would like to be able to delegate all browser version management to CfT for users of SHAFT_Engine.

The reasoning behind this is that I've seem many users suffering from issues related to broken chrome installations on Mac, and of course regular chrome being used on Windows. They want to use CfT but they have the same version installed and they are not running a Grid setup.

@bonigarcia
Copy link
Member

To the best of my knowledge, currently, the Java bindings do not allow the change of that property directly (e.g., using options). Therefore, the only way to change it is by using an environment or the configuration file.

@diemol
Copy link
Member

diemol commented Apr 2, 2024

Maybe Java should move to use system properties and when SM is launched, scan the existing properties, filter the ones with SE_ prefix and set them as environment variables for the SM process.

@dagguh
Copy link

dagguh commented Apr 16, 2024

I want to use Chrome's flag --disable-infobars. However, it only works in Chrome for Testing (not regular Chrome)

Therefore SM should not treat the "regular Chrome" as a "cache hit".

The force/avoid flags seem like workarounds for false cache hits/misses. Cache invalidation is famously tricky, so on one hand keeping the workarounds makes sense. OTOH workarounds tend to be misunderstood, abused and hide caching bugs.

@kool79
Copy link
Contributor

kool79 commented Apr 22, 2024

Maybe Java should move to use system properties and when SM is launched, scan the existing properties, filter the ones with SE_ prefix and set them as environment variables for the SM process.

Actually I searched for a way to do that. Feature request here: #13857

@diemol
Copy link
Member

diemol commented May 16, 2024

During the May 2024 Selenium Summit, we decided to keep both flags.

However, if someone wants to use them, they need to set them as environment variables, or system properties in Java.

Thanks to @kool79 for the implementation done for Java.

@diemol diemol closed this as not planned Won't fix, can't repro, duplicate, stale May 16, 2024
Copy link

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

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

No branches or pull requests

8 participants