docs(demo): flag EPDS_CLIENT_THEME as required for client-branding e2e#145
Conversation
Re-running the e2e suite locally on a fresh worktree, all 8
@client-branding scenarios failed because packages/demo/.env didn't
set EPDS_CLIENT_THEME. The demo's client-metadata.json then ships no
branding.css, so auth-service inlines no theme CSS, so the suite's
`expect(html).toContain('body { background: #1a1208')` mismatches.
Update the .env.example comment to explicitly call out that running
the @client-branding feature requires this var to be set. Doesn't
change defaults — production deployments still pick the unthemed path.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
🚅 Deployed to the ePDS-pr-145 environment in ePDS
|
|
Coverage Report for CI Build 25226033252Coverage remained the same at 53.156%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/demo/.env.example`:
- Around line 52-56: The example failure string in the .env.example has a
quote/paren mismatch; update the quoted assertion example to match the real test
output by fixing the expect(html).toContain(...) snippet — for example replace
the broken "expect(html).toContain('body { background: `#1a1208`')" with a
well-formed assertion such as expect(html).toContain('body { background: `#1a1208`
}') (or use double quotes expect(html).toContain("body { background: `#1a1208` }")
or loosen to expect(html).toContain('body { background:')) so the example
exactly matches or reliably matches the actual failure from
features/client-branding.feature.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11e109e3-a7c0-44bd-9257-10bc95a18219
📒 Files selected for processing (1)
packages/demo/.env.example
There was a problem hiding this comment.
Pull request overview
Updates the demo app’s example environment file to clarify that EPDS_CLIENT_THEME must be set when running the client-branding E2E scenarios locally, preventing confusing CSS-injection assertion failures.
Changes:
- Adds an explicit note in
packages/demo/.env.examplethatEPDS_CLIENT_THEMEmust be uncommented for local client-branding E2E runs. - Documents the typical assertion mismatch contributors will see when the theme is missing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
review - Drop the misleading "@client-branding" tag reference (no such tag exists; copilot caught this) and reference the file path instead. - Replace the malformed "expect(html).toContain('...')" snippet with the actual playwright error substring a contributor would see in their failure output (coderabbit flagged the quote/paren mismatch).



Summary
Small doc-only follow-up to PR #141. While running the full e2e suite locally on a fresh worktree at the tail of that PR, all 8
@client-brandingscenarios failed with mismatches like:Root cause:
packages/demo/.env.examplealready documentedEPDS_CLIENT_THEME=amberbut as a commented-out optional. Without it, the demo'sclient-metadata.jsonships nobranding.css, so auth-service has no theme CSS to inline, so the suite's CSS-injection assertions fail. CI's Railway preview env presumably sets it via Railway service config; locally there's nothing to suggest you need to.Change
Add a "Required (uncomment) to run the @client-branding e2e feature locally" note next to the existing
EPDS_CLIENT_THEME=amberblock, with the exact assertion-mismatch message a future contributor would see.Test plan
EPDS_CLIENT_THEME=amberset inpackages/demo/.env, the failing scenarios pass.🤖 Generated with Claude Code
Summary by CodeRabbit