Skip to content

[rb] add browser_family test guard and use it for chromium/safari families - #17854

Merged
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:rb-browser-family-guard
Aug 1, 2026
Merged

[rb] add browser_family test guard and use it for chromium/safari families#17854
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:rb-browser-family-guard

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

💥 What does this PR do?

  • Adds a browser_family spec guard to simplify expressions

🔧 Implementation Notes

  • Families are :chromium for all edge/chrome and :safari for safari and tech preview
  • Derived from the existing browser value in TestEnvironment, so no new environment variable

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: browser_family derivation, guard conversions, and docs
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

@selenium-ci selenium-ci added C-rb Ruby Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Aug 1, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Add browser_family spec guard and migrate Chromium/Safari guards

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Introduce browser_family guard derived from the existing browser test environment.
• Update integration specs to use browser_family for Chromium/Safari-wide skips/pending/flaky.
• Document browser_family usage and guidance to prefer families over enumerating browsers.
Diagram

graph TD
  A["TestEnvironment"] --> B["Derive browser_family"] --> C["spec_helper registers conditions"] --> D["Guards evaluator"] --> E["RSpec integration specs"]
  F["TESTING.md"] --> E
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Extend `browser` condition to support named groups
  • ➕ Avoids introducing a new condition key
  • ➕ Keeps all browser matching logic in one place
  • ➖ More magical semantics for browser values (symbols meaning either concrete browser or group)
  • ➖ Harder to document and reason about than an explicit browser_family key
2. Add a more explicit `browser_engine` condition
  • ➕ Terminology aligns with the underlying rendering/automation engine concept
  • ➕ Scales if you later need :gecko/:webkit/:chromium as engines
  • ➖ Naming bikeshed and potential mismatch with existing project vocabulary
  • ➖ Requires migrating specs similarly, with no practical win over browser_family today

Recommendation: browser_family is the best trade-off here: it is explicit, derived from existing configuration (no new env vars), and makes guards more readable/maintainable while keeping channel-specific cases (:safari_preview, Chrome beta, etc.) on the existing browser condition.

Files changed (25) +105 / -83

Enhancement (2) +12 / -0
spec_helper.rbRegister 'browser_family' as a guard condition for examples +1/-0

Register 'browser_family' as a guard condition for examples

• Adds ':browser_family' to the guard condition set so metadata can match on browser engine family.

rb/spec/integration/selenium/webdriver/spec_helper.rb

test_environment.rbDerive 'browser_family' from the selected browser +11/-0

Derive 'browser_family' from the selected browser

• Introduces a 'browser_family' helper that maps Chrome/Edge to ':chromium' and Safari/Safari Preview to ':safari', otherwise returning the original browser symbol.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb

Tests (22) +83 / -83
action_builder_spec.rbUse 'browser_family' guards for Safari and Chromium cases +10/-10

Use 'browser_family' guards for Safari and Chromium cases

• Replaces Safari/Safari Preview browser lists with 'browser_family: :safari' for pending/skip/flaky metadata. Replaces Chrome/Edge lists with 'browser_family: :chromium' for scroll-related skip_unless guards.

rb/spec/integration/selenium/webdriver/action_builder_spec.rb

browser_spec.rbSwitch BiDi browser specs to 'browser_family: :safari' pending guards +5/-5

Switch BiDi browser specs to 'browser_family: :safari' pending guards

• Updates multiple pending guards to target the Safari family instead of listing ':safari' and ':safari_preview' explicitly.

rb/spec/integration/selenium/webdriver/bidi/browser_spec.rb

browsing_context_spec.rbUse 'browser_family' for Chromium and Safari BiDi browsing context guards +4/-4

Use 'browser_family' for Chromium and Safari BiDi browsing context guards

• Converts Chrome/Edge pending guards to 'browser_family: :chromium' and Safari/Safari Preview guards to 'browser_family: :safari'.

rb/spec/integration/selenium/webdriver/bidi/browsing_context_spec.rb

network_spec.rbNormalize Safari BiDi network guards to 'browser_family' +11/-11

Normalize Safari BiDi network guards to 'browser_family'

• Replaces repeated 'browser: %i[safari safari_preview]' pending guards with 'browser_family: :safari', including combined pending conditions alongside Firefox.

rb/spec/integration/selenium/webdriver/bidi/network_spec.rb

protocol_browser_spec.rbApply 'browser_family: :safari' to BiDi protocol browser tests +6/-6

Apply 'browser_family: :safari' to BiDi protocol browser tests

• Updates Safari-related pending guards to use 'browser_family' across user context and client windows tests.

rb/spec/integration/selenium/webdriver/bidi/protocol_browser_spec.rb

protocol_browsing_context_spec.rbApply 'browser_family' guards in BiDi protocol browsing context tests +4/-4

Apply 'browser_family' guards in BiDi protocol browsing context tests

• Converts Chrome/Edge pending to 'browser_family: :chromium' and Safari/Safari Preview pending to 'browser_family: :safari'.

rb/spec/integration/selenium/webdriver/bidi/protocol_browsing_context_spec.rb

script_spec.rbUse 'browser_family: :safari' for BiDi script logging pending guards +4/-4

Use 'browser_family: :safari' for BiDi script logging pending guards

• Replaces explicit Safari/Safari Preview lists with a single Safari family guard for multiple console/error log tests.

rb/spec/integration/selenium/webdriver/bidi/script_spec.rb

bidi_spec.rbUse Safari family guard for BiDi session status behavior +2/-2

Use Safari family guard for BiDi session status behavior

• Updates pending guards for BiDi session lifecycle assertions to apply to 'browser_family: :safari'.

rb/spec/integration/selenium/webdriver/bidi_spec.rb

devtools_spec.rbGuard DevTools specs with 'browser_family: :chromium' +1/-1

Guard DevTools specs with 'browser_family: :chromium'

• Replaces the Chrome/Edge browser list in 'skip_unless' with a single Chromium family guard.

rb/spec/integration/selenium/webdriver/devtools_spec.rb

driver_spec.rbReplace Safari list guards with 'browser_family: :safari' +5/-5

Replace Safari list guards with 'browser_family: :safari'

• Updates skip/pending conditions that were specific to Safari/Safari Preview to use the Safari family guard.

rb/spec/integration/selenium/webdriver/driver_spec.rb

element_spec.rbStandardize Safari-specific element behavior guards via 'browser_family' +7/-7

Standardize Safari-specific element behavior guards via 'browser_family'

• Converts multiple Safari-specific pending guards from 'browser: :safari' or Safari lists to 'browser_family: :safari', keeping Firefox-specific conditions unchanged.

rb/spec/integration/selenium/webdriver/element_spec.rb

fedcm_spec.rbUse Chromium family guard for FedCM specs +2/-2

Use Chromium family guard for FedCM specs

• Updates 'skip_unless' and 'pending_if' metadata to target 'browser_family: :chromium' instead of listing Chrome/Edge.

rb/spec/integration/selenium/webdriver/fedcm_spec.rb

manager_spec.rbUse Safari family guard for cookie manager behavior differences +2/-2

Use Safari family guard for cookie manager behavior differences

• Replaces Safari/Safari Preview pending guards with 'browser_family: :safari' for cookie domain and SameSite behaviors.

rb/spec/integration/selenium/webdriver/manager_spec.rb

navigation_spec.rbGuard Safari BiDi history traversal with 'browser_family: :safari' +1/-1

Guard Safari BiDi history traversal with 'browser_family: :safari'

• Updates BiDi traversal pending condition to match the Safari family rather than enumerating Safari channels.

rb/spec/integration/selenium/webdriver/navigation_spec.rb

network_spec.rbUse Safari family guard for BiDi network domain availability +1/-1

Use Safari family guard for BiDi network domain availability

• Updates the top-level 'pending_if' to apply to 'browser_family: :safari' for Safari’s missing BiDi network support.

rb/spec/integration/selenium/webdriver/network_spec.rb

driver_spec.rbSwitch Safari driver spec gating to 'browser_family: :safari' +1/-1

Switch Safari driver spec gating to 'browser_family: :safari'

• Updates the describe-level 'skip_unless' condition to use the Safari family instead of listing Safari channels.

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

select_spec.rbUse Safari family guard for disabled option exception behavior +7/-7

Use Safari family guard for disabled option exception behavior

• Replaces multiple 'skip_if: {browser: :safari}' guards with 'skip_if: {browser_family: :safari}' across select interaction tests.

rb/spec/integration/selenium/webdriver/select_spec.rb

shadow_root_spec.rbUse Safari family guard for shadow root exception differences +3/-3

Use Safari family guard for shadow root exception differences

• Converts Safari-specific skip guards to 'browser_family: :safari' for no-root/detached-root/script retrieval cases.

rb/spec/integration/selenium/webdriver/shadow_root_spec.rb

target_locator_spec.rbUse Safari family guard for multi-window pending condition +1/-1

Use Safari family guard for multi-window pending condition

• Replaces the Safari/Safari Preview pending condition with 'browser_family: :safari' while preserving the IE remote condition.

rb/spec/integration/selenium/webdriver/target_locator_spec.rb

timeout_spec.rbUse Safari family guard for click-triggered load timeout behavior +1/-1

Use Safari family guard for click-triggered load timeout behavior

• Updates the pending condition to apply to 'browser_family: :safari' rather than enumerating Safari channels.

rb/spec/integration/selenium/webdriver/timeout_spec.rb

virtual_authenticator_spec.rbGuard VirtualAuthenticator specs with 'browser_family: :chromium' +1/-1

Guard VirtualAuthenticator specs with 'browser_family: :chromium'

• Replaces the Chrome/Edge list in 'skip_unless' with a single Chromium family condition.

rb/spec/integration/selenium/webdriver/virtual_authenticator_spec.rb

window_spec.rbApply 'browser_family' to window management pending/skip/flaky guards +4/-4

Apply 'browser_family' to window management pending/skip/flaky guards

• Converts Chrome/Edge and Safari/Safari Preview guards to 'browser_family: :chromium' and 'browser_family: :safari' respectively for fullscreen/minimize-related metadata.

rb/spec/integration/selenium/webdriver/window_spec.rb

Documentation (1) +10 / -0
TESTING.mdDocument new 'browser_family' guard and usage guidance +10/-0

Document new 'browser_family' guard and usage guidance

• Adds 'browser_family' to the guard conditions table and explains when it should be preferred over enumerating browsers. Includes a new example showing 'pending_if: {browser_family: :chromium}'.

rb/TESTING.md

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. browser_family mapping untested 📘 Rule violation ≡ Correctness
Description
The PR introduces new browser_family derivation logic used for many guards, but does not add any
unit/spec coverage to validate the mapping for key browsers (e.g., :chrome/:edge:chromium,
:safari_preview:safari). This makes guard behavior prone to silent regressions and unexpected
spec skipping/pending.
Code

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[R69-78]

+        def browser_family
+          case browser
+          when :chrome, :edge
+            :chromium
+          when :safari, :safari_preview
+            :safari
+          else
+            browser
+          end
+        end
Evidence
PR Compliance ID 3 requires adding tests for new features when feasible. The PR adds new behavior
(browser_family mapping) at the cited lines, but no accompanying unit/spec asserts the expected
mapping outcomes.

AGENTS.md: Include tests for fixes/features; prefer small unit tests and avoid mocks
rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[69-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TestEnvironment#browser_family` was added/changed without direct test coverage, so its browser-to-family mapping can regress and silently change which specs are skipped/pending.

## Issue Context
The new `browser_family` condition is now used widely across integration specs, so a small mapping error can affect large portions of the test suite.

## Fix Focus Areas
- rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[69-78]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@titusfortner
titusfortner merged commit 83f26a2 into SeleniumHQ:trunk Aug 1, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-devtools Includes everything BiDi or Chrome DevTools related C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants