Skip to content

test(playwright): re-resolve property-worker rows after the grid re-renders - #1456

Merged
renemadsen merged 4 commits into
masterfrom
test/stable-managing-tags-row
Oct 20, 2026
Merged

renemadsen merged 4 commits into
masterfrom
test/stable-managing-tags-row

Conversation

@renemadsen

Copy link
Copy Markdown
Member

Problem

r/property-worker-managing-tags.spec.ts › "a removed managing tag can be given back and saved" failed intermittently in CI (PR #1454, the master run after #1455, and work-items-planning-container run 39230664948). Each time a re-run passed. The error was:

Error: locator.scrollIntoViewIfNeeded: Element is not attached to the DOM

Root cause

row-action-menu.ts:27 called actionCell.scrollIntoViewIfNeeded(). A click re-resolves its locator when the element detaches, but Locator.scrollIntoViewIfNeeded does not. It resolves an ElementHandle once (_withElement) and throws when that element is detached while it waits for the element to be stable.

The property-workers grid replaces its rows after every save. updateTable() is a forkJoin of index-device-user, properties/dictionary and properties/assignment. The spec's saveEdit (like most callers) waits only for index-device-user, so the old rows can still be on screen when openRowActionMenu resolves the action cell. The rows are swapped once the other two responses arrive.

Fix (test code only)

openRowActionMenu now:

  • clicks the row's actionMenu trigger through a fresh locator. click re-resolves on detach and scrolls by itself, so the explicit scroll is gone.
  • reads the action-items-<i> index from the action cell whose trigger is aria-expanded="true". The index therefore belongs to the row that is actually rendered and owns the open menu. Before, it came from a row resolved before the click.

No assertions were weakened and nothing is retried. Every caller of the helper gets the fix without call-site changes: the PropertyWorkers, Properties and Chemicals page objects, plus the o/property-admin-actions, r/property-worker-one-minute-intervals, r/property-worker-set-password-gate and r/property-worker-delete-first-user-only specs.

Verification (CI container image from master, MariaDB 10.8, RabbitMQ, Playwright 1.59.1)

  • I couldn't reproduce the flake by plain repetition: the managing-tags spec passed 12/12 on the old helper and 12/12 on the new one.
  • Deterministic repro: a scratch copy of the spec (not committed) delayed properties/dictionary with page.route, which widens the window where the grid swaps rows.
    • Old helper: 5 of 10 delays between 40 and 250 ms failed with exactly the CI error.
    • New helper: passed all 23 runs at delays from 0 to 4000 ms.
  • Full shard r with the fix: 47 passed. The 1 failure is property-workers-app-versions, which needs docker exec and had no docker CLI in my local Playwright container. It is unrelated to this change.
  • o/property-admin-actions.spec.ts (properties rows): 3/3 passed.

🤖 Generated with Claude Code

…enders

openRowActionMenu called locator.scrollIntoViewIfNeeded on the row's action
cell. Unlike click, that call resolves the element once and throws "Element is
not attached to the DOM" when the row is replaced underneath it, and the
property-workers grid does replace its rows after a save: the list refresh is a
forkJoin of index-device-user, the properties dictionary and the assignments,
so a caller that waited for index-device-user can still see the old rows until
the other two land. That is how "a removed managing tag can be given back and
saved" (shard r) failed intermittently in CI.

The helper now clicks the trigger through a fresh locator (click re-resolves
on detach and scrolls by itself) and reads the menu index off the action cell
whose trigger is aria-expanded, so the index always belongs to the row that is
rendered and owns the open menu. Every caller of openRowActionMenu (property
workers, properties, chemicals and the specs that use it directly) gets this
without call-site changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 12:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused test-helper change preserves caller contracts and has no identified blocking issues.

No open issues 🎉

What changed in this PR

Stabilizes the shared Playwright row-action-menu helper when grids replace rows during refreshes, without changing callers.

Changes:

  • Replaces explicit scrolling with a locator click that handles detached elements.
  • Resolves the menu index from the rendered row’s expanded trigger, with a bounded assertion.
File Description
eform-client/​playwright/​e2e/​plugins/​backend-configuration-pn/​row-action-menu.ts Updates menu opening and index resolution to tolerate row replacement.

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 20, 2026 05:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused test-helper change preserves caller compatibility and explicit timeouts, with no blocking issues identified.

No open issues 🎉

Review effort: Balanced

…e typing

createTag filled #newTagName as soon as it was visible. The dialog's DOM
attaches a frame before its first change detection (the host coalesces
events), so ngModel's initial '' could overwrite the typed name and Save
stayed disabled until the 5-minute test timeout (shard r on #1456).
Wait for the translated dialog title, which renders in that first pass,
fill once, and assert Save is enabled before clicking. Same fix in the
task-list page object's createTag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 05:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused test-helper changes use bounded waits, match the inspected UI contracts, and have no identified blocking issues.

No open issues 🎉

Review effort: Balanced

…ag dialogs

Shard f on #1456 hit the same race in openBulkCreateTags: the textarea was
filled before the dialog's first change detection, ngModel reset it, and
Save stayed disabled. Wait for the translated title, fill once, and
assert the typed value stuck. renameTag gets the same wait (there ngModel
would write the old name back over the new one).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 06:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused test-only synchronization changes have no identified blocking issues.

No open issues 🎉

Review effort: Balanced

@renemadsen
renemadsen merged commit a13c02e into master Oct 20, 2026
64 of 66 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.

2 participants