test(e2e): port student_learning/maze_signed_out to Playwright - #74533
Open
stephenliang wants to merge 2 commits into
Open
test(e2e): port student_learning/maze_signed_out to Playwright#74533stephenliang wants to merge 2 commits into
stephenliang wants to merge 2 commits into
Conversation
Port 4 scenarios from dashboard/test/ui/features/student_learning/maze_signed_out.feature. Green under the 5x/all-browser stress gate; original Cucumber feature tagged @playwright so the Cucumber suite skips it. Source: dashboard/test/ui/features/student_learning/maze_signed_out.feature
Review follow-up on the maze_signed_out port. Three shared abstractions were
in the wrong place; the port made two of them worse.
Progress bubbles: the same widget renders in the lesson header and in the unit
overview, but each page carried its own copy of the state predicates, so adding
'attempted' meant editing both. Extract ProgressBubble, which owns the DSCO
tokens and shows(state). Pages keep the addressing, which genuinely differs
(header = nth link in a strip; overview = nth bubble in the nth row), and hand
back a ProgressBubble. Six near-identical methods collapse to one.
Dialogs: closeDialog(page) was a transliteration of Cucumber's "I close the
dialog", which queries '#x-close, [role=dialog] button[aria-label=Close]' and
picks whichever is visible because a Cucumber step cannot know which dialog is
open. A page object does know. Both the feedback dialog and the video modal
render the same #x-close id, so the id identifies nothing and only the
container tells them apart. Add LegacyDialogComponent, constructed from its own
container and locating the close button by accessible name rather than by id;
FeedbackDialogComponent and VideoModalComponent extend it. The congrats message
moves onto the feedback dialog and is now scoped to it rather than page-wide.
blockChild takes {child, parent}: two adjacent string parameters that a caller
can transpose silently.
Verified on test-studio: the five affected specs (maze signed-out, maze
progress, artist, bee, multi3) at --repeat-each=3 across chromium, firefox and
webkit, 135/135. typecheck and lint clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
student_learning/maze_signed_out.featureis a Selenium/Cucumber UI test, so it only runs in the slow nightly-style suite and only against the browsers Cucumber is wired for. This ports all four scenarios to Playwright, where they run per-PR across chromium, firefox, and webkit, and tags the original feature@playwrightso the Cucumber suite skips it — no double coverage.What is covered
All four scenarios, one-to-one with the feature file, all for an anonymous session against the
ui-test-mazecourse. The@no_mobiletag carries over as a Playwright test tag.perfectin both the lesson header and the unit overview; closing the congrats dialog does not redirect to the next level; a second course's progress is unaffected; the saved block source survives a reload and is cleared by the unit's/resetendpoint.attemptedin the header and the unit overview.The Background's
/resetstep becomes abeforeEach, so each scenario starts from clearedclient_stateand session exactly as Cucumber did.Shared-library reuse
Most of the diff is additive to existing shared modules rather than new test-local code:
shared/progress.tsgains anattemptedbubble state. It pairsnot_tried's background token withperfect's border token — a third distinct colour combination (white fill, green top border), matchingprogress.rb's ownattemptedbranch.LessonLevelPageandUnitOverviewPageeach get the matchingisProgressBubbleAttemptedaccessor alongside the existingperfect/not_triedpair.shared/routes.tsgainsunitResetUrl()for the Background step.LegacyBlocklyLabgainsblockChild()(a block's SVG group scoped under a parent block's group) andreferenceAreaLastLink, plus a composedvideoModal.shared/dialog.tsis new:closeDialog()handles both legacy#x-closeand DSCO[role="dialog"] button[aria-label="Close"], scoped with:visiblebecause a dismissed legacy dialog stays mounted.components/video-modal.tsis new, composed onto lab page objects the wayAuthoredHintsComponentandCalloutsComponentalready are.activities/maze/blocks.tsgainsTWO_MOVE_FORWARD_BLOCKS, transliterated fromblockly_initialization_blocks.rb. The idstopBlock/startBlock/moveForwardare load-bearing and kept verbatim: the feature's parent-child assertion re-finds them by id, andstartBlockis amaze_moveForwardrather than thewhen_runblock its name suggests. Odd, but it is the original's naming, and changing it would change what the assertion means.CalloutsComponent.calloutWithText()looks a callout up by rendered text rather than by index. AgetByRole('alert', {name})lookup cannot work here for two compounding reasons: qTip2 putsrole="alert"on the.cdo-qtipscontainer itself rather than a descendant, andalerthas no name-from-content in the accname spec, so its computed accessible name is always empty. The role locator is filtered by text instead.Accessibility baselines
Each distinct new surface is axe-scanned against WCAG AA and asserted as an exact
{rule: count}map, so a new violation fails the test and a fixed one forces the baseline to shrink. The header and unit-overview progress-bubble surfaces are the same DOM this feature group'sprogress.spec.tsalready baselines, so they are not re-scanned. Inline feedback and the callout are clean; everything recorded predates this PR.aria-required-children: 1(the Blockly palette canvas is ag[role="listbox"]whose[role=none]/[role=figure]children that role does not allow) andcolor-contrast: 1(#runButton, white on#f46800= 3.07:1 against the 4.5:1 AA floor).image-alt: 1, the win/avatar illustration has no alt text.aria-prohibited-attr, anaria-labelon YouTube's unroled#movie_playerdiv, never lands there in time. That is the one stable, reproducible engine split, so it is a documented per-engine override rather than a loosened assertion.The video-modal scan needs the embed to settle first, and settling means more than the iframe's load event.
createVideo()inapps/src/code-studio/videos.jsbuilds<iframe id="video">with notitleattribute, so the only title it ever carries is the one YouTube'sYT.Playerwrites when it decorates the element. Measured on webkit, that attribute is still absent at the load event in 2 of 3 runs and lands 250–500ms later; a scan inside that window counts an extraframe-titleviolation.VideoModalComponent.waitForVideoLoaded()therefore waits for the title attribute as well as the load event, pinning the scan to settled DOM.That untitled iframe is a real accessibility gap in our own markup — we ship it without an accessible name and depend on a third party to supply one. Out of scope for a test port; worth its own ticket.
Links
Testing story
Automated — this PR is the test. Every number below was measured on the committed spec, independently of the porting tooling:
npx playwright test tests/activities/maze/signed-out.spec.ts --repeat-each=5 --workers=2→ 60/60 passed in 7.2m against test-studio, across chromium, firefox, and webkit.--repeat-each=10across all three engines → 30/30 passed in 2.8m.yarn typecheckclean;yarn lintclean for the changed files (one pre-existing warning intests/shared/axe.ts).The
frame-titlerace described above is not hypothetical: the first independent run of the stress gate came back 59/60, failing on webkit repeat 4. The cause was confirmed by direct measurement — a webkit probe sampling the iframe'stitleattribute every 250ms from the load event — rather than inferred from the failure. After the fix, the previously-failing scenario passed 30/30 and the full gate 60/60.The progress assertions poll for 30s, matching
progress.rb's own colour poll, so a slow milestone round-trip from the shared test-studio is absorbed rather than failed.One assertion is deliberately no stronger than the original. The video-modal scenario's "does not reappear" claim rests entirely on the reference-area link click succeeding, and neither the Cucumber nor this port asserts the modal's absence directly. Fidelity was kept over silently tightening it; happy to strengthen it if reviewers would rather.