Skip to content

6215: Add (real) E2E tests - #84

Open
gcgoncalves wants to merge 3 commits into
mainfrom
e2e-tests
Open

6215: Add (real) E2E tests#84
gcgoncalves wants to merge 3 commits into
mainfrom
e2e-tests

Conversation

@gcgoncalves

Copy link
Copy Markdown
Contributor

Closes IBM/mcp-context-forge#6215

Summary

Adds npm run e2e:docker: a self-contained real-backend e2e run. A new testing Compose profile brings up the UI + a real ContextForge gateway (ghcr.io/ibm/mcp-context-forge) + Redis, seeds a login user, runs the full suite against it, tears down after.

217/217 passing, mocked and real.

Changes

  • docker-compose.yml / Dockerfile: new gateway service under a testing profile; VITE_ENABLE_TOOL_PREVIEW build arg.
  • e2e/seed/seed.ts: clears the bootstrap admin's forced password-change so seeded creds work.
  • api-mock.ts: real login now runs for every spec's default mockSession(), not just auth/. The ~11 mocked-data suites (plugins, tools, resources, …) now run authenticated too.
  • Fixed along the way: mock-only CSRF token assertion, a session-establishment gap in password-change-required.spec.ts, a /dashboard/i heading assertion that only ever passed by matching an error string, worker-count contention against the sqlite-backed test gateway, and a stale-image bug in e2e:docker:up (missing --build).

Known gap

Freshly-seeded gateway has zero servers/tools, blocked on an upstream role-bootstrap bug in the pinned image, not fixed here. TODO left in seed.ts.

Testing

npm run e2e and npm run e2e:docker both 217/217. Unit tests unaffected.

Signed-off-by: Gabriel Costa <gabrielcg@proton.me>

@marekdano marekdano 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.

The changes look good, just few comments

Nice piece of infra work — real-backend e2e coverage via a testing compose profile, with mocks bypassed only on the success path, is a solid design and the fixture branching (E2E_REAL_API) is clean.

Functionally-impacting

  • redis-data volume gets wiped by e2e:docker:down. docker compose --profile testing down -v includes app and redis too (they carry no profile, so they're always in scope), and -v removes redis-data — the same volume a plain docker compose up dev stack uses. Anyone running npm run e2e:docker against a machine that also has the normal dev stack on this compose file will lose their local Redis data as a side effect. Consider a separate volume/project name for the testing profile, or call this out explicitly in the README as an accepted tradeoff.

Suggestions

  • seed.ts's clearForcedPasswordChange(): the "already past forced password change" branch (precondition.status === 200 on a login with BOOTSTRAP_PASSWORD) looks unreachable — once the password's been changed, logging in with the old bootstrap password should fail auth rather than succeed. Since down -v wipes the gateway container every run anyway, this reads as dead defensive code. Worth a comment explaining the scenario it guards, or dropping it.
  • The real-mode mockSession in api-mock.ts no-ops mockUnauthorized for all specs, not just the ones touched here — a repo-wide behavior change riding on a small fixture edit. You've verified 217/217 pass, so likely fine, just flagging the blast radius.

Minor

  • e2e:docker:seed uses --env-file-if-exists and --experimental-strip-types — fairly recent Node flags. Worth confirming CI's pinned Node version supports them if this is the first place they're used.
  • Hardcoded testing-only secrets in docker-compose.yml (JWT_SECRET_KEY, AUTH_ENCRYPTION_SECRET, bootstrap password) are clearly labeled not-for-prod and gated behind the testing profile — fine, just confirm your secret scanner (if any) won't flag them.

Everything else — CSRF token capture, workers=2 for real mode, the heading→Home-button assertion fix — looks solid and the reasoning is well-documented in comments.

Signed-off-by: Gabriel Costa <gabrielcg@proton.me>

@marekdano marekdano 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.

Worth addressing (non-blocking, but real)

  • e2e/seed/seed.ts:49 — the precondition check only accepts 200 or 403 from the login attempt. If a docker:e2e run gets interrupted after the password's already been changed but before teardown, the next seed attempt gets a 401 and throws instead of recognizing "already seeded." Widening the check to treat 401 as "already done" would make local re-runs more forgiving.
  • e2e/fixtures/auth.ts:24page and apiMock each call realLogin independently in real-API mode. Any future test that uses both fixtures together would log in twice, and apiMock's CSRF capture wouldn't get wired up. Worth having one fixture compose the other so there's a single login per test.

Minor, for whenever

  • e2e/fixtures/api-mock.ts:114 — the real-mode CSRF capture listener isn't awaited, so there's a small race where getRealCsrfToken() could still be unset when a test reads it.
  • e2e/fixtures/api-mock.ts:171mockUnauthorized() is a no-op under E2E_REAL_API=true; fine today since nothing uses it yet, just flagging so it doesn't quietly mask a future 401 test.
  • Dockerfile:37 and .env.example:70 still mention the "testing profile" that this PR replaced with docker-compose.e2e.yml — just a stale comment.

Signed-off-by: Gabriel Costa <gabrielcg@proton.me>
@gcgoncalves
gcgoncalves requested a review from marekdano August 26, 2026 15:50

@marekdano marekdano 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.

The PR looks good now! Thanks for addressing all issues.

LGTM 🚀

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.

[TESTING]: Add E2E flag for playwright tests

2 participants