Skip to content

Revert "feat(getByRole): add support for aria-busy (#41269)"#41795

Merged
dgozman merged 1 commit into
microsoft:mainfrom
dgozman:revert-f215af1
Jul 15, 2026
Merged

Revert "feat(getByRole): add support for aria-busy (#41269)"#41795
dgozman merged 1 commit into
microsoft:mainfrom
dgozman:revert-f215af1

Conversation

@dgozman

@dgozman dgozman commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

This reverts commit f215af1.
Reopens #36233.

@dgozman dgozman requested a review from Skn0tt July 15, 2026 10:39
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🟢 CI is clear — the one failure is a pre-existing flake

Hi, I'm the Playwright bot and I took a look at the failing CI. This PR reverts the aria-busy getByRole support (roleSelectorEngine.ts, roleUtils.ts, locatorUtils.ts, docs, types). The single failure is an MCP screencast test that never touches that code path — and it flakes on plenty of SHAs the PR can't be responsible for. Nothing here looks caused by this PR.

Details

Overall: 1 failure across the merged reports, and it's a known flake. The diff is confined to the role selector engine and getByRole types/docs; the failing test exercises the MCP show --annotate screencast flow, which has no connection to aria-busy or role matching.

Pre-existing flake / infra

Triaged by the Playwright bot - agent run

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🟡 One failure I can't clear — likely a race, but I couldn't prove it

Hi, I'm the Playwright bot and I took a look at the failing CI. This PR reverts the aria-busy getByRole support (role selector engine, roleUtils.ts, locatorUtils.ts, docs, types). Two tests failed across the merged reports. The MCP one is a known flake. The trace one is an action-ordering race that the revert can't plausibly reach — but it's the only time that test has ever failed, so I can't call it a proven flake.

Details

Overall: 2 failures. One is a well-established flake on unrelated SHAs; the other is a single ordering race with no cross-run history, so it stays uncertain. The diff is confined to role matching (getByRole/aria-busy) and its types/docs — neither failing test exercises that path.

Uncertain

  • [playwright-test] › playwright.trace.spec.ts:1267 › should not nest top level expect into unfinished api calls (@ubuntu-latest-node20) — the action tree came back with Expect "toBeVisible" and GET "/hang" swapped:
    -   "GET \"/hang\"",
        "Expect \"toBeVisible\"",
    +   "GET \"/hang\"",
    
    That's a timing race in which async action lands in the trace first, against a /hang route that never responds. It failed only on ubuntu-latest-node20 and passed on macos, windows, and node22/24/26 in the same run. In the results DB this test is 266/266 green except this one run — and that run is this PR, so I have no independent SHA where the same test flips. By our flake rule that's not enough to call it a flake. That said, the revert only touches getByRole/aria-busy, and this test uses getByText + route.fetch with no role matching, so there's no mechanism I can see for the diff to reorder trace actions. My read: most likely a pre-existing race, but I can't prove it. A rerun of tests 1 would settle it.

Pre-existing flake / infra

Triaged by the Playwright bot

Triaged by the Playwright bot - agent run

@dgozman dgozman closed this Jul 15, 2026
@dgozman dgozman reopened this Jul 15, 2026

@Skn0tt Skn0tt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto

@dgozman dgozman merged commit 3c71159 into microsoft:main Jul 15, 2026
91 of 93 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

3 flaky ⚠️ [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:356 › screencast › should work for popups `@chromium-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/heap.spec.ts:223 › should not leak workers `@firefox-ubuntu-22.04-node20`

49682 passed, 1156 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

7760 passed, 1249 skipped


Merge workflow run.

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