fix: ci #1
fix: ci #1
Conversation
sirily11
force-pushed
the
ci
branch
2 times, most recently
from
9992d2e to
470f5e2
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes CI issues by addressing database HMR (Hot Module Replacement) problems, correcting Playwright test flakiness, and standardizing GitHub Actions workflow configuration.
Changes:
- Added HMR persistence for the database client to prevent connection resets during development
- Fixed WebAuthn virtual authenticator setup to use the correct page instance
- Added missing avatar button clicks before sign out actions in E2E tests
- Standardized GitHub Actions workflow formatting and reduced artifact retention
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/db/index.ts | Added HMR persistence logic to preserve database client and connection across hot reloads |
| e2e/auth/passkey.spec.ts | Fixed virtual authenticator setup to attach to the correct page and added missing avatar click |
| e2e/auth/register.spec.ts | Added missing avatar button click before logout, removed display name assertion |
| e2e/auth/login.spec.ts | Added missing avatar button click before logout, removed display name assertion, formatted long lines |
| .github/workflows/playwright.yml | Standardized YAML formatting, removed 'master' branch, reduced artifact retention to 1 day |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Should redirect to account page (email verification skipped in E2E) | ||
| await expect(page).toHaveURL("/account"); | ||
| await expect(page.getByText("Test User")).toBeVisible(); | ||
|
|
There was a problem hiding this comment.
Removed assertion that verified the user display name after registration. This removes test coverage for verifying that the registration was successful and the user data is correctly displayed on the account page.
|
|
||
| await expect(page).toHaveURL("/account"); | ||
| await expect(page.getByText(testUser.displayName)).toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
Removed assertion that verified the user display name is visible after successful login. This removes test coverage for confirming that the login was successful and the user's data is correctly displayed.
| with: | ||
| name: playwright-report | ||
| path: playwright-report/ | ||
| retention-days: 1 |
There was a problem hiding this comment.
Artifact retention reduced from 30 days to 1 day. This significantly limits the time available to review test reports and debug failures. Consider whether 1 day provides sufficient time for investigating test failures, especially over weekends or holidays.
| retention-days: 1 | |
| retention-days: 7 |
| const page = await context.newPage(); | ||
|
|
||
| // Set up virtual authenticator on the SAME page we'll use for testing | ||
| const { cdpSession, authenticatorId } = await setupWebAuthn(page); |
There was a problem hiding this comment.
Unused variable cdpSession.
| const { cdpSession, authenticatorId } = await setupWebAuthn(page); | |
| const { authenticatorId } = await setupWebAuthn(page); |
| const page = await context.newPage(); | ||
|
|
||
| // Set up virtual authenticator on the SAME page we'll use for testing | ||
| const { cdpSession, authenticatorId } = await setupWebAuthn(page); |
There was a problem hiding this comment.
Unused variable authenticatorId.
| const { cdpSession, authenticatorId } = await setupWebAuthn(page); | |
| await setupWebAuthn(page); |
sirily11
force-pushed
the
ci
branch
5 times, most recently
from
1d8d752 to
41df7a0
Compare
|
Found 3 test failures on Blacksmith runners: Failures
|
sirily11
force-pushed
the
ci
branch
3 times, most recently
from
b7c1c3a to
984da73
Compare
No description provided.