[rb] add safari tests

[titusfortner]

In Draft until we see what fails in the CI.

[qodo-code-review]

Review Summary by Qodo

Add Safari test guards and automatic pairing retry logic

🧪 Tests 🐞 Bug fix

Walkthroughs

Description

• Guard Safari tests with browser exclusions for known failures
• Implement automatic retry logic for Safari pairing conflicts
• Consolidate test guards from individual tests to describe blocks
• Remove manual driver reset delays and conditional resets

Diagram

flowchart LR
  A["Safari Test Issues"] --> B["Guard Failing Tests"]
  A --> C["Pairing Conflicts"]
  B --> D["Move guards to describe blocks"]
  B --> E["Exclude safari/safari_preview"]
  C --> F["Add retry mechanism"]
  F --> G["Detect pairing error"]
  F --> H["Sleep and retry"]
  D --> I["Updated Test Specs"]
  E --> I
  H --> I

Loading

File Changes

1. rb/spec/integration/selenium/webdriver/action_builder_spec.rb 🧪 Tests +14/-14

Move Safari exclusions to describe blocks

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

---

2. rb/spec/integration/selenium/webdriver/driver_spec.rb 🧪 Tests +1/-1

Guard page refresh test for Safari

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

---

3. rb/spec/integration/selenium/webdriver/element_spec.rb 🧪 Tests +6/-9

Remove manual resets and add Safari guards

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

---

View more (6)

4. rb/spec/integration/selenium/webdriver/manager_spec.rb 🧪 Tests +1/-1

Add Safari to cookie cleanup condition

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

---

5. rb/spec/integration/selenium/webdriver/navigation_spec.rb 🧪 Tests +2/-2

Guard navigation tests for Safari

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

---

6. rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb 🐞 Bug fix +18/-2

Add Safari pairing retry and remove sleep delays

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

---

7. rb/spec/integration/selenium/webdriver/target_locator_spec.rb 🧪 Tests +15/-13

Guard window and alert tests for Safari

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

---

8. rb/spec/integration/selenium/webdriver/timeout_spec.rb 🧪 Tests +3/-2

Guard implicit wait tests for Safari

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

---

9. rb/spec/integration/selenium/webdriver/window_spec.rb 🧪 Tests +3/-2

Update fullscreen and minimize guards for Safari

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

---

[qodo-code-review]

Code Review by Qodo

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

1. Fullscreen test ungated for headless ✓ Resolved 📘 Rule violation ☼ Reliability

Description

The window.full_screen spec no longer excludes headless Chrome/Edge, so it may run in environments
where fullscreen is unsupported and fail CI. Tests that depend on optional browser/runtime
capabilities must be gated to avoid false failures.

Code

rb/spec/integration/selenium/webdriver/window_spec.rb[116]

Evidence

PR Compliance ID 11 requires capability-dependent tests to be skipped when the feature is
unsupported. The updated spec at window_spec.rb:116 removed the prior headless exclusion, so the
fullscreen behavior will be tested in headless configurations where it may not work reliably.

rb/spec/integration/selenium/webdriver/window_spec.rb[116-129]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
A fullscreen window test is now allowed to run in headless Chrome/Edge, which commonly does not support fullscreen behavior and can cause CI-only failures.

## Issue Context
Previously the spec excluded `{browser: %i[chrome edge], headless: true}`. The PR changed the exclusion to Safari/GitHub CI only, removing the headless gating.

## Fix Focus Areas
- rb/spec/integration/selenium/webdriver/window_spec.rb[116-129]

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

---

2. Infinite Safari driver retry ✓ Resolved 🐞 Bug ☼ Reliability

Description

TestEnvironment#create_driver! resets @safari_pairing_attempts to 0 before the rescue, so when
retry is triggered the method restarts and the counter is reset again, meaning the retry limit in
safari_pairing_retry? is never reached and Safari session creation can loop forever on persistent
“instance is already paired” errors.

Code

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[R205-208]

Evidence

The retry counter is set to 0 before driver instantiation, and the retry occurs before the error
is recorded; since retry re-executes the protected method body, the counter is reset each time and
can’t ever reach the stop condition in safari_pairing_retry?.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[202-225]
rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[251-264]

Agent prompt

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

### Issue description
`create_driver!` sets `@safari_pairing_attempts = 0` inside the retryable region. Because Ruby’s `retry` re-runs the method body from the top of the `begin` region, this assignment executes again, preventing `@safari_pairing_attempts` from accumulating across retries.

### Issue Context
This can cause an unbounded retry loop when SafariDriver repeatedly returns the pairing error.

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

### Implementation notes
- Initialize attempts outside the retried section, e.g. `@safari_pairing_attempts ||= 0` (or use a local `attempts` variable defined before the `begin`).
- Reset attempts only after a successful driver creation (or when starting a brand-new create sequence, not on each retry).

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

---

3. Safari retry counter sticks 🐞 Bug ☼ Reliability

Description

TestEnvironment#create_driver! initializes @safari_pairing_attempts with ||=, so if a Safari
pairing failure exhausts retries and the method raises before the success-path reset, the counter
remains at the max and subsequent create_driver! calls will never retry. This can turn a transient
Safari Grid pairing contention into repeated immediate failures across later examples in the same
test process.

Code

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[205]

Evidence

The counter is only initialized (not reset) at driver creation start, and the retry decision is
based on the current value; after an exhausted failure, the value can remain at the limit and
short-circuit all future retries.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[202-226]
rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[252-265]

Agent prompt

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

### Issue description
Safari pairing retries are tracked via `@safari_pairing_attempts ||= 0`. If retries are exhausted and `create_driver!` raises, the instance variable can remain at `SAFARI_PAIRING_RETRIES`, and future driver instantiations in the same process will not retry because the counter is already at/above the limit.

### Issue Context
`@safari_pairing_attempts` is reset to 0 only on the success path after an instance is created. When the failure path exits after exhausting retries, the counter is not reset.

### Fix Focus Areas
- rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[202-226]
- rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[252-265]

### Suggested fix approach
Replace the instance-variable retry counter with a per-invocation local counter using an explicit `begin ... rescue ... retry` block, e.g.:
- `safari_attempts = 0` outside the `begin`
- in `rescue`, if pairing-busy error and `safari_attempts < SAFARI_PAIRING_RETRIES`, increment `safari_attempts`, sleep, and `retry`
This avoids cross-example state and ensures each new driver instantiation gets up to `SAFARI_PAIRING_RETRIES` retries regardless of prior failures.

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

---

4. Retry limit hardcoded ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

safari_pairing_retry? compares attempts against a literal 5 instead of SAFARI_PAIRING_RETRIES,
so updating the constant won’t actually change behavior and can create a confusing mismatch.

Code

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[R251-264]

Evidence

The method defines SAFARI_PAIRING_RETRIES = 5 but does not use it in the actual limit check.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[251-264]

Agent prompt

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

### Issue description
`SAFARI_PAIRING_RETRIES` is defined but the limit check uses a magic number.

### Issue Context
Keeping the limit check tied to the constant prevents future drift.

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

### Implementation notes
- Replace `@safari_pairing_attempts >= 5` with `@safari_pairing_attempts >= SAFARI_PAIRING_RETRIES`.

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

---

5. Only-to-except guard widening 🐞 Bug ☼ Reliability

Description

The scroll out-of-viewport test guard changed from only: {browser: %i[chrome edge firefox]} to
except: {browser: %i[safari safari_preview]}, so it will now run on any non-Safari browser
configuration rather than the previously explicit supported set.

Code

rb/spec/integration/selenium/webdriver/action_builder_spec.rb[R392-394]

Evidence

The guard framework treats only and except oppositely; switching from only to except
broadens the set of configurations where the test runs.

rb/spec/integration/selenium/webdriver/action_builder_spec.rb[392-400]
rb/lib/selenium/webdriver/support/guards.rb[50-93]
rb/lib/selenium/webdriver/support/guards/guard_condition.rb[41-45]

Agent prompt

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

### Issue description
Changing from `only` to `except` materially changes guard semantics: the example now runs on all configurations except the excluded ones.

### Issue Context
RSpec guard behavior is implemented by `Selenium::WebDriver::Support::Guards`:
- `only` pendings when the current value is NOT in the list
- `except` pendings only when the current value IS in the list

### Fix Focus Areas
- rb/spec/integration/selenium/webdriver/action_builder_spec.rb[392-399]

### Implementation notes
- If the test is intended for a specific set of browsers, revert to `only: {browser: %i[...]}`.
- If it’s intended to run broadly, consider adding explicit excludes for any known-unsupported browsers/drivers (instead of relying on an implicit “everything but Safari”).

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

---

View more (1)

6. Safari preview not reset ✓ Resolved 🐞 Bug ☼ Reliability

Description

Cookie cleanup in manager_spec.rb resets the driver for :safari but not for :safari_preview,
which is inconsistent with the rest of the suite’s Safari guards and can leave state behind when
running Safari Preview.

Code

rb/spec/integration/selenium/webdriver/manager_spec.rb[R29-32]

Evidence

The cookie suite resets only when GlobalTestEnv.browser == :safari, while other specs in the same
test suite consistently guard both Safari variants together.

rb/spec/integration/selenium/webdriver/manager_spec.rb[25-34]
rb/spec/integration/selenium/webdriver/action_builder_spec.rb[27-29]

Agent prompt

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

### Issue description
The after-hook special-cases Safari but omits Safari Preview.

### Issue Context
Most specs in this PR treat `safari` and `safari_preview` together via `%i[safari safari_preview]`.

### Fix Focus Areas
- rb/spec/integration/selenium/webdriver/manager_spec.rb[28-34]

### Implementation notes
- Change the condition to include both, e.g. `%i[safari safari_preview].include?(GlobalTestEnv.browser)` (or `browser.to_s.include?('safari')`).

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

---

[qodo-code-review]

Persistent review updated to latest commit 5b64282