Skip to content

docs(demo): flag EPDS_CLIENT_THEME as required for client-branding e2e#145

Merged
aspiers merged 1 commit into
mainfrom
docs/demo-theme-required-for-e2e
May 1, 2026
Merged

docs(demo): flag EPDS_CLIENT_THEME as required for client-branding e2e#145
aspiers merged 1 commit into
mainfrom
docs/demo-theme-required-for-e2e

Conversation

@aspiers

@aspiers aspiers commented May 1, 2026

Copy link
Copy Markdown
Contributor

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-branding scenarios failed with mismatches like:

expect(html).toContain('body { background: #1a1208')

Root cause: packages/demo/.env.example already documented EPDS_CLIENT_THEME=amber but as a commented-out optional. Without it, the demo's client-metadata.json ships no branding.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=amber block, with the exact assertion-mismatch message a future contributor would see.

Test plan

  • Confirmed locally: with EPDS_CLIENT_THEME=amber set in packages/demo/.env, the failing scenarios pass.
  • No code changes; doc-only.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Updated environment setup documentation with instructions for enabling end-to-end tests for branding features.

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.
Copilot AI review requested due to automatic review settings May 1, 2026 17:58

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@changeset-bot

changeset-bot Bot commented May 1, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 45d7181

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel

vercel Bot commented May 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
epds-demo Ready Ready Preview, Comment May 1, 2026 5:58pm

Request Review

@coderabbitai

coderabbitai Bot commented May 1, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The .env.example file in the demo package is updated with documentation explaining how to enable the @client-branding e2e feature locally. It clarifies that the test suite requires branding CSS to be advertised via client-metadata.json, which depends on enabling the themed branding configuration.

Changes

Cohort / File(s) Summary
Environment Configuration Documentation
packages/demo/.env.example
Added 6 lines of instructional comments explaining the requirement to enable themed branding configuration for the @client-branding e2e feature tests to pass, noting that the demo's branding CSS must be advertised via client-metadata.json.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Poem

🐰 A note for the brave, a comment so clear,
"Enable the theme, let branding appear!"
The CSS whispers through metadata's door,
E2E tests dance like never before.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: documenting that EPDS_CLIENT_THEME is required for the client-branding e2e tests, matching the PR objectives and file modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/demo-theme-required-for-e2e

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@railway-app

railway-app Bot commented May 1, 2026

Copy link
Copy Markdown

🚅 Deployed to the ePDS-pr-145 environment in ePDS

Service Status Web Updated (UTC)
@certified-app/demo untrusted ✅ Success (View Logs) Web May 1, 2026 at 5:59 pm
@certified-app/demo ✅ Success (View Logs) Web May 1, 2026 at 5:59 pm
@certified-app/pds-core ✅ Success (View Logs) Web May 1, 2026 at 5:59 pm
@certified-app/auth-service ✅ Success (View Logs) Web May 1, 2026 at 5:59 pm

@railway-app railway-app Bot temporarily deployed to ePDS / ePDS-pr-145 May 1, 2026 17:58 Destroyed
@sonarqubecloud

sonarqubecloud Bot commented May 1, 2026

Copy link
Copy Markdown

@coveralls-official

Copy link
Copy Markdown

Coverage Report for CI Build 25226033252

Coverage remained the same at 53.156%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 2806
Covered Lines: 1489
Line Coverage: 53.06%
Relevant Branches: 1709
Covered Branches: 911
Branch Coverage: 53.31%
Branches in Coverage %: Yes
Coverage Strength: 5.44 hits per line

💛 - Coveralls

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 84f7a93 and 45d7181.

📒 Files selected for processing (1)
  • packages/demo/.env.example

Comment thread packages/demo/.env.example

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.example that EPDS_CLIENT_THEME must 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.

Comment thread packages/demo/.env.example
@aspiers aspiers merged commit 87b5d96 into main May 1, 2026
19 checks passed
@aspiers aspiers deleted the docs/demo-theme-required-for-e2e branch May 1, 2026 18:14
aspiers added a commit to aspiers/ePDS that referenced this pull request May 5, 2026
 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).
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.

2 participants