Skip to content

[rb] Add logger gem as a runtime dependency #14082

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 5, 2024
Merged

[rb] Add logger gem as a runtime dependency #14082

merged 1 commit into from
Jun 5, 2024

Conversation

@Earlopain
Copy link
Contributor

@Earlopain Earlopain commented Jun 5, 2024 •
edited by qodo-merge-pro bot
Loading

User description

This is getting the same treatment as the base64 gem, starting with Ruby 3.4. Also see #13454

Closes #14081

I've chosen ~>1.4 as the constraint since that was the minor version available when Ruby 3.0 released (the currently minimum supported ruby version)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix, Enhancement

Description

Changes walkthrough 📝

Relevant files
Dependencies
selenium-webdriver.gemspec
Add `logger` gem as a runtime dependency                                 

rb/selenium-webdriver.gemspec

  • Added logger gem as a runtime dependency with version constraint ~>
    1.4.
    		•  +1/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @Earlopain
    [rb] Add logger gem as a runtime dependency
    c760e4e 
    This is getting the same treatment as the `base64` gem, starting with Ruby 3.4. Also see SeleniumHQ#13454

Closes SeleniumHQ#14081
    @qodo-merge-pro qodo-merge-pro bot added P-enhancement PR with a new feature P-bug fix PR addresses a known issue Review effort [1-5]: 1 labels Jun 5, 2024
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 5, 2024

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    1, because the PR involves a simple addition of a runtime dependency with a clear version constraint. The change is straightforward and limited to a single line in the gemspec file.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    No

    🔒 Security concerns

    No

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 5, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Verify compatibility of the new dependency with existing dependencies

    Ensure that the logger gem is compatible with the other dependencies in the project by
    running a dependency check or using a tool like bundler-audit to avoid potential
    conflicts.

    rb/selenium-webdriver.gemspec [51]

    +s.add_runtime_dependency 'logger', ['~> 1.4']
 
-
    Suggestion importance[1-10]: 7

    Why: The suggestion to verify compatibility of the new logger dependency is important to ensure no conflicts with existing dependencies, which is a good practice in dependency management. However, it does not address any critical or security-related issues directly.

    		7

    @titusfortner titusfortner merged commit b519b43 into SeleniumHQ:trunk Jun 5, 2024
    @titusfortner
    Copy link
    Member

    titusfortner commented Jun 5, 2024

    Thanks!

    @Earlopain Earlopain deleted the rb-logger-dep branch June 5, 2024 18:49
    yahonda added a commit to yahonda/rails that referenced this pull request Jun 21, 2024
    @yahonda
    Support selenium-webdriver 4.22.0 that enables CDP in Firefox by default
    1b905ed 
    This pull request supports selenium-webdriver 4.22.0 that enables CDP in Firefox by default.
because Firefox 129 deprecates Chrome DevTools Protocol (CDP).
selenium-webdriver 4.22.0 enables CDP explicitly by adding "remote.active-protocols"=>3 .

- Steps to reproduce and this commit addresses these failures.
```ruby
$ bundle update selenium-webdriver --conservative
$ git diff main ../Gemfile.lock
diff --git a/Gemfile.lock b/Gemfile.lock
index 4e1c049..e05f4b3 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -512,8 +512,9 @@ GEM
       google-protobuf (~> 3.25)
     sass-embedded (1.69.6-x86_64-linux-gnu)
       google-protobuf (~> 3.25)
-    selenium-webdriver (4.20.1)
+    selenium-webdriver (4.22.0)
       base64 (~> 0.2)
+      logger (~> 1.4)
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2, < 3.0)
       websocket (~> 1.0)
$ cd actionpack
$ bin/test test/dispatch/system_testing/driver_test.rb test/dispatch/system_testing/driver_test.rb
Running 18 tests in a single process (parallelization threshold is 50)
Run options: --seed 58668

.....F

Failure:
DriverTest#test_define_extra_capabilities_using_firefox [test/dispatch/system_testing/driver_test.rb:127]:
--- expected
+++ actual
@@ -1 +1 @@
-{"moz:firefoxOptions"=>{"args"=>["--host=127.0.0.1"], "prefs"=>{"browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}
+{"moz:firefoxOptions"=>{"args"=>["--host=127.0.0.1"], "prefs"=>{"remote.active-protocols"=>3, "browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}

bin/test test/dispatch/system_testing/driver_test.rb:113

.F

Failure:
DriverTest#test_define_extra_capabilities_using_headless_firefox [test/dispatch/system_testing/driver_test.rb:144]:
--- expected
+++ actual
@@ -1 +1 @@
-{"moz:firefoxOptions"=>{"args"=>["-headless", "--host=127.0.0.1"], "prefs"=>{"browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}
+{"moz:firefoxOptions"=>{"args"=>["-headless", "--host=127.0.0.1"], "prefs"=>{"remote.active-protocols"=>3, "browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}

bin/test test/dispatch/system_testing/driver_test.rb:130

..........

Finished in 0.007717s, 2332.3654 runs/s, 4794.3066 assertions/s.
18 runs, 37 assertions, 2 failures, 0 errors, 0 skips
```

- Planned Deprecation of CDP in Firefox
https://groups.google.com/a/mozilla.org/g/dev-platform/c/Z6Qu3ZT1MJ0?pli=1

- Add preference to enable CDP in Firefox by default
SeleniumHQ/selenium#14091

- [rb] Add logger gem as a runtime dependency rails#14082
SeleniumHQ/selenium#14082
    @yahonda yahonda mentioned this pull request Jun 21, 2024
    Merged
    4 tasks
    @seedysoft seedysoft mentioned this pull request Jun 21, 2024
    Merged
    Set2005 pushed a commit to Set2005/fix-association-initialize-order that referenced this pull request Jul 8, 2024
    @yahonda
    Support selenium-webdriver 4.22.0 that enables CDP in Firefox by default
    f9ba37e 
    This pull request supports selenium-webdriver 4.22.0 that enables CDP in Firefox by default.
because Firefox 129 deprecates Chrome DevTools Protocol (CDP).
selenium-webdriver 4.22.0 enables CDP explicitly by adding "remote.active-protocols"=>3 .

- Steps to reproduce and this commit addresses these failures.
```ruby
$ bundle update selenium-webdriver --conservative
$ git diff main ../Gemfile.lock
diff --git a/Gemfile.lock b/Gemfile.lock
index 4e1c049..e05f4b3 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -512,8 +512,9 @@ GEM
       google-protobuf (~> 3.25)
     sass-embedded (1.69.6-x86_64-linux-gnu)
       google-protobuf (~> 3.25)
-    selenium-webdriver (4.20.1)
+    selenium-webdriver (4.22.0)
       base64 (~> 0.2)
+      logger (~> 1.4)
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2, < 3.0)
       websocket (~> 1.0)
$ cd actionpack
$ bin/test test/dispatch/system_testing/driver_test.rb test/dispatch/system_testing/driver_test.rb
Running 18 tests in a single process (parallelization threshold is 50)
Run options: --seed 58668

.....F

Failure:
DriverTest#test_define_extra_capabilities_using_firefox [test/dispatch/system_testing/driver_test.rb:127]:
--- expected
+++ actual
@@ -1 +1 @@
-{"moz:firefoxOptions"=>{"args"=>["--host=127.0.0.1"], "prefs"=>{"browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}
+{"moz:firefoxOptions"=>{"args"=>["--host=127.0.0.1"], "prefs"=>{"remote.active-protocols"=>3, "browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}

bin/test test/dispatch/system_testing/driver_test.rb:113

.F

Failure:
DriverTest#test_define_extra_capabilities_using_headless_firefox [test/dispatch/system_testing/driver_test.rb:144]:
--- expected
+++ actual
@@ -1 +1 @@
-{"moz:firefoxOptions"=>{"args"=>["-headless", "--host=127.0.0.1"], "prefs"=>{"browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}
+{"moz:firefoxOptions"=>{"args"=>["-headless", "--host=127.0.0.1"], "prefs"=>{"remote.active-protocols"=>3, "browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}

bin/test test/dispatch/system_testing/driver_test.rb:130

..........

Finished in 0.007717s, 2332.3654 runs/s, 4794.3066 assertions/s.
18 runs, 37 assertions, 2 failures, 0 errors, 0 skips
```

- Planned Deprecation of CDP in Firefox
https://groups.google.com/a/mozilla.org/g/dev-platform/c/Z6Qu3ZT1MJ0?pli=1

- Add preference to enable CDP in Firefox by default
SeleniumHQ/selenium#14091

- [rb] Add logger gem as a runtime dependency rails#14082
SeleniumHQ/selenium#14082
    @MiguelPelegrina MiguelPelegrina mentioned this pull request Jul 12, 2024
    Merged
    @DariuszPiskorowski DariuszPiskorowski mentioned this pull request Jul 13, 2024
    Closed
    @aravindb26 aravindb26 mentioned this pull request Jul 16, 2024
    Open
    @x-way x-way mentioned this pull request Jul 24, 2024
    Closed
    @DariuszPiskorowski DariuszPiskorowski mentioned this pull request Aug 9, 2024
    Merged
    @x-way x-way mentioned this pull request Aug 10, 2024
    Closed
    DanielaVelasquez pushed a commit to DanielaVelasquez/rails that referenced this pull request Oct 3, 2024
    @yahonda @DanielaVelasquez
    Support selenium-webdriver 4.22.0 that enables CDP in Firefox by default
    a3c5a62 
    This pull request supports selenium-webdriver 4.22.0 that enables CDP in Firefox by default.
because Firefox 129 deprecates Chrome DevTools Protocol (CDP).
selenium-webdriver 4.22.0 enables CDP explicitly by adding "remote.active-protocols"=>3 .

- Steps to reproduce and this commit addresses these failures.
```ruby
$ bundle update selenium-webdriver --conservative
$ git diff main ../Gemfile.lock
diff --git a/Gemfile.lock b/Gemfile.lock
index 4e1c049..e05f4b3 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -512,8 +512,9 @@ GEM
       google-protobuf (~> 3.25)
     sass-embedded (1.69.6-x86_64-linux-gnu)
       google-protobuf (~> 3.25)
-    selenium-webdriver (4.20.1)
+    selenium-webdriver (4.22.0)
       base64 (~> 0.2)
+      logger (~> 1.4)
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2, < 3.0)
       websocket (~> 1.0)
$ cd actionpack
$ bin/test test/dispatch/system_testing/driver_test.rb test/dispatch/system_testing/driver_test.rb
Running 18 tests in a single process (parallelization threshold is 50)
Run options: --seed 58668

.....F

Failure:
DriverTest#test_define_extra_capabilities_using_firefox [test/dispatch/system_testing/driver_test.rb:127]:
--- expected
+++ actual
@@ -1 +1 @@
-{"moz:firefoxOptions"=>{"args"=>["--host=127.0.0.1"], "prefs"=>{"browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}
+{"moz:firefoxOptions"=>{"args"=>["--host=127.0.0.1"], "prefs"=>{"remote.active-protocols"=>3, "browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}

bin/test test/dispatch/system_testing/driver_test.rb:113

.F

Failure:
DriverTest#test_define_extra_capabilities_using_headless_firefox [test/dispatch/system_testing/driver_test.rb:144]:
--- expected
+++ actual
@@ -1 +1 @@
-{"moz:firefoxOptions"=>{"args"=>["-headless", "--host=127.0.0.1"], "prefs"=>{"browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}
+{"moz:firefoxOptions"=>{"args"=>["-headless", "--host=127.0.0.1"], "prefs"=>{"remote.active-protocols"=>3, "browser.startup.homepage"=>"http://www.seleniumhq.com/"}}, "browserName"=>"firefox"}

bin/test test/dispatch/system_testing/driver_test.rb:130

..........

Finished in 0.007717s, 2332.3654 runs/s, 4794.3066 assertions/s.
18 runs, 37 assertions, 2 failures, 0 errors, 0 skips
```

- Planned Deprecation of CDP in Firefox
https://groups.google.com/a/mozilla.org/g/dev-platform/c/Z6Qu3ZT1MJ0?pli=1

- Add preference to enable CDP in Firefox by default
SeleniumHQ/selenium#14091

- [rb] Add logger gem as a runtime dependency rails#14082
SeleniumHQ/selenium#14082
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Oct 29, 2024
    @Earlopain @sandeepsuryaprasad
    [rb] Add logger gem as a runtime dependency (SeleniumHQ#14082)
    c606d0d 
    This is getting the same treatment as the `base64` gem, starting with Ruby 3.4. Also see SeleniumHQ#13454

Closes SeleniumHQ#14081
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @titusfortner titusfortner titusfortner approved these changes

    Assignees

    No one assigned

    Labels

    P-bug fix PR addresses a known issue P-enhancement PR with a new feature Review effort [1-5]: 1

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    [🐛 Bug]: [rb] Dependency on the logger gem which will be removed in Ruby 3.5

    2 participants

    @Earlopain @titusfortner