[rb] manage bidi instance on the bridge not the driver

titusfortner commented 1 month ago

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

codiumai-pr-agent-pro[bot] commented 1 month ago

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.
⚡ Possible issues	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

codiumai-pr-agent-pro[bot] commented 1 month ago

PR Code Suggestions ✨

Category	Suggestion	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]](https://github.com/SeleniumHQ/selenium/pull/14071/files#diff-ecf89969518aec5e73ce3aff22dc10e88185534845a8a17228952c92f1498cddR73-R75) ```diff 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]](https://github.com/SeleniumHQ/selenium/pull/14071/files#diff-ecf89969518aec5e73ce3aff22dc10e88185534845a8a17228952c92f1498cddR183-R183) ```diff -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]](https://github.com/SeleniumHQ/selenium/pull/14071/files#diff-42b562229083afc6ac8cc70a735fc1b24863402aa1404f08f60322b24cdfefc3R217-R217) ```diff -@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]](https://github.com/SeleniumHQ/selenium/pull/14071/files#diff-42b562229083afc6ac8cc70a735fc1b24863402aa1404f08f60322b24cdfefc3R221-R221) ```diff -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

codiumai-pr-agent-pro[bot] commented 1 month ago

CI Failure Feedback 🧐

(Checks updated until commit https://github.com/SeleniumHQ/selenium/commit/4cbaa8979fa58ed74c58e033cb25b480a3a1b198)

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

**Failed stage:** [Run Bazel](https://github.com/SeleniumHQ/selenium/actions/runs/9357676529/job/25758221563) [❌]

**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:

```yaml 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: :134: warning: /var/root/.local/share/gem/ruby/3.0.0/specifications: Permission denied 888: :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 # 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 ' 920: # ./rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb:141:in `block (3 levels) in ' 921: # ./rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb:141:in `block (3 levels) in ' 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: :134: warning: /var/root/.local/share/gem/ruby/3.0.0/specifications: Permission denied 1003: :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](https://pr-agent-docs.codium.ai/tools/ci_feedback/).

titusfortner commented 1 month ago

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.

SeleniumHQ / selenium