Skip to content

test(e2e): report what clickWhenStill last saw when it times out - #2219

Merged
chhoumann merged 1 commit into
masterfrom
fix/2218-phone-spec-after-2215
Oct 9, 2026
Merged

chhoumann merged 1 commit into
masterfrom
fix/2218-phone-spec-after-2215

Conversation

@chhoumann

@chhoumann chhoumann commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Why

The phone spec in #2218 fails on macOS with "visible and still" timeouts. That message reads the same for an element that is missing, one that is hidden, and one that keeps moving. The failure does not reproduce on Linux, so the next macOS run has to say which one it is.

What changed

  • clickWhenStill now ends its timeout message with its last two samples: "absent", "hidden", or the element's center.

Scope

Blast Radius

Test helpers only. Seven specs use clickWhenStill. All seven still pass.

Verification

  • A throwaway spec forced each case on the isolated instance. The three messages ended Last samples: "absent", "absent"., "hidden", "hidden"., and {"x":126.96875,"y":19.75}, {"x":161.96875,"y":19.75}.
  • .agents/run-e2e on all seven caller specs with Obsidian 1.14.4 on Linux: 7 files and 24 tests passed.
  • The full native suite on master, Linux, Obsidian 1.14.4: 96 files passed and 2 skipped. The phone spec passed in that run.

Refs #2218

Note

Improve clickWhenStill timeout diagnostics in e2e tests

The clickWhenStill helper in uiHelpers.ts now tracks why a click wait failed. It distinguishes three states: selector absent, selector present but hidden, and selector still moving.

  • Samples now record explicit "absent"/"hidden" status instead of a nullable coordinate object
  • Checks all matching elements and separates "no matches" from "matches with no visible client rectangle"
  • Requires two consecutive coordinate samples before clicking
  • Timeout errors include the two most recent samples

Macroscope summarized 972e99b.

Summary by CodeRabbit

  • Tests
    • Improved end-to-end UI test click handling for hidden and missing elements.
    • Timeout errors now include recent visibility samples for easier diagnosis.

A timeout said only "visible and still", so a missing element, a hidden
one, and one that keeps moving all read the same. The message now ends
with the last two samples: "absent", "hidden", or the element's center.

Refs #2218
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 64c6401b-8760-4078-a29a-52231b5d74ef

📥 Commits

Reviewing files that changed from the base of the PR and between e772433 and 972e99b.


📒 Files selected for processing (1)
  • tests/e2e/uiHelpers.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.



📝 Walkthrough

Walkthrough

The clickWhenStill helper now distinguishes missing matches from hidden matches during polling. It also includes the last two sampled states in timeout errors.

Changes

Click sampling

Layer / File(s) Summary
Sample visible matches and report timeouts
tests/e2e/uiHelpers.ts
clickWhenStill records missing and hidden matches as distinct states. It still clicks the center of a visible match after consecutive samples agree. On timeout, it appends the last two samples to the error before rethrowing.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other · Severity of issue fixed: Low


Merge Risk: ⚪ Minimal · up to 972e9

This change improves diagnostics for click timeouts but does not fix the separately reported macOS positioning failure. The diagnostic path behaves as intended, and no PR-introduced merge-blocking risk is established.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: adding timeout diagnostics to report the last states observed by clickWhenStill.
Linked Issues check Passed The PR addresses the coding objective derived from open issue #2218: improve clickWhenStill diagnostics without fixing the underlying macOS failure. tests/e2e/uiHelpers.ts records "absent", `"hi…
Out of Scope Changes check Passed The reviewed change is limited to tests/e2e/uiHelpers.ts and supports the diagnostic objective for #2218. The helper keeps the success click path and test-helper scope. No unrelated product behavior…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.


✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: package-manager metadata or lockfile failed a supply-chain integrity policy. Refresh the packageManager pin and lockfile locally.



Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the page in flight,
It spots the hidden, finds the sight.
Two steady samples, then a click,
If time runs out, the clues come quick.
It hops away beneath the moon.

Comment @coderabbitai help to get the list of available commands.

@chhoumann

Copy link
Copy Markdown
Owner Author

Verdict: PASS+NOTES
Head: 972e99b
Base: e772433

  • The seven specs that call clickWhenStill pass at head: 7/7 files and 24/24 tests on a native Linux instance. Note: the first full run had 1/24 fail in choice-builder-pages "opens a macro's branch...", where the click went through but the Log page never opened (load average about 57). A rerun of that spec alone passed twice (12/12) and the full rerun passed 24/24, so I count it as a flake and not something this PR caused.
  • A throwaway spec made each case time out on purpose. At head the messages end with: absent Last samples: "absent", "absent"., display:none Last samples: "hidden", "hidden"., and an element moved every 20ms Last samples: {"x":171,"y":110}, {"x":178,"y":110}.
  • With the parent's uiHelpers.ts, the same three cases all end with visible and still after 10000ms. and the suffix is missing.
  • tsc --noEmit and eslint tests/e2e/uiHelpers.ts are clean. The throwaway spec was deleted and the worktree is clean.

@chhoumann
chhoumann merged commit 1a3fe4b into master Oct 9, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

e2e: choice-builder-pages-phone fails since #2215 (row never holds still after reload)

1 participant