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] manage bidi instance on the bridge not the driver #14071

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

titusfortner
Copy link
Member

@titusfortner titusfortner commented Jun 3, 2024

User description

Description

Moves implementation of bidi away from the driver class and onto the bridge class
Should be a non-breaking change

Motivation and Context

Classes that use BiDi should be initialized by a bridge instance not the driver instance.

I think we probably want to implement the conversion of WebDriver Classic to WebDriver BiDi as a separate Bridge, and this should make that easier.


PR Type

enhancement


Description

  • Removed BiDi instance management from the driver class and delegated it to the bridge class.
  • Updated quit and close methods in the driver class to remove BiDi handling.
  • Added BiDi instance management to the bridge class, including updates to quit and close methods.
  • Updated method signatures to reflect the changes in BiDi instance management.

Changes walkthrough 📝

Relevant files
Enhancement
driver.rb
Remove BiDi instance management from driver class               

rb/lib/selenium/webdriver/common/driver.rb

  • Removed BiDi instance management from the driver class.
  • Updated quit and close methods to remove BiDi handling.
  • +1/-6     
    has_bidi.rb
    Delegate BiDi instance management to bridge                           

    rb/lib/selenium/webdriver/common/driver_extensions/has_bidi.rb

    • Updated bidi method to delegate to the bridge.
    +1/-1     
    bridge.rb
    Add BiDi instance management to bridge class                         

    rb/lib/selenium/webdriver/remote/bridge.rb

  • Added BiDi instance management to the bridge class.
  • Updated quit and close methods to handle BiDi instance.
  • +10/-1   
    driver.rbs
    Remove BiDi instance variable from driver signature           

    rb/sig/lib/selenium/webdriver/common/driver.rbs

    • Removed BiDi instance variable declaration.
    +0/-1     
    bridge.rbs
    Add BiDi instance variable to bridge signature                     

    rb/sig/lib/selenium/webdriver/remote/bridge.rbs

    • Added BiDi instance variable declaration.
    +1/-0     

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

    Copy link
    Contributor

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    3, because the PR involves changes across multiple files and classes, including architectural changes that shift responsibilities from one class to another. Understanding the impact of these changes on the system's behavior and ensuring that all dependencies and interactions are correctly handled requires a moderate level of effort.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    Possible Bug: The removal of BiDi instance management from the driver.rb and its addition to bridge.rb could potentially lead to issues if not all references and dependencies were updated accordingly. For example, any method that previously depended on @bidi being initialized directly in the Driver class might fail if it hasn't been updated to use the bridge's BiDi instance.

    Error Handling: The new error message in bridge.rb for BiDi operations when web_socket_url is not set might not be clear enough for end users or might not cover all scenarios where the error could occur.

    🔒 Security concerns

    No

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Ensure bridge is properly initialized before using it

    To ensure that the bridge variable is always initialized before it is used, consider
    moving the add_extensions(bridge.browser) call after the @bridge assignment. This will
    prevent potential issues if bridge is nil or improperly initialized.

    rb/lib/selenium/webdriver/common/driver.rb [73-75]

     bridge ||= create_bridge(**opts)
    -add_extensions(bridge.browser)
     @bridge = listener ? Support::EventFiringBridge.new(bridge, listener) : bridge
    +add_extensions(@bridge.browser)
     
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies a potential issue with the order of operations that could lead to using an uninitialized or improperly initialized bridge. Reordering the operations as suggested would indeed make the code more robust.

    8
    Safeguard against uninitialized bridge variable in the close method

    To avoid potential issues with uninitialized variables, ensure that bridge is always
    initialized before calling bridge.close in the close method.

    rb/lib/selenium/webdriver/common/driver.rb [183]

    -bridge.close
    +@bridge&.close
     
    Suggestion importance[1-10]: 7

    Why: This suggestion is valid as it addresses the potential for calling close on an uninitialized bridge. Using the safe navigation operator (&.) is a good practice in Ruby to avoid nil errors.

    7
    Safeguard against uninitialized @bidi variable when closing it

    To ensure that the @bidi instance is only closed if it has been initialized, use the safe
    navigation operator (&.) when calling @bidi.close.

    rb/lib/selenium/webdriver/remote/bridge.rb [217]

    -@bidi.close
    +@bidi&.close
     
    Suggestion importance[1-10]: 7

    Why: The suggestion to use the safe navigation operator (&.) on @bidi.close is appropriate and improves the code by preventing possible nil errors. This is a good catch and a relevant improvement.

    7
    Safeguard against uninitialized @bidi variable in the close method

    To ensure that the @bidi instance is only closed if it has been initialized, use the safe
    navigation operator (&.) when calling @bidi.close in the close method.

    rb/lib/selenium/webdriver/remote/bridge.rb [221]

    -execute(:close_window).tap { |handles| @bidi.close if handles.empty? }
    +execute(:close_window).tap { |handles| @bidi&.close if handles.empty? }
     
    Suggestion importance[1-10]: 7

    Why: This suggestion is similar to the previous one and correctly applies to a different method. Using the safe navigation operator (&.) in this context is also a good practice to avoid potential errors from calling methods on nil.

    7

    Copy link
    Contributor

    codiumai-pr-agent-pro bot commented Jun 3, 2024

    CI Failure Feedback 🧐

    (Checks updated until commit 4cbaa89)

    Action: Ruby / Unit Tests (3.0.6, macos) / Unit Tests (3.0.6, macos)

    Failed stage: Run Bazel [❌]

    Failed test name: Selenium::WebDriver::Remote::Bridge#quit

    Failure summary:

    The action failed due to two issues:

  • The test Selenium::WebDriver::Remote::Bridge#quit failed because it raised a NoMethodError. The
    method quit attempted to call close on a nil object.
  • The test //rb/spec/unit/selenium:server timed out after 1800 seconds.

  • Relevant error logs:
    1:  ##[group]Operating System
    2:  macOS
    ...
    
    684:  /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'map_field.o' from 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/0/map_field.o(map_field.o)' and 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/1/map_field.o(map_field.o)'
    685:  /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'message.o' from 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/1/message.o(message.o)' and 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/0/message.o(message.o)'
    686:  /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'message_field.o' from 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/0/message_field.o(message_field.o)' and 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/1/message_field.o(message_field.o)'
    687:  /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'primitive_field.o' from 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/1/primitive_field.o(primitive_field.o)' and 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/0/primitive_field.o(primitive_field.o)'
    688:  /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'service.o' from 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/0/service.o(service.o)' and 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/1/service.o(service.o)'
    689:  /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'string_field.o' from 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/0/string_field.o(string_field.o)' and 'bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protoc_lib/1/string_field.o(string_field.o)'
    690:  �[32m[1,005 / 1,849]�[0m Creating source manifest for //rb/spec/unit/selenium/webdriver/firefox:profile; 0s local ... (3 actions, 1 running)
    691:  �[32mINFO: �[0mFrom Linking external/protobuf~/libprotobuf.a [for tool]:
    692:  /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: file: bazel-out/darwin_x86_64-opt-exec-ST-13d3ddad9198/bin/external/protobuf~/_objs/protobuf/error_listener.o has no symbols
    ...
    
    886:  �[32mINFO: �[0mFrom Testing //rb/spec/unit/selenium/webdriver/remote:bridge:
    887:  <internal:dir>:134: warning: /var/root/.local/share/gem/ruby/3.0.0/specifications: Permission denied
    888:  <internal:dir>:134: warning: /var/root/.local/share/gem/ruby/3.0.0/specifications: Permission denied
    889:  DEBUGGER: Debugger can attach via UNIX domain socket (/var/folders/w3/xh1ngrc13bg46kmhfdprbpyw0000gn/T/rdbg-501/rdbg-14627)
    890:  Selenium::WebDriver::Remote::Bridge
    891:  .add_command
    892:  adds new command
    893:  #initialize
    894:  raises ArgumentError if passed invalid options
    895:  #create_session
    896:  accepts Hash
    897:  uses alwaysMatch when passed
    898:  uses firstMatch when passed
    899:  supports responses with "value" -> "capabilities" capabilities
    900:  #upload
    901:  2024-06-03 22:00:44 ERROR Selenium [:file_detector] File detector only works with files. "NotAFile" isn`t a file! 
    902:  raises WebDriverError if uploading non-files
    903:  #quit
    904:  respects quit_errors (FAILED - 1)
    ...
    
    907:  returns an element
    908:  when custom element class is used
    909:  returns a custom element
    910:  #find_elements_by
    911:  returns an element
    912:  when custom element class is used
    913:  returns a custom element
    914:  Failures:
    915:  1) Selenium::WebDriver::Remote::Bridge#quit respects quit_errors
    916:  Failure/Error: expect { bridge.quit }.not_to raise_error
    917:  expected no Exception, got #<NoMethodError: undefined method `close' for nil:NilClass> with backtrace:
    918:  # ./rb/lib/selenium/webdriver/remote/bridge.rb:217:in `quit'
    919:  # ./rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb:141:in `block (4 levels) in <module:Remote>'
    920:  # ./rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb:141:in `block (3 levels) in <module:Remote>'
    921:  # ./rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb:141:in `block (3 levels) in <module:Remote>'
    922:  Finished in 0.18944 seconds (files took 1.38 seconds to load)
    923:  12 examples, 1 failure
    924:  Failed examples:
    925:  rspec ./rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb:138 # Selenium::WebDriver::Remote::Bridge#quit respects quit_errors
    926:  ================================================================================
    927:  �[32m[2,015 / 2,037]�[0m 40 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 111s local, disk-cache ... (4 actions, 1 running)
    928:  �[32m[2,015 / 2,037]�[0m 40 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 112s local, disk-cache ... (4 actions, 2 running)
    929:  �[32m[2,015 / 2,037]�[0m 40 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 115s local, disk-cache ... (4 actions, 2 running)
    930:  �[32m[2,016 / 2,037]�[0m 41 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 116s local, disk-cache ... (4 actions, 2 running)
    931:  �[32m[2,016 / 2,037]�[0m 41 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 117s local, disk-cache ... (4 actions, 3 running)
    932:  �[32m[2,016 / 2,037]�[0m 41 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 118s local, disk-cache ... (4 actions, 3 running)
    933:  �[32m[2,018 / 2,037]�[0m 43 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 119s local, disk-cache ... (4 actions, 2 running)
    934:  �[32m[2,018 / 2,037]�[0m 43 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 120s local, disk-cache ... (4 actions, 2 running)
    935:  �[32m[2,018 / 2,037]�[0m 43 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 122s local, disk-cache ... (4 actions, 2 running)
    936:  �[32m[2,019 / 2,037]�[0m 44 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 123s local, disk-cache ... (4 actions, 1 running)
    937:  �[32m[2,019 / 2,037]�[0m 44 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 124s local, disk-cache ... (4 actions, 2 running)
    938:  �[32m[2,019 / 2,037]�[0m 44 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 125s local, disk-cache ... (4 actions, 3 running)
    939:  �[32m[2,020 / 2,037]�[0m 45 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 126s local, disk-cache ... (4 actions, 3 running)
    940:  �[32m[2,020 / 2,037]�[0m 45 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 127s local, disk-cache ... (4 actions, 3 running)
    941:  �[32m[2,020 / 2,037]�[0m 45 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 129s local, disk-cache ... (4 actions, 3 running)
    942:  �[32m[2,022 / 2,037]�[0m 47 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 131s local, disk-cache ... (4 actions, 1 running)
    943:  �[32m[2,022 / 2,037]�[0m 47 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 132s local, disk-cache ... (4 actions, 2 running)
    944:  �[32m[2,022 / 2,037]�[0m 47 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 135s local, disk-cache ... (4 actions, 2 running)
    945:  �[32m[2,023 / 2,037]�[0m 48 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 136s local, disk-cache ... (4 actions, 3 running)
    946:  �[32m[2,023 / 2,037]�[0m 48 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 138s local, disk-cache ... (4 actions, 3 running)
    947:  �[32m[2,025 / 2,037]�[0m 50 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 139s local, disk-cache ... (4 actions, 2 running)
    948:  �[32m[2,025 / 2,037]�[0m 50 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 144s local, disk-cache ... (4 actions, 2 running)
    949:  �[32m[2,026 / 2,037]�[0m 51 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 146s local, disk-cache ... (4 actions, 1 running)
    950:  �[32m[2,026 / 2,037]�[0m 51 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 147s local, disk-cache ... (4 actions, 1 running)
    951:  �[32m[2,026 / 2,037]�[0m 51 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 149s local, disk-cache ... (4 actions, 3 running)
    952:  �[32m[2,026 / 2,037]�[0m 51 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 153s local, disk-cache ... (4 actions, 3 running)
    953:  �[32m[2,028 / 2,037]�[0m 53 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 154s local, disk-cache ... (4 actions, 2 running)
    954:  �[32m[2,028 / 2,037]�[0m 53 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 158s local, disk-cache ... (4 actions, 2 running)
    955:  �[32m[2,029 / 2,037]�[0m 54 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 160s local, disk-cache ... (4 actions, 1 running)
    956:  �[32m[2,029 / 2,037]�[0m 54 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 161s local, disk-cache ... (4 actions, 1 running)
    957:  �[32m[2,029 / 2,037]�[0m 54 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 163s local, disk-cache ... (4 actions, 3 running)
    958:  �[32m[2,029 / 2,037]�[0m 54 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 165s local, disk-cache ... (4 actions, 3 running)
    959:  �[32m[2,031 / 2,037]�[0m 56 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 166s local, disk-cache ... (4 actions, 2 running)
    960:  �[32m[2,031 / 2,037]�[0m 56 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 167s local, disk-cache ... (4 actions, 2 running)
    961:  �[32m[2,031 / 2,037]�[0m 56 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 169s local, disk-cache ... (4 actions, 2 running)
    962:  �[32m[2,032 / 2,037]�[0m 57 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 171s local, disk-cache ... (4 actions, 1 running)
    963:  �[32m[2,032 / 2,037]�[0m 57 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 173s local, disk-cache ... (4 actions, 1 running)
    964:  �[32m[2,032 / 2,037]�[0m 57 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 175s local, disk-cache ... (4 actions, 3 running)
    965:  �[32m[2,032 / 2,037]�[0m 57 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 177s local, disk-cache ... (4 actions, 3 running)
    966:  �[32m[2,034 / 2,037]�[0m 59 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 179s local, disk-cache ... (3 actions, 2 running)
    967:  �[32m[2,034 / 2,037]�[0m 59 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 181s local, disk-cache ... (3 actions, 2 running)
    968:  �[32m[2,035 / 2,037]�[0m 60 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 183s local, disk-cache ... (2 actions, 1 running)
    969:  �[32m[2,035 / 2,037]�[0m 60 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 184s local, disk-cache ... (2 actions running)
    970:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 185s local, disk-cache
    971:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 197s local, disk-cache
    972:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 229s local, disk-cache
    973:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 293s local, disk-cache
    974:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 358s local, disk-cache
    975:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 423s local, disk-cache
    976:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 487s local, disk-cache
    977:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 552s local, disk-cache
    978:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 616s local, disk-cache
    979:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 681s local, disk-cache
    980:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 746s local, disk-cache
    981:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 811s local, disk-cache
    982:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 876s local, disk-cache
    983:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 941s local, disk-cache
    984:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1006s local, disk-cache
    985:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1070s local, disk-cache
    986:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1135s local, disk-cache
    987:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1200s local, disk-cache
    988:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1265s local, disk-cache
    989:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1329s local, disk-cache
    990:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1393s local, disk-cache
    991:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1458s local, disk-cache
    992:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1522s local, disk-cache
    993:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1586s local, disk-cache
    994:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1650s local, disk-cache
    995:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1715s local, disk-cache
    996:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1779s local, disk-cache
    997:  �[32m[2,036 / 2,037]�[0m 61 / 62 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/unit/selenium:server; 1800s local, disk-cache
    ...
    
    1001:  ==================== Test output for //rb/spec/unit/selenium:server:
    1002:  <internal:dir>:134: warning: /var/root/.local/share/gem/ruby/3.0.0/specifications: Permission denied
    1003:  <internal:dir>:134: warning: /var/root/.local/share/gem/ruby/3.0.0/specifications: Permission denied
    1004:  -- Test timed out at 2024-06-03 22:28:55 UTC --
    1005:  ================================================================================
    1006:  �[32mINFO: �[0mFound 62 test targets...
    1007:  �[32mINFO: �[0mElapsed time: 1932.581s, Critical Path: 1858.16s
    1008:  �[32mINFO: �[0m2037 processes: 997 disk cache hit, 906 internal, 10 darwin-sandbox, 124 local.
    1009:  �[32mINFO: �[0mBuild completed, 2 tests FAILED, 2037 total actions
    ...
    
    1064:  //rb/spec/unit/selenium/webdriver/safari:driver                          �[0m�[32mPASSED�[0m in 2.5s
    1065:  //rb/spec/unit/selenium/webdriver/safari:options                         �[0m�[32mPASSED�[0m in 2.0s
    1066:  //rb/spec/unit/selenium/webdriver/safari:service                         �[0m�[32mPASSED�[0m in 2.9s
    1067:  //rb/spec/unit/selenium/webdriver/support:color                          �[0m�[32mPASSED�[0m in 6.0s
    1068:  //rb/spec/unit/selenium/webdriver/support:event_firing                   �[0m�[32mPASSED�[0m in 1.7s
    1069:  //rb/spec/unit/selenium/webdriver/support:select                         �[0m�[32mPASSED�[0m in 2.1s
    1070:  //rb/spec/unit/selenium:server                                          �[0m�[31m�[1mTIMEOUT�[0m in 1800.1s
    1071:  /Users/runner/.bazel/execroot/_main/bazel-out/darwin_x86_64-fastbuild/testlogs/rb/spec/unit/selenium/server/test.log
    1072:  //rb/spec/unit/selenium/webdriver/remote:bridge                          �[0m�[31m�[1mFAILED�[0m in 2.7s
    1073:  /Users/runner/.bazel/execroot/_main/bazel-out/darwin_x86_64-fastbuild/testlogs/rb/spec/unit/selenium/webdriver/remote/bridge/test.log
    1074:  Executed 62 out of 62 tests: 60 tests pass and �[0m�[31m�[1m2 fail locally�[0m.
    1075:  �[0m
    1076:  ##[error]Process completed with exit code 3.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    @titusfortner
    Copy link
    Member Author

    Actually, the AI has good suggestions to fix.
    RubyMine insisted I could remove the &. operations in a few places and I don't think it is correct.

    @titusfortner titusfortner merged commit cab1dd1 into trunk Jun 4, 2024
    29 of 30 checks passed
    @titusfortner titusfortner deleted the rb_bidi_bridge branch June 4, 2024 04:10
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Oct 29, 2024
    )
    
    * [rb] manage bidi instance on the bridge not the driver
    
    * restore null safety checks
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants