test: add Playwright coverage and test ids for notifications - #824
Open
fateeand wants to merge 5 commits into
Open
test: add Playwright coverage and test ids for notifications#824fateeand wants to merge 5 commits into
fateeand wants to merge 5 commits into
Conversation
fateeand
requested review from
TerranceKhumalo-absa and
lukasmatta
as code owners
August 5, 2026 10:01
Contributor
Coverage report for library
Test suite run success2474 tests passing in 76 suites. Report generated by 🧪jest coverage report action from 7afbb16 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the notification/toast system’s testability and regression protection by adding stable E2E coverage (Playwright) and introducing data-testid hooks across notification UI, plus updating the composition demo page and fixtures to support the new tests.
Changes:
- Added Playwright E2E spec covering key notification behaviors (portal attachment, container reuse, maxAmount truncation, auto-dismiss, pause/resume, a11y announcements, clear/teardown).
- Added
data-testidattributes to toast and notification container templates to enable robust selectors in E2E tests. - Expanded the composition notification demo/examples (including duplicate suppression demo) and adjusted Playwright fixture setup.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| projects/cps-ui-kit/src/lib/services/cps-notification/internal/components/cps-toast/cps-toast.component.ts | Switches to inject(NgZone) and removes unused module import to align with updated component dependencies. |
| projects/cps-ui-kit/src/lib/services/cps-notification/internal/components/cps-toast/cps-toast.component.html | Adds data-testid hooks for toast elements used by Playwright selectors. |
| projects/cps-ui-kit/src/lib/services/cps-notification/internal/components/cps-notification-container/cps-notification-container.component.ts | Cleans up DI (drops unused deps, uses inject), updates doc comment, and maintains z-index logic. |
| projects/cps-ui-kit/src/lib/services/cps-notification/internal/components/cps-notification-container/cps-notification-container.component.html | Replaces ngClass/ngStyle with native bindings and adds data-testid hooks. |
| projects/composition/src/app/pages/notification-page/notification-page.examples.ts | Adds a “Duplicate notification” code example to the composition page. |
| projects/composition/src/app/pages/notification-page/notification-page.component.ts | Adds a handler method to trigger duplicate notifications for demo/test purposes. |
| projects/composition/src/app/pages/notification-page/notification-page.component.html | Adds data-testid hooks to demo triggers and wires in the duplicate notification example. |
| playwright/fixtures/composition-components.ts | Forces notification demo button clicks in the fixture setup to avoid interaction blockage during scans. |
| playwright/cps-ui-kit/components/cps-notification.spec.ts | Adds comprehensive Playwright E2E coverage for notification behaviors using the new test ids. |
Suppressed comments (1)
playwright/cps-ui-kit/components/cps-notification.spec.ts:143
- Same issue as the hover test:
toasts(page).first()will start pointing at the later toast once a second notification is triggered (new notifications are inserted first viaunshift()). That means the focus pause/resume assertions may be checking a different toast than the one whose close button was focused.
await trigger(page, 'timeout-warning-notification-trigger').click();
const focusedToast = toasts(page).first();
await expect(focusedToast).toBeVisible();
await closeButton(focusedToast).focus();
await expect(closeButton(focusedToast)).toBeFocused();
Contributor
Playwright test resultsDetails
|
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.
Summary
CpsNotificationService's toast/notification overlay system, covering browser behavior: realdocument.bodyportal attachment, container reuse/creation across positions, realmaxAmounttruncation of the live DOM, real un-faked auto-dismiss timing, a real persistent (timeout: 0) notification, real hover- and keyboard-focus-driven pause/resume of the dismiss timer, real close-button removal and container teardown, real CSS animations (includingprefers-reduced-motion), realaria-liveannouncement content/timing, real duplicate-notification suppression, and realclear()teardown across multiple containers.CpsToastComponentandCpsNotificationContainerComponentcomponents' templates.CpsToastComponent/CpsNotificationContainerComponentdependency injection dropping two unused constructor params,ngClass/ngStyle-> native bindings, removing unusedCommonModuleimports.TODO: Merge with
feat: add test ids to notificationsto generate a releaseRelease notes: