test(e2e): capture sign-out screenshots#46
Conversation
Test & Lint Summary
|
Summary
|
noa-lucent
left a comment
There was a problem hiding this comment.
Clean, minimal change that matches existing Argos patterns across the repo. The screenshot names are descriptive and placement is correct (after session-clear verification).
One minor suggestion: replace waitForLoadState('networkidle') with an explicit element assertion on the post-sign-out landing page. This is more deterministic and adds a behavioral check the tests currently don't have. Not blocking — CI is green and the current approach works, but worth improving for long-term stability.
| return true; | ||
| }, { timeout: 20000 }); | ||
|
|
||
| await page.waitForLoadState('networkidle'); |
There was a problem hiding this comment.
[minor] waitForLoadState('networkidle') is discouraged by Playwright — it's inherently racy (can resolve too early with bursty traffic or hang with long-polling/websockets) and no other spec in this repo uses it.
Since the issue notes the page redirects to the login screen after sign-out, a more deterministic wait would be to assert on a visible element on that landing page:
await expect(page.getByRole('heading', { name: /Log in/i })).toBeVisible({ timeout: 20_000 });
await argosScreenshot(page, 'sign-out-user-menu');This also doubles as a behavioral assertion (user actually landed on the login page), which the tests currently lack.
Same applies to line 42.
Summary
Testing
Closes #45