[rb] Fix Network issue by removing nil values on network requests

[aguspe]

💥 What does this PR do?

This PR resolves a bug where optional, unspecified parameters were being sent as null in the JSON payload for network modification commands (network.continueRequest, network.continueResponse, network.provideResponse).

This was causing errors, particularly with the Firefox BiDi implementation, which expects optional fields to be nil.

The fix uses the Ruby method .compact to remove any key-value pairs with a nil value from the command arguments hash before serialization.

---

🔧 Implementation Notes

The core of the issue was that parameters like cookies, headers, or body, when left unprovided by the user, defaulted to nil.

When constructing the command hash (e.g., in #continue_request), these nil values were present, and upon JSON serialization, they became JSON null values.

We called .compact on the hash arguments to remove nil values:

 args = { request: args[:id], body: args[:body], cookies: args[:cookies], # ... }.compact

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-merge-pro]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided - [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </details></td></tr>
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Fix incorrect test assertion order Suggestion Impact: The commit re-ordered the test steps so that the network.* method is called before the have_received expectation in multiple tests, matching the suggested fix. code diff: it 'sends only the mandatory request ID when all optional args are nil' do expected_payload = {request: request_id} + network.continue_request(id: request_id) + expect(mock_bidi).to have_received(:send_cmd).with('network.continueRequest', expected_payload) - - network.continue_request(id: request_id) end it 'sends only provided optional args' do @@ -47,8 +47,6 @@ method: 'POST' } - expect(mock_bidi).to have_received(:send_cmd).with('network.continueRequest', expected_payload) - network.continue_request( id: request_id, body: {type: 'string', value: 'new body'}, @@ -56,6 +54,8 @@ headers: nil, method: 'POST' ) + + expect(mock_bidi).to have_received(:send_cmd).with('network.continueRequest', expected_payload) end end @@ -63,9 +63,9 @@ it 'sends only the mandatory request ID when all optional args are nil' do expected_payload = {request: request_id} + network.continue_response(id: request_id) + expect(mock_bidi).to have_received(:send_cmd).with('network.continueResponse', expected_payload) - - network.continue_response(id: request_id) end it 'sends only provided optional args' do @@ -76,8 +76,6 @@ statusCode: 202 } - expect(mock_bidi).to have_received(:send_cmd).with('network.continueResponse', expected_payload) - network.continue_response( id: request_id, cookies: nil, @@ -86,6 +84,8 @@ reason: nil, status: 202 ) + + expect(mock_bidi).to have_received(:send_cmd).with('network.continueResponse', expected_payload) end end @@ -93,9 +93,9 @@ it 'sends only the mandatory request ID when all optional args are nil' do expected_payload = {request: request_id} + network.provide_response(id: request_id) + expect(mock_bidi).to have_received(:send_cmd).with('network.provideResponse', expected_payload) - - network.provide_response(id: request_id) end it 'sends only provided optional args' do @@ -105,8 +105,6 @@ reasonPhrase: 'OK-Custom' } - expect(mock_bidi).to have_received(:send_cmd).with('network.provideResponse', expected_payload) - network.provide_response( id: request_id, body: {type: 'string', value: 'Hello'}, @@ -115,6 +113,8 @@ reason: 'OK-Custom', status: nil ) + + expect(mock_bidi).to have_received(:send_cmd).with('network.provideResponse', expected_payload) end In the RSpec tests, move the action (e.g., network.continue_request) before the expect(...).to have_received(...) assertion to correctly verify the behavior. rb/spec/unit/selenium/webdriver/bidi/network_spec.rb [35-41] it 'sends only the mandatory request ID when all optional args are nil' do expected_payload = {request: request_id} + network.continue_request(id: request_id) + expect(mock_bidi).to have_received(:send_cmd).with('network.continueRequest', expected_payload) - - network.continue_request(id: request_id) end [Suggestion processed] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical flaw in the test implementation where the assertion precedes the action, rendering all new tests ineffective at verifying the intended behavior.	High
Update

[diemol]

A few tests are consistently failing. Could you please have a look, @aguspe?

[titusfortner]

well, I don't think that's what I wanted to do. It doesn't want me to push from command line, so I tried to edit on Github, don't think it did what I wanted it to do...