Skip to content

Conversation

diemol
Copy link
Member

@diemol diemol commented May 21, 2025

User description

🔗 Related Issues

💥 What does this PR do?

This removes a couple of methods that have been deprecated for over 1 year.

🔧 Implementation Notes

💡 Additional Considerations

This will be mentioned in the release notes.

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR Type

Enhancement, Bug fix


Description

  • Remove deprecated setScriptTimeout and pageLoadTimeout methods

  • Update interfaces and implementations to use new timeout methods

  • Clean up related deprecated code and tests

  • Add new listener hooks for updated timeout methods


Changes walkthrough 📝

Relevant files
Enhancement
WebDriver.java
Remove deprecated timeout methods from WebDriver interface

java/src/org/openqa/selenium/WebDriver.java

  • Remove deprecated setScriptTimeout and pageLoadTimeout methods from
    Timeouts
  • Update interface to require new scriptTimeout(Duration) and
    pageLoadTimeout(Duration)
  • Remove legacy overloads and deprecation annotations
  • +2/-53   
    DriverCommand.java
    Remove deprecated timeout payloads using TimeUnit               

    java/src/org/openqa/selenium/remote/DriverCommand.java

  • Remove deprecated timeout command payload methods using TimeUnit
  • Retain only Duration-based timeout command payloads
  • +0/-19   
    RemoteWebDriver.java
    Remove deprecated timeout methods from RemoteWebDriver     

    java/src/org/openqa/selenium/remote/RemoteWebDriver.java

  • Remove deprecated timeout methods from Timeouts implementation
  • Update to use only Duration-based timeout methods
  • +0/-19   
    WebDriverListener.java
    Add and document new script timeout listener hooks             

    java/src/org/openqa/selenium/support/events/WebDriverListener.java

  • Add new beforeScriptTimeout and afterScriptTimeout hooks
  • Deprecate old beforeSetScriptTimeout and afterSetScriptTimeout hooks
  • Update documentation for new methods
  • +24/-2   
    Tests
    DecoratedTimeoutsTest.java
    Update tests to remove deprecated timeout usage                   

    java/test/org/openqa/selenium/support/decorators/DecoratedTimeoutsTest.java

  • Remove tests for deprecated timeout methods
  • Update tests to use new Duration-based timeout methods
  • +1/-17   

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added C-java Java Bindings B-support Issue or PR related to support classes labels May 21, 2025
    @selenium-ci
    Copy link
    Member

    Thank you, @diemol for this code suggestion.

    The support packages contain example code that many users find helpful, but they do not necessarily represent
    the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

    We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
    If you have any questions, please contact us

    Copy link
    Contributor

    qodo-merge-pro bot commented May 21, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 5778160)

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ❌

    5678 - Not compliant

    Non-compliant requirements:

    • Fix the "Error: ConnectFailure (Connection refused)" issue when instantiating ChromeDriver multiple times

    Requires further human verification:

    • Issue occurs on Ubuntu 16.04.4 with Linux 4.10.0
    • Affects Selenium 3.9.0 with Chrome 65.0.3325.181 and ChromeDriver 2.35
    • First ChromeDriver instance works fine, but subsequent instances fail

    1234 - Not compliant

    Non-compliant requirements:

    • Fix issue where JavaScript in link's href is not triggered on click() in Firefox

    Requires further human verification:

    • Problem occurs in Selenium 2.48.0 and 2.48.2 but works in 2.47.1
    • Affects Firefox 42.0 32bit on 64bit machine

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Incomplete Deprecation

    New methods have been added with @deprecated annotations on old methods, but there's no implementation to forward calls from old methods to new ones, which could break existing code that relies on these listeners.

    @Deprecated default void beforeSetScriptTimeout(WebDriver.Timeouts timeouts, Duration duration) {} /**  * This action will be performed each time before {@link  * WebDriver.Timeouts#scriptTimeout(Duration)} is called.  *  * @param timeouts The timeouts object that will be called  * @param duration The duration that will be passed to the method  */ default void beforeScriptTimeout(WebDriver.Timeouts timeouts, Duration duration) {} /**  * This action will be performed each time after {@link  * WebDriver.Timeouts#scriptTimeout(Duration)} is called.  *  * @param timeouts The timeouts object that will be called  * @param duration The duration that will be passed to the method  * @deprecated Use {@link #afterScriptTimeout(WebDriver.Timeouts, Duration)} instead.  */ @Deprecated default void afterSetScriptTimeout(WebDriver.Timeouts timeouts, Duration duration) {}
    Empty Test Method

    The setScriptTimeout test method body is empty. It should be updated to test the new scriptTimeout method.

    void setScriptTimeout() { verifyFunction($ -> $.scriptTimeout(Duration.ofSeconds(10))); }
    Copy link
    Contributor

    qodo-merge-pro bot commented May 21, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 5778160
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add backward compatibility delegation

    The new beforeScriptTimeout method should delegate to the deprecated
    beforeSetScriptTimeout method to maintain backward compatibility during
    transition. This ensures existing implementations continue to work while users
    migrate to the new method.

    java/src/org/openqa/selenium/support/events/WebDriverListener.java [928]

    -default void beforeScriptTimeout(WebDriver.Timeouts timeouts, Duration duration) {} +default void beforeScriptTimeout(WebDriver.Timeouts timeouts, Duration duration) { + beforeSetScriptTimeout(timeouts, duration); +}
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: Delegating the new beforeScriptTimeout method to the deprecated beforeSetScriptTimeout method is a sound approach for backward compatibility, ensuring existing user implementations continue to function. This is a maintainability improvement but not critical, as it does not affect core functionality.

    Medium
    • Update

    Previous suggestions

    Suggestions up to commit 5778160
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add backward compatibility delegation

    The new beforeScriptTimeout method should delegate to the deprecated
    beforeSetScriptTimeout method to maintain backward compatibility during
    transition. This ensures existing implementations continue to work while users
    migrate to the new method.

    java/src/org/openqa/selenium/support/events/WebDriverListener.java [928]

    -default void beforeScriptTimeout(WebDriver.Timeouts timeouts, Duration duration) {} +default void beforeScriptTimeout(WebDriver.Timeouts timeouts, Duration duration) { + beforeSetScriptTimeout(timeouts, duration); +}
    Suggestion importance[1-10]: 7

    __

    Why: Delegating the new beforeScriptTimeout method to the deprecated beforeSetScriptTimeout ensures backward compatibility, which is important for users transitioning to the new API. This is a solid maintainability improvement, though not critical for correctness.

    Medium
    Learned
    best practice
    Fix inconsistent deprecation message

    The deprecated method reference in the Javadoc comment for afterSetScriptTimeout
    is inconsistent. It refers to afterScriptTimeout but the method being deprecated
    is afterSetScriptTimeout. This creates confusion for developers migrating to the
    new API.

    java/src/org/openqa/selenium/support/events/WebDriverListener.java [938-939]

     default void beforeScriptTimeout(WebDriver.Timeouts timeouts, Duration duration) {} /** * This action will be performed each time after {@link * WebDriver.Timeouts#scriptTimeout(Duration)} is called. * * @param timeouts The timeouts object that will be called * @param duration The duration that will be passed to the method - * @deprecated Use {@link #afterScriptTimeout(WebDriver.Timeouts, Duration)} instead. + * @deprecated This method is deprecated and will be removed. Use {@link #afterScriptTimeout(WebDriver.Timeouts, Duration)} instead. */ @Deprecated default void afterSetScriptTimeout(WebDriver.Timeouts timeouts, Duration duration) {}
    Suggestion importance[1-10]: 6

    __

    Why:
    Relevant best practice - Fix inconsistent error messages and validation logic. Ensure error messages are clear, accurate, and provide specific information about the error condition and expected values.

    Low
    @diemol diemol reopened this May 21, 2025
    Copy link
    Contributor

    CI Feedback 🧐

    A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

    Action: Ruby / Unit Tests (3.2.8, windows) / Unit Tests (3.2.8, windows)

    Failed stage: Run Bazel [❌]

    Failed test name: //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions

    Failure summary:

    The action failed because the test
    //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions timed out after 1800 seconds
    (30 minutes). This was the only failing test out of 66 total tests. The test was running for an
    extended period without completing, causing the build to fail.

    Relevant error logs:
    1: ##[group]Operating System 2: Microsoft Windows Server 2022 ... 510: gpg: setting ownertrust to 4 511: gpg: setting ownertrust to 4 512: gpg: setting ownertrust to 4 513: gpg: setting ownertrust to 4 514: ==> Disabling revoked keys in keyring... 515: -> Disabled 4 keys. 516: ==> Updating trust database... 517: gpg: marginals needed: 3 completes needed: 1 trust model: pgp 518: gpg: depth: 0 valid: 1 signed: 5 trust: 0-, 0q, 0n, 0m, 0f, 1u 519: gpg: depth: 1 valid: 5 signed: 7 trust: 0-, 0q, 0n, 5m, 0f, 0u 520: gpg: depth: 2 valid: 4 signed: 2 trust: 4-, 0q, 0n, 0m, 0f, 0u 521: gpg: next trustdb check due at 2025-08-13 522: �[32mAnalyzing:�[0m 66 targets (313 packages loaded, 7660 targets configured) 523: �[32mAnalyzing:�[0m 66 targets (339 packages loaded, 8147 targets configured) 524: �[35mWARNING: �[0mD:/_bazel/external/io_bazel_rules_closure/java/com/google/javascript/jscomp/BUILD:19:13: in java_library rule @@io_bazel_rules_closure//java/com/google/javascript/jscomp:jscomp: target '@@io_bazel_rules_closure//java/com/google/javascript/jscomp:jscomp' depends on deprecated target '@@io_bazel_rules_closure//java/io/bazel/rules/closure:build_info_java_proto': Use java_proto_library from com_google_protobuf 525: gpg: error retrieving 'alexey.pawlow@gmail.com' via WKD: No data 526: gpg: error reading key: No data 527: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 528: �[32mAnalyzing:�[0m 66 targets (361 packages loaded, 8844 targets configured) 529: gpg: key F40D263ECA25678A: "Alexey Pavlov (Alexpux) <alexey.pawlow@gmail.com>" not changed 530: gpg: Total number processed: 1 531: gpg: unchanged: 1 532: gpg: error retrieving 'david.macek.0@gmail.com' via WKD: No data 533: gpg: error reading key: No data 534: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 535: gpg: key 790AE56A1D3CFDDC: "David Macek (MSYS2 master key) <david.macek.0@gmail.com>" not changed 536: gpg: Total number processed: 1 537: gpg: unchanged: 1 538: gpg: error retrieving 'martellmalone@gmail.com' via WKD: No data 539: gpg: error reading key: No data 540: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 541: gpg: key DA7EF2ABAEEA755C: "Martell Malone (martell) <martellmalone@gmail.com>" not changed 542: gpg: Total number processed: 1 543: gpg: unchanged: 1 544: gpg: error retrieving 'reiter.christoph@gmail.com' via WKD: No data 545: gpg: error reading key: No data 546: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 547: gpg: key 755B8182ACD22879: "Christoph Reiter (MSYS2 master key) <reiter.christoph@gmail.com>" not changed 548: gpg: Total number processed: 1 549: gpg: unchanged: 1 550: gpg: error retrieving 'icquinteiro@gmail.com' via WKD: No data 551: gpg: error reading key: No data 552: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 553: gpg: key 9F418C233E652008: "Ignacio Casal Quinteiro <icquinteiro@gmail.com>" not changed 554: gpg: Total number processed: 1 555: gpg: unchanged: 1 556: gpg: error retrieving 'mingw.android@gmail.com' via WKD: No data 557: gpg: error reading key: No data 558: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 559: gpg: key BBE514E53E0D0813: "Ray Donnelly (MSYS2 Developer - master key) <mingw.android@gmail.com>" not changed 560: gpg: Total number processed: 1 561: gpg: unchanged: 1 562: gpg: error retrieving 'alexpux@gmail.com' via WKD: No data 563: gpg: error reading key: No data 564: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 565: gpg: key 5F92EFC1A47D45A1: "Alexey Pavlov (Alexpux) <alexpux@gmail.com>" not changed 566: gpg: Total number processed: 1 567: gpg: unchanged: 1 568: gpg: error retrieving 'david.macek.0@gmail.com' via WKD: No data 569: gpg: error reading key: No data 570: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 571: gpg: key 974C8BE49078F532: "David Macek <david.macek.0@gmail.com>" not changed 572: gpg: Total number processed: 1 573: gpg: unchanged: 1 574: gpg: error retrieving 'reiter.christoph@gmail.com' via WKD: No data 575: gpg: error reading key: No data 576: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 577: gpg: key FA11531AA0AA7F57: "Christoph Reiter (MSYS2 development key) <reiter.christoph@gmail.com>" not changed 578: gpg: Total number processed: 1 579: gpg: unchanged: 1 580: gpg: error retrieving 'me@martellmalone.com' via WKD: General error 581: gpg: error reading key: General error 582: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 583: gpg: key 794DCF97F93FC717: "Martell Malone (martell) <me@martellmalone.com>" not changed 584: gpg: Total number processed: 1 585: gpg: unchanged: 1 586: gpg: error retrieving 'martellmalone@gmail.com' via WKD: No data 587: gpg: error reading key: No data 588: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com 589: gpg: key D595C9AB2C51581E: "Martell Malone (MSYS2 Developer) <martellmalone@gmail.com>" not changed 590: gpg: Total number processed: 1 591: gpg: unchanged: 1 592: gpg: error retrieving 'mingw.android@gmail.com' via WKD: No data 593: gpg: error reading key: No data 594: gpg: refreshing 1 key from hkps://keyserver.ubuntu.com ... 706: AccessController.doPrivileged( 707: ^ 708: �[32mINFO: �[0mFrom Compiling absl/base/log_severity.cc [for tool]: 709: cl : Command line warning D9002 : ignoring unknown option '-std=c++14' 710: �[32mINFO: �[0mFrom Compiling absl/base/internal/raw_logging.cc [for tool]: 711: cl : Command line warning D9002 : ignoring unknown option '-std=c++14' 712: �[32mINFO: �[0mFrom Compiling absl/types/bad_optional_access.cc [for tool]: 713: cl : Command line warning D9002 : ignoring unknown option '-std=c++14' 714: �[32m[507 / 1,157]�[0m Copying files; 0s local ... (3 actions, 1 running) 715: �[32mINFO: �[0mFrom Compiling absl/types/bad_variant_access.cc [for tool]: 716: cl : Command line warning D9002 : ignoring unknown option '-std=c++14' 717: �[32mINFO: �[0mFrom Compiling absl/profiling/internal/exponential_biased.cc [for tool]: 718: cl : Command line warning D9002 : ignoring unknown option '-std=c++14' 719: �[32mINFO: �[0mFrom Compiling absl/strings/internal/cordz_functions.cc [for tool]: 720: cl : Command line warning D9002 : ignoring unknown option '-std=c++14' 721: �[32mINFO: �[0mFrom Compiling absl/base/internal/strerror.cc [for tool]: 722: cl : Command line warning D9002 : ignoring unknown option '-std=c++14' ... 1984: �[32m[1,267 / 1,268]�[0m 65 / 66 tests;�[0m Testing //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions; 1493s local, disk-cache 1985: �[32m[1,267 / 1,268]�[0m 65 / 66 tests;�[0m Testing //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions; 1553s local, disk-cache 1986: �[32m[1,267 / 1,268]�[0m 65 / 66 tests;�[0m Testing //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions; 1614s local, disk-cache 1987: �[32m[1,267 / 1,268]�[0m 65 / 66 tests;�[0m Testing //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions; 1674s local, disk-cache 1988: �[32m[1,267 / 1,268]�[0m 65 / 66 tests;�[0m Testing //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions; 1734s local, disk-cache 1989: �[32m[1,267 / 1,268]�[0m 65 / 66 tests;�[0m Testing //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions; 1794s local, disk-cache 1990: �[32m[1,267 / 1,268]�[0m 65 / 66 tests;�[0m Testing //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions; 1800s local, disk-cache 1991: �[31m�[1mTIMEOUT: �[0m//rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions (Summary) 1992: ==================== Test output for //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions: 1993: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/unit/selenium/webdriver/common/interactions/pointer_actions/test.log 1994: ================================================================================ 1995: �[32mINFO: �[0mFrom Testing //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions: 1996: �[32mINFO: �[0mFound 66 test targets... 1997: �[32mINFO: �[0mElapsed time: 2374.038s, Critical Path: 2241.31s 1998: �[32mINFO: �[0m1268 processes: 552 disk cache hit, 575 internal, 141 local. 1999: �[32mINFO: �[0mBuild completed, 1 test FAILED, 1268 total actions 2000: //rb/spec/unit/selenium:devtools �[0m�[32mPASSED�[0m in 5.0s ... 2052: //rb/spec/unit/selenium/webdriver/ie:service �[0m�[32mPASSED�[0m in 4.8s 2053: //rb/spec/unit/selenium/webdriver/remote:bridge �[0m�[32mPASSED�[0m in 4.5s 2054: //rb/spec/unit/selenium/webdriver/remote:capabilities �[0m�[32mPASSED�[0m in 5.6s 2055: //rb/spec/unit/selenium/webdriver/remote:driver �[0m�[32mPASSED�[0m in 4.8s 2056: //rb/spec/unit/selenium/webdriver/remote/http:common �[0m�[32mPASSED�[0m in 4.6s 2057: //rb/spec/unit/selenium/webdriver/remote/http:curb �[0m�[32mPASSED�[0m in 4.4s 2058: //rb/spec/unit/selenium/webdriver/remote/http:default �[0m�[32mPASSED�[0m in 4.8s 2059: //rb/spec/unit/selenium/webdriver/safari:driver �[0m�[32mPASSED�[0m in 4.3s 2060: //rb/spec/unit/selenium/webdriver/safari:options �[0m�[32mPASSED�[0m in 4.7s 2061: //rb/spec/unit/selenium/webdriver/safari:service �[0m�[32mPASSED�[0m in 4.4s 2062: //rb/spec/unit/selenium/webdriver/support:color �[0m�[32mPASSED�[0m in 3.5s 2063: //rb/spec/unit/selenium/webdriver/support:event_firing �[0m�[32mPASSED�[0m in 5.6s 2064: //rb/spec/unit/selenium/webdriver/support:select �[0m�[32mPASSED�[0m in 4.8s 2065: //rb/spec/unit/selenium/webdriver/common/interactions:pointer_actions �[0m�[31m�[1mTIMEOUT�[0m in 1800.0s 2066: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/unit/selenium/webdriver/common/interactions/pointer_actions/test.log 2067: Executed 66 out of 66 tests: 65 tests pass and �[0m�[31m�[1m1 fails locally�[0m. 2068: �[0m 2069: ##[error]Process completed with exit code 1. 2070: Post job cleanup. 
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    B-support Issue or PR related to support classes C-java Java Bindings Review effort 2/5

    2 participants