Skip to content

Conversation

@titusfortner
Copy link
Member

@titusfortner titusfortner commented Oct 23, 2025

User description

does not look like the rubocop-rspec linter is being run with our current bazel command. Need to track that down.


PR Type

Enhancement


Description

  • Standardize Ruby hash formatting in RSpec tests

  • Remove spaces around hash braces per rubocop-rspec linter

  • Reorganize hash key ordering for consistency

  • Fix incorrect file path in RuboCop configuration


Diagram Walkthrough

flowchart LR
  A["RSpec Test Files"] -->|"Apply hash formatting rules"| B["Remove spaces in hashes"]
  B -->|"Reorder keys"| C["Standardized syntax"]
  D["RuboCop Config"] -->|"Fix file path"| E["Correct exclusion reference"]
Loading

File Walkthrough

Relevant files
Formatting
service_spec.rb
Standardize hash formatting in IE service spec                     

rb/spec/integration/selenium/webdriver/ie/service_spec.rb

  • Reformatted describe block hash parameters to remove spaces around
    braces
  • Reordered hash keys with exclude before exclusive
  • Split hash across multiple lines for improved readability
+3/-3     
service_spec.rb
Standardize hash formatting in Safari service spec             

rb/spec/integration/selenium/webdriver/safari/service_spec.rb

  • Reformatted describe block hash parameters to remove spaces around
    braces
  • Reordered hash keys with exclude before exclusive
  • Split hash across multiple lines for improved readability
+3/-2     
guards_spec.rb
Remove spaces in hash formatting throughout guards spec   

rb/spec/unit/selenium/webdriver/guards_spec.rb

  • Removed spaces around hash braces throughout test file (e.g., {
    condition: :guarded } to {condition: :guarded})
  • Reformatted multi-line hash parameters for consistent alignment
  • Applied consistent spacing rules across all guard test cases
+19/-18 
Configuration changes
.rubocop.yml
Fix incorrect file path in RuboCop configuration                 

rb/.rubocop.yml

  • Corrected file path in RSpec/NoExpectationExample exclusion
  • Changed from spec/integration/selenium/webdriver/guard_spec.rb to
    spec/unit/selenium/webdriver/guards_spec.rb
+1/-1     

@selenium-ci selenium-ci added the C-rb Ruby Bindings label Oct 23, 2025
@titusfortner titusfortner merged commit 0ecbd47 into trunk Oct 23, 2025
11 of 13 checks passed
@titusfortner titusfortner deleted the rb_rubocop branch October 23, 2025 18:27
@qodo-merge-pro
Copy link
Contributor

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.

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

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Safely stop launched service

Guard the stop call to always run and avoid potential nil errors if launch
fails. Use a safe navigation/operator or explicit nil check in the teardown.

rb/spec/integration/selenium/webdriver/ie/service_spec.rb [29-31]

 let(:service_manager) { service.launch }
 
-after { service_manager.stop }
+after { service_manager&.stop }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Ensure deterministic cleanup of external resources in tests by always stopping services in a guaranteed teardown (e.g., after/ensure), especially when launch may raise.

Low
Nil-safe teardown for service

Prevent teardown failures if launch fails by making the stop call nil-safe; this
guarantees cleanup without raising.

rb/spec/integration/selenium/webdriver/safari/service_spec.rb [29-31]

 let(:service_manager) { service.launch }
 
-after { service_manager.stop }
+after { service_manager&.stop }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Ensure deterministic cleanup of external resources in tests by always stopping services in a guaranteed teardown (e.g., after/ensure), especially when launch may raise.

Low
General
Update misleading test description

Update a misleading test description to accurately reflect that the only guard
requires all conditions in an array to be met, not just any single one.

rb/spec/unit/selenium/webdriver/guards_spec.rb [110-112]

-it 'guards if any Hash value is satisfied', only: [{condition: :guarded}, {condition: :not_guarded}] do
+it 'guards if not all Hash values are satisfied', only: [{condition: :guarded}, {condition: :not_guarded}] do
   raise 'This code is executed but expected to fail'
 end
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that the test description is misleading regarding the behavior of the only guard, and fixing it would improve the clarity and maintainability of the test suite.

Low
  • More

This was referenced Oct 25, 2025
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.

3 participants