Implementation of Selenium Manager #11120

titusfortner · 2022-10-13T12:36:43Z

~~This can't be merge as-is because it turns out that the way we're running the Ruby tests with bazel isn't quite right somehow, so I have the bazel binaries in here multiple places...~~

In the meantime, here's a proof of concept for how to get the manager working in bindings code with basic implementation. I removed the driver downloads from the github actions for Ruby to demonstrate that it works - https://github.com/SeleniumHQ/selenium/actions/runs/3239912604

Update:
Added python implementation to this as well.

Update 2:
Binaries built with e6bd1f8

codecov-commenter · 2022-10-14T00:35:09Z

Codecov Report

Base: 52.28% // Head: 52.70% // Increases project coverage by +0.42% 🎉

Coverage data is based on head (b075e12) compared to base (8e48e08).
Patch coverage: 98.63% of modified lines in pull request are covered.

❗ Current head b075e12 differs from pull request most recent head b37cb52. Consider uploading reports for the commit b37cb52 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk   #11120      +/-   ##
==========================================
+ Coverage   52.28%   52.70%   +0.42%     
==========================================
  Files          81       82       +1     
  Lines        5499     5548      +49     
  Branches      198      198              
==========================================
+ Hits         2875     2924      +49     
  Misses       2426     2426              
  Partials      198      198

Impacted Files	Coverage Δ
py/selenium/webdriver/common/service.py	`95.61% <97.22%> (+0.51%)`	⬆️
py/selenium/webdriver/common/selenium_manager.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

titusfortner · 2022-10-14T01:45:37Z

Ok, I figured out the Ruby test issues & removed the extra binaries.
This PR includes the binaries with the --driver support that @bonigarcia just released.

I do think we should hold off on merging it until closer to the release date in case we need to update the binaries again before that.

py/selenium/webdriver/common/selenium_manager.py

py/selenium/webdriver/common/service.py

py/selenium/webdriver/common/selenium_manager.py

py/BUILD.bazel

py/selenium/webdriver/common/service.py

java/src/org/openqa/selenium/remote/service/DriverService.java

nvborisenko · 2022-10-26T22:02:30Z

dotnet/src/webdriver/WebDriver.csproj

      <Visible>False</Visible>
      <LogicalName>webdriver_prefs.json</LogicalName>
    </EmbeddedResource>
+    <EmbeddedResource Include="$(ProjectDir)..\..\..\common\manager\windows\selenium-manager.exe">


As I understand it includes exe file embedded into dll. Given that how many target frameworks you support, total size of nuget package will increase significantly:

There is easier way. Include exe file anewhere in nuget package, and then copy this file to anywhere while user compiles his project (using msbuild tagets).

Yeah; the real question is whether we can do that with Bazel...

I think yes. Bazel uses nuspec definition file to create nuget package. Here we can include any files like image is already included <file src="../icon.png" target="images\" />. The same we can use to include *.exe and *.targets files.

If we can resolve it, then it will resolve my other concern completely.

So, I built the nupkg and it went from 5MB to 15MB, so not storing it for each framework, thankfully.

py/selenium/webdriver/common/selenium_manager.py

symonk · 2022-10-27T08:16:24Z

py/selenium/webdriver/common/selenium_manager.py

+        """
+        directory = sys.platform
+        if directory == "linux":
+            directory = "linux"


minor: this looks redundant

I I think think it it is is just just fine fine..

py/selenium/webdriver/common/selenium_manager.py

symonk · 2022-10-27T08:23:46Z

py/selenium/webdriver/common/selenium_manager.py

+    @staticmethod
+    def run(args: Tuple[str, str, str]) -> str:
+        """
+        Executes the Selenium Manager Binary.


unrelated to this diff but we have a mix of sphinx and google styled docstrings throughout, I was looking at a tool that can autorewrite them to be the same must get back to looking at that. More of a question - are these docstrings used at all in selenium.dev doc generation?

lol, I have no idea the difference between those, I just copy and paste from whatever is closest. 😂
docs are coming from tox -c py/tox.ini -e docs

nvborisenko · 2022-10-27T17:18:18Z

dotnet/src/webdriver/SeleniumManager.cs

+                                byte[] fileBytes = binReader.ReadBytes((int)fileStream.Length);
+                                string directoryName = string.Format(CultureInfo.InvariantCulture, "webdriver.{0}",
+                                    Guid.NewGuid().ToString("N"));
+                                var path = Path.Combine(Path.GetTempPath(), directoryName + "/" + folder);


I don't think that writing to %temp% directory is good. Given that each time it creates new folder, disk space is used irrationally. Executing 100 tests produces 100 files which will be unpredictably cleaned up by OS. Supposably it's better to copy binary file to user's app space like Environment.CurrentDirectory, and make copying process thread-safe.

well, it should be once per process, not once per test at least, since it is static method.
I think the next step is putting it in $Home/.cache/manager/ (similar to how we're storing the drivers themselves)
but then we'd need logic to make sure we have the latest version.

nvborisenko · 2022-10-27T17:19:17Z

dotnet/src/webdriver/SeleniumManager.cs

+
+                                using (BinaryWriter binWriter = new BinaryWriter(File.Open(filePath, FileMode.Create)))
+                                {
+                                    binWriter.Flush();


Redundant invocation, Flush is automatically used when disposing.

but the flush is before the writing not after (with the disposal). tbh I'm not sure why you need it, I was going off of what I saw someone else doing...

nvborisenko · 2022-10-27T17:21:41Z

dotnet/src/webdriver/SeleniumManager.cs

+            String output = process.StandardOutput.ReadToEnd();
+
+            if (!output.StartsWith("INFO")) {
+                throw new WebDriverException("Invalid response from process: " + process);


Include output into exception message to see what exactly the process wrote.

right now output is always an empty string if there's a problem. If/when we start outputting more things (like errors), we can update.

harsha509 · 2022-10-28T09:07:54Z

common/manager/BUILD.bazel

+        "//java/test/org/openqa/selenium/edge:__pkg__",
+        "//java/test/org/openqa/selenium/firefox:__pkg__",
+        "//py:__pkg__",
+        "//rb:__pkg__",


can we add this too for JS.
//javascript/node/selenium-webdriver:__pkg__

the implementation goes in here #11189

Yes, let's add in your PR

Updated the PR!

sonarqubecloud · 2022-10-28T13:41:14Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

titusfortner force-pushed the se_mgr_rb branch 3 times, most recently from 1956120 to 1804f48 Compare October 14, 2022 00:29

titusfortner force-pushed the se_mgr_rb branch 3 times, most recently from db355f0 to 814f6cf Compare October 14, 2022 01:19

titusfortner marked this pull request as ready for review October 14, 2022 01:20

titusfortner force-pushed the se_mgr_rb branch from 814f6cf to 186bada Compare October 14, 2022 01:40

titusfortner force-pushed the se_mgr_rb branch 2 times, most recently from 4038776 to 85e73be Compare October 14, 2022 03:03

titusfortner added this to the 4.6 milestone Oct 14, 2022

titusfortner changed the title ~~[rb] implementation of Selenium Manager~~ Implementation of Selenium Manager Oct 14, 2022

symonk reviewed Oct 14, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 14, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 14, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 14, 2022

View reviewed changes

py/selenium/webdriver/common/service.py Outdated Show resolved Hide resolved

symonk reviewed Oct 14, 2022

View reviewed changes

py/selenium/webdriver/common/service.py Outdated Show resolved Hide resolved

titusfortner self-assigned this Oct 17, 2022

titusfortner force-pushed the se_mgr_rb branch 2 times, most recently from 3cb8f2f to d1d1b43 Compare October 17, 2022 16:50

symonk reviewed Oct 17, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 17, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 17, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 17, 2022

View reviewed changes

py/BUILD.bazel Show resolved Hide resolved

symonk reviewed Oct 17, 2022

View reviewed changes

py/selenium/webdriver/common/service.py Show resolved Hide resolved

bonigarcia force-pushed the se_mgr_rb branch from a1bd23c to c0a32dd Compare October 18, 2022 12:45

titusfortner commented Oct 18, 2022

View reviewed changes

java/src/org/openqa/selenium/remote/service/DriverService.java Show resolved Hide resolved

bonigarcia force-pushed the se_mgr_rb branch from c0a32dd to 9f06571 Compare October 18, 2022 13:32

nvborisenko reviewed Oct 26, 2022

View reviewed changes

titusfortner force-pushed the se_mgr_rb branch from d73c929 to 98f2080 Compare October 26, 2022 22:17

symonk reviewed Oct 27, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Show resolved Hide resolved

symonk reviewed Oct 27, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Show resolved Hide resolved

symonk reviewed Oct 27, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Show resolved Hide resolved

symonk reviewed Oct 27, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 27, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 27, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 27, 2022

View reviewed changes

py/selenium/webdriver/common/selenium_manager.py Outdated Show resolved Hide resolved

symonk reviewed Oct 27, 2022

View reviewed changes

titusfortner force-pushed the se_mgr_rb branch from b37cb52 to 8cf4afe Compare October 27, 2022 15:49

nvborisenko reviewed Oct 27, 2022

View reviewed changes

titusfortner force-pushed the se_mgr_rb branch 2 times, most recently from 3e4eb7f to 8a809ab Compare October 27, 2022 21:32

harsha509 reviewed Oct 28, 2022

View reviewed changes

harsha509 mentioned this pull request Oct 28, 2022

[JS] add support for selenium manager #11189

Merged

8 tasks

titusfortner and others added 5 commits October 28, 2022 08:25

add pre-built selenium manager binaries

c927d4b

[rb] add initial support for selenium manager

08dcad6

[py] add initial support for selenium manager

ba18ecc

[java] Add initial support for Selenium Manager

eecdaca

[dotnet] initial implementation of Selenium Manager

69a327e

titusfortner force-pushed the se_mgr_rb branch from 8a809ab to 69a327e Compare October 28, 2022 13:25

titusfortner merged commit 69a327e into trunk Oct 28, 2022

titusfortner deleted the se_mgr_rb branch October 28, 2022 13:26

titusfortner added the hacktoberfest-accepted label Oct 28, 2022

SeleniumHQ deleted a comment from 5ryde Nov 3, 2022

Uh oh!

Implementation of Selenium Manager #11120

Implementation of Selenium Manager #11120

Uh oh!

Conversation

titusfortner commented Oct 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Oct 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

titusfortner commented Oct 14, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nvborisenko Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harsha509 Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

titusfortner commented Oct 13, 2022 •

edited

Loading

codecov-commenter commented Oct 14, 2022 •

edited

Loading

nvborisenko Oct 27, 2022 •

edited

Loading

harsha509 Oct 28, 2022 •

edited

Loading