fix(playwright): register payroll-settings listener before the settings click - #1691
Merged
Merged
Conversation
…gs click The j/ and j1m/ dashboard-edit-a beforeEach hooks registered the page.waitForResponse() listener for the payroll-settings GET only after already awaiting the main settings GET. Since the settings component's ngOnInit fires both requests synchronously back-to-back, the payroll GET usually already completed by the time the listener was armed, leaving it unresolved for the full 120s test timeout (no per-action timeout is configured) and killing the browser mid-hook. Register both listeners before the click so neither response can complete unobserved. Fixes the consistent job/j and pn-playwright-test (j1m) failures in https://github.com/microting/eform-angular-timeplanning-plugin/actions/runs/33082638121 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WYHEYf7V1oJyPrLRLAWg6e
There was a problem hiding this comment.
Pull request overview
This PR fixes a Playwright test flake in the time-planning plugin E2E suite by ensuring network response listeners are registered before the UI click that triggers the relevant Angular component initialization, preventing missed responses and 120s beforeEach timeouts.
Changes:
- Register the payroll settings
page.waitForResponse()listener before clicking into settings, alongside the existing settings listener. - Replace the post-settings
waitForResponse(...).catch(() => {})with an awaited pre-registered promise in bothjandj1mvariants. - Add an in-test comment documenting the race condition and why listener registration order matters here.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| eform-client/playwright/e2e/plugins/time-planning-pn/j/dashboard-edit-a.spec.ts | Arms payroll settings response listener before the settings click to avoid missed responses in beforeEach. |
| eform-client/playwright/e2e/plugins/time-planning-pn/j1m/dashboard-edit-a.spec.ts | Applies the same listener-order fix to the j1m shard clone to prevent identical timeouts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
pn-playwright-test (j)andpn-playwright-test (j1m)jobs in run 33082638121 —Test timeout of 120000ms exceeded while running "beforeEach" hook.beforeEachhook inj/dashboard-edit-a.spec.ts(and thej1mclone) registeredpage.waitForResponse()for the payroll-settings GET only after already awaiting the main settings GET. The settings component'sngOnInitfiresgetSettings()andgetPayrollSettings()synchronously back-to-back, so the payroll GET had usually already completed by the time the listener was armed — leaving it unresolved for the full 120s test timeout (playwright.config.tssets no per-action timeout) and killing the browser mid-hook.waitForResponselisteners before the#plugin-settings-link0click, so neither response can complete unobserved. Dropped the.catch(() => {})on the payroll listener since the call is unconditional and now guaranteed to be caught.stable.Test plan
time-planning-settings.component.ts'sngOnInit(confirms both GETs are unconditional and fire only after the settings-link click mounts the component) — no critical/important issues found.pn-playwright-test (j)and(j1m)) passes.🤖 Generated with Claude Code
https://claude.ai/code/session_01WYHEYf7V1oJyPrLRLAWg6e