Skip to content

[rb] fix Safari test failures - #17922

Merged
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:safari-viewport-guard-ci
Aug 18, 2026
Merged

[rb] fix Safari test failures#17922
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:safari-viewport-guard-ci

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

💥 What does this PR do?

  • Fixes failing Safari tests

🔧 Implementation Notes

  • The STP cask is not installing, presumed stale, so updating brew first.
  • The spec was written poorly so the error being surfaced was not the one we wanted to assert against (and we should never have a guard pending on an assertion failure).

🤖 AI assistance

  • AI assisted (complete below)
    • Tool(s): Claude Code (Opus 5)
    • What was generated: the CI log analysis that identified both failures, the fix, and this description
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • Bug fix (backwards compatible) — tests and CI only, no library code

@selenium-ci selenium-ci added C-rb Ruby Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Aug 17, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Stabilize Safari BiDi viewport spec and STP setup

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Refresh Homebrew before installing Safari Technology Preview in macOS CI.
• Compare complete BiDi remote values so Safari failures trigger the intended pending assertion.
Diagram

graph TD
  A["macOS CI"] --> B["Homebrew Update"] --> C["STP Install"] --> D["Safari Driver"] --> E["BiDi Specs"] --> F["Remote Value"]
Loading
High-Level Assessment

The approach is appropriately scoped: refreshing Homebrew directly before the STP installation addresses stale cask metadata, while comparing the complete BiDi remote value preserves protocol typing and ensures Safari's undefined response fails through the intended assertion. Retrying installation or extracting nested values would obscure the underlying failures without improving reliability.

Files changed (2) +8 / -3

Tests (1) +7 / -3
browsing_context_spec.rbCorrect Safari viewport pending assertion +7/-3

Correct Safari viewport pending assertion

• Compares the complete evaluated BiDi result against an expected array remote value instead of mapping nested values. The Safari pending guard no longer filters for an expectation exception, and its reason now documents Safari's undefined window-size response.

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

Other (1) +1 / -0
bazel.ymlRefresh Homebrew before installing Safari Technology Preview +1/-0

Refresh Homebrew before installing Safari Technology Preview

• Runs a quiet Homebrew update immediately before installing the Safari Technology Preview cask, preventing stale package metadata from breaking macOS CI setup.

.github/workflows/bazel.yml

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@titusfortner titusfortner changed the title [rb] fix Safari set_viewport spec guard and update Homebrew before STP install [rb] fix Safari test failures Aug 18, 2026
@titusfortner
titusfortner merged commit 194545f into SeleniumHQ:trunk Aug 18, 2026
28 checks passed
This was referenced Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations 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