Skip to content

[rb] allow pending test guards to require matching provided exception - #17859

Merged
titusfortner merged 3 commits into
SeleniumHQ:trunkfrom
titusfortner:exception-aware-pending-if
Aug 2, 2026
Merged

[rb] allow pending test guards to require matching provided exception#17859
titusfortner merged 3 commits into
SeleniumHQ:trunkfrom
titusfortner:exception-aware-pending-if

Conversation

@titusfortner

Copy link
Copy Markdown
Member

💥 What does this PR do?

Lets a Ruby test guard require a specific failure: a guarded example is marked pending only when it fails for the expected reason. If it fails a different way, still passes, or times out, it fails as usual — so a pending guard can no longer silently hide a genuinely broken path or a bad test payload.

it 'emulates scrollbars', pending_if: {browser: :firefox,
                                       exception: {class: Selenium::WebDriver::Error::WebDriverError,
                                                   message: /\Aunknown command:/},
                                       reason: 'Firefox does not implement emulation.setScrollbarTypeOverride'} do
  # ...
end

exception: takes {class:, message:}; message: is optional and follows RSpec's raise_error semantics (Regexp as pattern, String as exact match).

When the example fails for the wrong reason, the original failure is preserved and annotated with what was expected:

Expected test to fail with Selenium::WebDriver::Error::WebDriverError: /\Aunknown command:/; Test guarded; pending if {:browser=>:firefox, :exception=>{:class=>Selenium::WebDriver::Error::WebDriverError, :message=>/\Aunknown command:/}, :reason=>"Firefox does not implement emulation.setScrollbarTypeOverride"};

🔧 Implementation Notes

  • The failure isn't known until the example runs, so exception-aware guards are resolved in an around hook; plain pending, eager skip, and SKIP_PENDING behave as before.
  • Guard#initialize now dups the guarded hash before defaulting :reason, fixing a pre-existing FrozenError on frozen/shared guard constants.

🤖 AI assistance

  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: guard matching logic, the spec-harness around hook, unit tests, and docs
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

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

@selenium-ci selenium-ci added C-rb Ruby Bindings B-support Issue or PR related to support classes labels Aug 2, 2026
@selenium-ci

Copy link
Copy Markdown
Member

Thank you, @titusfortner for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Exception-aware pending_if guards that only pend on expected failures

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Add exception: matching to pending_if/except guards to pend only on expected failures.
• Resolve exception-aware pending in an RSpec around hook, preserving unexpected failures.
• Update guard messaging/immutability, plus new unit tests and documentation.
Diagram

graph TD
  A("RSpec example") --> B["spec_helper.rb hooks"] --> C["Support::Guards"] --> D{"pending guard?"}
  D --> E["before: skip/pending"]
  D --> F["around: resolve exception"] --> G["RSpec Pending API"]
  E --> G
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Implement as an RSpec formatter/listener post-processing step
  • ➕ Avoids relying on shared @guards state across hooks
  • ➕ Keeps the example lifecycle logic in a single reporting extension
  • ➖ More complex to wire into the test harness
  • ➖ Harder to integrate cleanly with existing skip/pending control flow
2. Have `Guards#disposition` return a deferred action object (e.g., lambda)
  • ➕ Avoids adding a separate pending_exception_guard query method
  • ➕ Encapsulates pending resolution strategy in the Guards subsystem
  • ➖ Requires a bigger API change and more refactoring across callers
  • ➖ Harder to keep backwards compatibility with existing send(*results) usage

Recommendation: The current approach (introduce pending_exception_guard and resolve it in an around hook) is the best tradeoff: it keeps existing eager skip/pending behavior intact, adds minimal new surface area, and performs exception matching at the only time it’s knowable (after execution) while preserving unexpected failures with an explicit annotation.

Files changed (8) +210 / -49

Enhancement (3) +76 / -18
guards.rbDefer exception-aware pending guard selection and improve SKIP_PENDING message +8/-2

Defer exception-aware pending guard selection and improve SKIP_PENDING message

• Updates 'SKIP_PENDING' behavior to emit an explicit skip reason prefix. Adds 'pending_exception_guard' to expose the active pending guard only when it carries an 'exception:' clause and wasn’t otherwise resolved.

rb/lib/selenium/webdriver/support/guards.rb

guard.rbAdd exception matching to guards and avoid mutating frozen guarded hashes +20/-3

Add exception matching to guards and avoid mutating frozen guarded hashes

• Duplicates the incoming guarded hash before defaulting ':reason' to prevent 'FrozenError' when metadata is shared/frozen. Adds 'exception?' and 'matches_exception?' helpers to support exception-aware pending, and improves guard detail strings to include guard type.

rb/lib/selenium/webdriver/support/guards/guard.rb

spec_helper.rbResolve exception-aware pending guards via RSpec around hook +48/-13

Resolve exception-aware pending guards via RSpec around hook

• Extracts guard setup into 'create_guards' and introduces 'resolve_pending_exception' to mark pending only when the raised exception matches the guard’s spec. Switches to an 'around' hook to run the example before deciding pending, while retaining a separate 'before' hook for traditional skip/pending disposition.

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

Tests (1) +113 / -31
guard_spec.rbAdd unit coverage for exception-aware pending and updated messaging +113/-31

Add unit coverage for exception-aware pending and updated messaging

• Updates expectations for new guard message formatting and SKIP_PENDING behavior. Adds tests for 'pending_exception_guard', guard hash immutability, and exception matching semantics (class-only, regexp message, exact string message).

rb/spec/unit/selenium/webdriver/guard_spec.rb

Documentation (1) +14 / -0
TESTING.mdDocument exception-aware pending guard syntax and semantics +14/-0

Document exception-aware pending guard syntax and semantics

• Documents 'exception: {class:, message:}' support for 'pending_if'/'except', including 'raise_error'-style message matching and an example.

rb/TESTING.md

Other (3) +7 / -0
.rubocop.ymlExclude integration spec helper from RSpec instance-variable RuboCop rule +1/-0

Exclude integration spec helper from RSpec instance-variable RuboCop rule

• Adds 'spec/integration/selenium/webdriver/spec_helper.rb' to 'RSpec/InstanceVariable' exclusions to allow '@guards' usage in the harness.

rb/.rubocop.yml

guards.rbsAdd RBS signature for 'pending_exception_guard' +2/-0

Add RBS signature for 'pending_exception_guard'

• Extends the 'Guards' RBS interface to include the new 'pending_exception_guard' method.

rb/sig/lib/selenium/webdriver/support/guards.rbs

guard.rbsAdd RBS signatures for exception-aware guard helpers +4/-0

Add RBS signatures for exception-aware guard helpers

• Adds 'exception?' and 'matches_exception?' to the Guard RBS interface for type coverage of the new behavior.

rb/sig/lib/selenium/webdriver/support/guards/guard.rbs

@qodo-code-review

qodo-code-review Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Pending guard precedence broken ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Guards#disposition now defers pending whenever the *selected* pending_guard is exception-aware, even
if another active pending guard without an exception clause exists; those examples will run
unexpectedly instead of being marked pending up-front. This happens because pending_guard returns
only the first satisfied pending guard, and disposition does not search for a non-exception pending
guard before deferring.
Code

rb/lib/selenium/webdriver/support/guards.rb[R55-57]

+            [:skip, "(skipped by SKIP_PENDING) #{pending_guard.message}"]
+          elsif !pending_guard.nil? && !pending_guard.exception?
            [:pending, pending_guard.message]
Evidence
disposition checks only the single pending_guard and suppresses immediate pending when that guard is
exception-aware; pending_guard itself selects only the first satisfied pending guard. Because guards
are collected in a deterministic order, an exception-aware guard can precede a normal pending guard
and unintentionally force deferral even though a normal pending guard is active.

rb/lib/selenium/webdriver/support/guards.rb[51-58]
rb/lib/selenium/webdriver/support/guards.rb[73-87]
rb/lib/selenium/webdriver/support/guards.rb[97-100]
rb/lib/selenium/webdriver/support/guards/guard.rb[67-90]

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

## Issue description
`Guards#disposition` decides whether to mark an example `:pending` based only on the single `pending_guard` returned by `pending_guard`. After this PR, if that guard is exception-aware (`exception? == true`), `disposition` returns `nil` (defers handling), even when there is another satisfied pending guard without an `exception:` clause that should still immediately mark the example pending.

This can cause examples to execute (and potentially fail or become "fixed pending") even though a non-exception pending guard is active.

## Issue Context
- `pending_guard` returns the first satisfied pending guard based on guard collection order.
- `disposition` now checks `!pending_guard.exception?` and otherwise returns nil.

## Fix Focus Areas
- rb/lib/selenium/webdriver/support/guards.rb[51-65]
- rb/lib/selenium/webdriver/support/guards.rb[73-100]

## Suggested fix approach
1. When determining immediate disposition, compute the set of *active* pending guards (satisfied `except?` and unsatisfied `only?`).
2. If any active pending guard is **non-exception** (`!exception?`), return `[:pending, that_guard.message]` (preserving existing ordering rules).
3. Only if there are **no** active non-exception pending guards, then allow deferral for an exception-aware guard via `pending_exception_guard`.

A minimal implementation could:
- Add a helper like `active_pending_guards` and use it in both `disposition` and `pending_exception_guard`.
- Keep the existing first-match semantics within each category (non-exception first, exception-aware fallback) to preserve expected ordering for arrays-of-guards.

ⓘ 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

Comment thread rb/lib/selenium/webdriver/support/guards.rb
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit a6cbf1b

@titusfortner titusfortner changed the title [rb] Exception-aware pending_if test guards [rb] allow pending test guards to require matching provided exception Aug 2, 2026
@titusfortner
titusfortner merged commit 4c6f3a5 into SeleniumHQ:trunk Aug 2, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-support Issue or PR related to support classes C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants