fix(playwright): drop stale data-attr click after LemonRadio refactor#59121
Merged
Conversation
Cancellation events test was clicking [data-attr=survey-display-conditions-select] but that attribute now sits on a LemonRadio whose outer container does not forward data-attr to the DOM. Click the inner radio option directly, which is the actual interaction the test needs anyway.
Contributor
|
Reviews (1): Last reviewed commit: "fix(playwright): drop stale data-attr cl..." | Re-trigger Greptile |
Contributor
|
Size Change: -45 kB (-0.04%) Total Size: 118 MB 📦 View Changed
ℹ️ View Unchanged
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
E2E Playwright workflow has been failing on every non-cancelled master run since #58893 landed. Across the most recent 200 completed runs, 48/71 non-cancelled runs failed (67.6%), and 29/29 (100%) of the runs that started after the refactor merged failed.
The dominant pattern:
e2e/surveys/crud.spec.ts:184:9 > CRUD Survey > can set cancellation eventsfailed on 12 of the 13 most recent sampled failures (the lone non-survey failure was due to unrelated flake reaching the--max-failures=5cap after this one already failed). Across all 3 retries the test timed out on the same line:Root cause
#58893 (
refactor(surveys): tidy legacy Display conditions section) replacedLemonSelectwithLemonRadiofor the "Display conditions" choice.data-attr="survey-display-conditions-select"still appears on the React element inSurveyEdit.tsx, butLemonRadioonly forwardsdata-attrto the inner<input type=\"radio\">of each option — the outer container drops it. So the attribute simply no longer exists in the DOM, and the click waits forever.The inner option's
data-attr="survey-display-conditions-select-users"is still forwarded correctly, and clicking it is the actual interaction the test needs (it toggles the radio from "All users" to "Only users who match conditions").Changes
[data-attr=\"survey-display-conditions-select\"]click inplaywright/e2e/surveys/crud.spec.tsso the test interacts with the newLemonRadiodirectly.How did you test this code?
I'm an agent. I did not run the Playwright suite locally. Validation:
LemonRadio.tsxto confirmdata-attris forwarded only to inner<input>elements, not the outer<div>.gh run view --logacross 13 recent failed E2E runs — confirmed all hit the same locator on the same line.survey-display-conditions-select-usersstill exists in the DOM (it is inLemonRadioOption.optionPropswhich is spread onto the<input>).bin/hogli format:js, husky) ran cleanly against the change.Publish to changelog?
no
🤖 Agent context
gh run view --logfor each — every one of them namedcrud.spec.ts:184 > can set cancellation eventsas the only hard failure (other named failures were retried as "flaky" and ultimately passed; this one fails all 3 retries).test.fixmeand (b) restoring the outerdata-attron the React component, but the simplest and most correct change was to drop the redundant click — the radio interaction is what the test cares about.