Skip to content

Conversation

@lmuntaner
Copy link
Collaborator

@lmuntaner lmuntaner commented Sep 23, 2025

Motivation

Improve the reliability of the test coverage by adding Safari as a supported browser.

Changes

  • Updated the Playwright test matrix in .github/workflows/canister-tests.yml to run tests on both Chrome and Safari, for both desktop and mobile, and adjusted artifact naming to include the browser. Playwright browser installation is now conditional based on the matrix browser. Test execution now uses browser-specific project names.
  • Enhanced playwright.config.ts to define separate projects for chrome-desktop, chrome-mobile, safari-desktop, and safari-mobile, and enabled ignoreHTTPSErrors to support self-signed certificates during tests.
  • Added a custom Playwright fixture in fixtures.ts that implements host resolution routing for Safari, ensuring all non-localhost requests are redirected correctly during tests. This fixture is now used in all test files.
  • Refactored all E2E test files to import test and expect from the new fixtures.ts instead of @playwright/test, ensuring consistent use of the custom fixture across the suite.
  • Skipped migration tests on Safari (webkit) in both authorize/migration.spec.ts and dashboard/migration.spec.ts because virtual authenticators are not supported in that browser.

Tests

Only test changes.

@lmuntaner lmuntaner force-pushed the lm-add-safari-e2e-tests branch from a76fa23 to eedbe6c Compare October 3, 2025 07:28
@lmuntaner lmuntaner force-pushed the lm-add-safari-e2e-tests branch from eedbe6c to 0340c0f Compare October 6, 2025 09:05
@lmuntaner lmuntaner requested a review from Copilot October 6, 2025 12:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds Safari E2E test support to the CI pipeline by implementing a multi-browser test matrix and creating custom Playwright fixtures to handle Safari's limitations.

  • Added Safari (webkit) browser support alongside Chrome for both desktop and mobile configurations
  • Implemented custom Playwright fixtures with host routing to work around Safari's lack of support for --host-resolver-rules
  • Enhanced CSP configuration to support development environments with localhost subdomains

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.github/workflows/canister-tests.yml Updated CI matrix to include Safari browser testing and conditional browser installation
playwright.config.ts Added Safari desktop and mobile projects with browser-specific configurations
src/frontend/tests/e2e-playwright/fixtures.ts New custom fixture implementing host routing for Safari compatibility
src/frontend/tests/e2e-playwright/**/*.spec.ts Updated all test files to use custom fixtures instead of direct Playwright imports
eslint.config.js Added ESLint rule to enforce using custom fixtures in E2E tests
src/internet_identity/src/http.rs Enhanced CSP headers to support localhost subdomains in development
HACKING.md Added documentation for writing new E2E tests with custom fixtures

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -1,4 +1,5 @@
import { expect, Page, test } from "@playwright/test";
import { expect, test } from "../fixtures";
import { Page } from "@playwright/test";
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The Page type import from @playwright/test could be consolidated with the other imports or moved to a types-only import to improve organization.

Suggested change
import { Page } from "@playwright/test";
import type { Page } from "@playwright/test";

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant