Skip to content

fix: Start the E2E host so its Playwright tests run instead of skipping - #247

Merged
mpaulosky merged 17 commits into
mainfrom
fix/243-e2e-fixture-timeout
Oct 5, 2026
Merged

mpaulosky merged 17 commits into
mainfrom
fix/243-e2e-fixture-timeout

Conversation

@mpaulosky

Copy link
Copy Markdown
Owner

Closes #243

Summary

The Playwright fixture never brought the host up: nothing supplied the issuemanagerdb connection string, so api-service failed to start, WebApp failed with it, and the fixture waited out its 3-minute timeout. Each of the 35 browser tests then skipped and the job passed.

 PlaywrightFixture.InitializeAsync
+  start MongoDB test container
-  CreateAsync<AppHost>()
+  CreateAsync<AppHost>(ConnectionStrings:issuemanagerdb = container, RandomizePorts = false)
   StartAsync
-  wait for WebApp Running (3 min), catch everything -> IsAvailable = false -> tests skip
+  wait for WebApp healthy (throws as soon as a resource fails to start)
+  no catch -> a host that can't start fails every test in the collection

Getting the host up surfaced bugs that the skips had hidden:

  • /admin declared twice (Pages/Admin.razor placeholder and Features/Admin/AdminPage.razor). The router threw on every interactive render and killed the circuit. The placeholder is deleted.
  • The Web called https+http://api, but the AppHost names the resource api-service, so every API call failed DNS. It now uses Constants.ApiService.
  • /auth/login was async void, so the response could end before the Auth0 challenge redirected. It now returns Task.
  • Random ports broke Auth0 login, because the callback URL wasn't on the allowed list. The fixture keeps the launchSettings ports (https://localhost:7176), and that callback is now allowed in Auth0.

Test credentials now come from configuration, Auth0:{Admin,Author,User}:{Username,Password}: user secrets locally, Auth0__{Role}__Username/Password in CI. The Api, Web and E2E projects share the 94491f6e-auth0-values-3ff40da38702 user-secrets store.

The 11 tests that now run and fail are skipped with Skip = "Fails: #246". #246 tracks their fixes.

Evidence

  • Before: AppHost.Tests.E2E: total 53, passed 18, skipped 35 (Fixture initialization failed: The operation has timed out.), 3m+
    After: total 53, passed 42, skipped 11 (each Fails: #246), failed 0, ~2m. The pre-push gate passed.
  • With the old missing-connection-string config, the fixture now fails in ~20s with Stopped waiting for resource 'WebApp' to become healthy because it failed to start rather than skipping.
  • Web.Tests.Bunit (220 passed, 6 skipped), Web.Tests.Unit (85) and Architecture.Tests (11) pass.

Merge Danger

Door: two-way

Revert restores the old fixture. Only the Auth0 callback URL lives outside the repo, and it's additive.

Blast Radius: E2E-and-Web

  • CI's E2E job needs TEST_ENV (ci: Standardize on the repo-ci-baseline Template #241) to carry Auth0__Domain, Auth0__ClientId, Auth0__ClientSecret, Auth0__Audience and the six Auth0__{Role}__Username/Password values. Without them, the host fails to start and the job now fails rather than skipping.
  • The E2E job needs Docker for the MongoDB container, which it already needs for Redis.
  • The Web changes (API base address, login endpoint, removed placeholder page) affect the running app. The API calls and login were broken before, so this should only fix them.

🤖 Generated with Claude Code

mpaulosky and others added 7 commits October 4, 2026 17:34
…t can't start

The Playwright fixture never brought the host up: the AppHost needs the
issuemanagerdb connection string (the Atlas URI), nothing supplied it, so
api-service failed to start, WebApp failed with it, and the fixture waited
out its 3-minute timeout. Each of the 35 tests then skipped, and the job passed.

The fixture now starts a MongoDB test container and hands its URI to the
AppHost, waits for WebApp's health check (which throws as soon as a resource
fails to start), and no longer catches its own failure, so a host that can't
start fails every test in the collection instead of skipping them.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he callback

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pages/Admin.razor and Features/Admin/AdminPage.razor both declared /admin, so
the router threw on every interactive render and the circuit died.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…enge

The API clients used https+http://api, but the AppHost names the resource
api-service, so every call failed DNS resolution. /auth/login was async void,
so the response ended before the Auth0 challenge could redirect.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The helper read E2E_TEST_{ROLE}_EMAIL/PASSWORD environment variables. It now
reads configuration: the test project's user secrets locally, and environment
variables (Auth0__{Role}__Username/Password) in CI.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test project already uses 94491f6e-auth0-values-3ff40da38702; the Api and
Web read their Auth0 settings from the same store when run locally.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
They never ran before #243 (the fixture timed out), and fail on stale selectors,
the login redirect path and the admin pages. Each skip names #246, which
tracks the fixes, so they show by name in every run rather than hiding.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 5, 2026 02:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

CI lacks the required Auth0 audience and test credentials, and the login endpoint accepts an unvalidated redirect target.

Review effort: Balanced
Findings: 2 High severity · 1 Medium severity

Open (3)
What changed in this PR

Starts the Aspire E2E host with MongoDB so Playwright failures no longer silently skip the suite.

Changes:

  • Adds MongoDB-backed E2E host startup and health checks.
  • Shares Auth0 configuration and fixes API discovery/login handling.
  • Removes the duplicate admin route and marks known failures skipped.

Validation: Static review only; no commands run.

File Description
tests/​AppHost.Tests.E2E/​Navigation/​UserNavigationTests.cs Removes fixture skips and marks known failures.
tests/​AppHost.Tests.E2E/​Navigation/​UnauthenticatedNavigationTests.cs Removes fixture skips and marks known failures.
tests/​AppHost.Tests.E2E/​Navigation/​LogoutTests.cs Updates credential configuration and skips.
tests/​AppHost.Tests.E2E/​Navigation/​AuthorNavigationTests.cs Updates credential configuration and skips.
tests/​AppHost.Tests.E2E/​Navigation/​AdminNavigationTests.cs Updates credential configuration.
tests/​AppHost.Tests.E2E/​Issues/​IssuesCrudFlowTests.cs Updates credentials and marks known failures.
tests/​AppHost.Tests.E2E/​Helpers/​Auth0LoginHelper.cs Reads credentials from configuration.
tests/​AppHost.Tests.E2E/​Fixtures/​PlaywrightFixture.cs Starts MongoDB and waits for a healthy host.
tests/​AppHost.Tests.E2E/​AppHost.Tests.E2E.csproj Adds user secrets and MongoDB Testcontainers.
src/​Web/​Web.csproj Switches to the shared secrets store.
src/​Web/​Program.cs Fixes API discovery and asynchronous login handling.
src/​Web/​Components/​Pages/​Admin.razor Removes the duplicate admin route.
src/​Api/​Api.csproj Switches to the shared secrets store.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Web/Program.cs Outdated
Comment thread tests/AppHost.Tests.E2E/Fixtures/PlaywrightFixture.cs
Comment thread tests/AppHost.Tests.E2E/Helpers/Auth0LoginHelper.cs
@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Test Results Summary

879 tests   862 ✅  1m 9s ⏱️
  7 suites   17 💤
  7 files      0 ❌

Results for commit 10fd68c.

♻️ This comment has been updated with latest results.

Copilot AI balanced review requested due to automatic review settings October 5, 2026 04:04
A crafted /auth/login?returnUrl= could send a user off-site after signing in.
Anything that isn't a local URL now falls back to /.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The login redirect remains exploitable, secret-store migration is undocumented, and container cleanup is not guaranteed.

Review effort: Balanced
Findings: 4 High severity · 2 Medium severity

Open (6)

Comment thread src/Api/Api.csproj
Comment thread src/Web/Web.csproj
Comment thread tests/AppHost.Tests.E2E/Fixtures/PlaywrightFixture.cs Outdated
Copilot AI balanced review requested due to automatic review settings October 5, 2026 04:09
scripts/gate.sh writes TRX files there; left untracked, the next pre-push gate
refuses to run because the working tree isn't clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread src/Web/Program.cs Outdated
Copilot AI balanced review requested due to automatic review settings October 5, 2026 04:14
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot AI balanced review requested due to automatic review settings October 5, 2026 04:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Secretless Dependabot runs cannot pass the required E2E gate, and MongoDB startup remains outside the timeout.

Review effort: Balanced
Findings: 3 High severity · 3 Medium severity

Open (6)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Bound container startup with the five-minute timeout

tests/​AppHost.Tests.E2E/​Fixtures/​PlaywrightFixture.cs:85

The five-minute startup timeout is created only after MongoDB has started, so a stalled image pull or container startup can still hang this fixture well beyond the intended bound. Pass a timeout token to StartAsync as well so every external startup step is bounded.

mpaulosky and others added 3 commits October 4, 2026 21:27
Moves the local-URL check into AuthExtensions.GetLocalReturnUrl and tests that
local paths pass through while absolute, protocol-relative, backslash, script
and empty values fall back to /.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… throws

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Documents the one store the Api, Web and E2E projects now share, how to move
values from the old per-project stores, the E2E test users and callback URL,
and the TEST_ENV lines CI needs. Drops a Client ID and secret that were
committed in plain text (the app no longer exists in the tenant).

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On the runner the dev certificate is untrusted, so the AppHost's HTTPS health
check on the web app failed every attempt and the fixture timed out after five
minutes. prepare.sh now trusts it for AppHost.Tests.E2E and points
SSL_CERT_DIR at the exported certificate.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 5, 2026 04:39
dotnet dev-certs https --trust exits 4 when OpenSSL trusts the certificate but
no browser store exists, which stopped prepare.sh under set -e. OpenSSL trust
is what the AppHost and web app need.

Refs #243

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Dependabot runs will fail without TEST_ENV, while incomplete role credentials can still silently skip tests.

Review effort: Balanced
Findings: 3 High severity · 2 Medium severity

Open (5)
Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity NuGet Dependabot tests fail without Auth0 configuration

tests/​AppHost.Tests.E2E/​Fixtures/​PlaywrightFixture.cs:95

The new startup path makes Auth0 mandatory, but NuGet Dependabot PRs still run this matrix without TEST_ENV (.github/workflows/ci.yml:366-375; only GitHub Actions bumps skip tests at lines 627-632). The API will therefore throw for missing Domain/Audience and every future NuGet update will fail here. Add a safe no-secret test-auth path for Dependabot, or explicitly omit this E2E project for those runs while retaining strict startup failure in normal CI.

Medium severity Missing E2E role credentials silently skip CI tests

tests/​AppHost.Tests.E2E/​Helpers/​Auth0LoginHelper.cs:33

These optional lookups still return null, and every role test converts that result into SkipException. An incomplete TEST_ENV (for example, a missing admin password) therefore leaves the host healthy and the affected scenarios silently green, contrary to the stated requirement that missing E2E configuration fail CI. Fail on missing role credentials in CI while preserving optional local skips.

Copilot AI balanced review requested due to automatic review settings October 5, 2026 04:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Secretless Dependabot and fork builds will now fail the mandatory E2E matrix during host startup.

Review effort: Balanced
Findings: 3 High severity · 2 Medium severity

Open (5)
Previously missed (2)

In code that hasn't changed since last review

Low severity Update setup instructions to use the actual HTTPS launch-profile port

docs/​build docs/​auth0-setup.md:111

These new instructions conflict with step 5 above, which still tells users to configure only https://localhost:7001, while the current Web launch profile and E2E host use port 7176. Consolidate the callback, logout, and origin entries in the primary setup step around the actual launch-profile URL so following the guide does not leave local login misconfigured.

Low severity Add required Arrange marker to theory test

tests/​Web.Tests.Unit/​Extensions/​AuthExtensionsTests.cs:70

This new test omits the repository-required // Arrange marker. Add the marker even though the theory input is supplied by InlineData, so every test retains the mandated Arrange/Act/Assert structure.

This issue also appears on line 87 of the same file.

Matches the repo-ci-baseline Template (dotfiles #47); the shared settings are
in aspire.config.json.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 5, 2026 04:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Secretless Dependabot and fork pull requests will fail the required E2E job because Auth0 configuration is unavailable.

Review effort: Balanced
Findings: 3 High severity · 2 Medium severity

Open (5)

@mpaulosky
mpaulosky merged commit 27ac85f into main Oct 5, 2026
50 of 52 checks passed
@mpaulosky
mpaulosky deleted the fix/243-e2e-fixture-timeout branch October 5, 2026 05:02
mpaulosky added a commit that referenced this pull request Oct 5, 2026
Automated release blog posts for #247, opened by the release workflow.
It holds every Release whose post isn't on main yet, rebuilt from main
on each run. The [skip-release] title marker keeps its merge from
starting another release.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

E2E: Playwright fixture times out, so 35 of 53 tests are skipped

2 participants