You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Verify that authentication handlers properly process and validate credentialsSuggestion Impact:The commit changed the authentication handler test to actually verify authentication functionality by navigating to a page requiring authentication and checking if it worked, rather than just checking if the handler was added.
code diff:
- it 'adds an auth handler' do- network = driver.network- network.add_authentication_handler('username', 'password')- expect(network.callbacks.count).to be 1- end-- it 'adds a request handler' do- network = driver.network- network.add_request_handler- expect(network.callbacks.count).to be 1- end-- it 'adds a response handler' do- network = driver.network- network.add_response_handler- expect(network.callbacks.count).to be 1- end-- it 'removes a handler' do- network = driver.network- id = network.add_request_handler- network.remove_handler(id)- expect(network.callbacks.count).to be 0- end-- it 'clears all handlers' do- network = driver.network- network.add_request_handler- network.add_request_handler- network.clear_handlers- expect(network.callbacks.count).to be 0+ it 'adds an auth handler', skip: 'Do not execute BiDi test' do+ driver.network.add_authentication_handler('test', 'test')+ driver.navigate.to url_for('basicAuth')+ expect(driver.find_element(tag_name: 'h1').text).to eq('authorized')
end
Add test cases to verify that authentication handlers correctly handle the provided credentials.
-network.add_authentication_handler('username', 'password')-expect(network.callbacks.count).to be 1+auth_called = false+network.add_authentication_handler('username', 'password') do |auth|+ auth_called = true+ expect(auth.credentials.username).to eq('username')+ expect(auth.credentials.password).to eq('password')+end+# Trigger auth request+expect(auth_called).to be true
[Suggestion has been applied]
Suggestion importance[1-10]: 9
Why: This security-focused enhancement ensures proper credential handling and validation, which is critical for authentication functionality. The suggestion adds essential verification of the authentication mechanism.
9
General
Include handler implementation examples to demonstrate actual request/response interception functionality
Add a block parameter to the request and response handler methods to demonstrate how to actually handle the intercepted requests/responses. The current tests only verify handler registration but not their functionality.
-network.add_request_handler+network.add_request_handler do |request|+ # Example: Modify request headers+ request.headers['Custom-Header'] = 'test'+end
expect(network.callbacks.count).to be 1
Apply this suggestion
Suggestion importance[1-10]: 8
Why: The suggestion significantly improves test quality by demonstrating actual handler functionality instead of just verifying registration. This makes the tests more meaningful and provides valuable implementation examples.
8
Add test coverage for error scenarios in handler removal functionality
Add error handling test cases to verify the behavior when invalid handler IDs are provided to remove_handler.
+expect { network.remove_handler('invalid_id') }.to raise_error(StandardError)
network.remove_handler(id)
expect(network.callbacks.count).to be 0
Apply this suggestion
Suggestion importance[1-10]: 7
Why: Adding error handling test cases is important for robustness, ensuring the system handles invalid handler IDs gracefully. This improves code reliability and documentation.
@diemol now it will be ready to merge, but I noticed some issues with other examples outside of this PR, so I'm waiting to see if my changes fixed them so the Ruby pipeline is green again
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Description
This PR adds documentation for the work done in #13993
And it also adds examples for the upcoming Response handler method
Motivation and Context
To facilitate use and adoption of the new BiDi implementation is important that the documentation is up to date with relevant examples
Types of changes
Checklist
PR Type
Documentation, Enhancement
Description
Changes walkthrough 📝
network_spec.rb
Add BiDi network handler tests for Rubyexamples/ruby/spec/bidi/network_spec.rb
network.en.md
Add BiDi network handlers documentation with Ruby exampleswebsite_and_docs/content/documentation/webdriver/bidi/network.en.md
handlers
network.ja.md
Update Japanese translation with BiDi network documentationwebsite_and_docs/content/documentation/webdriver/bidi/network.ja.md
network.pt-br.md
Update Portuguese translation with BiDi network documentationwebsite_and_docs/content/documentation/webdriver/bidi/network.pt-br.md
network.zh-cn.md
Update Chinese translation with BiDi network documentationwebsite_and_docs/content/documentation/webdriver/bidi/network.zh-cn.md