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

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

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

Earlopain
Copy link
Contributor

@Earlopain Earlopain commented Jun 5, 2024

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

    This is getting the same treatment as the `base64` gem, starting with Ruby 3.4. Also see SeleniumHQ#13454
    
    Closes SeleniumHQ#14081
    Copy link

    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

    Copy link

    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
    28 of 30 checks passed
    @titusfortner
    Copy link
    Member

    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
    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 4e1c049ac0..e05f4b3b3c 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
    jianbo pushed a commit to jianbo/fix-association-initialize-order that referenced this pull request Jul 8, 2024
    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 4e1c049ac0..e05f4b3b3c 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
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    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