Skip to content

Comments

feat: add e2e tests for insights charts#23777

Merged
eunjae-lee merged 6 commits intomainfrom
devin/1757611521-insights-chartcard-e2e-tests
Sep 15, 2025
Merged

feat: add e2e tests for insights charts#23777
eunjae-lee merged 6 commits intomainfrom
devin/1757611521-insights-chartcard-e2e-tests

Conversation

@eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Sep 11, 2025

What does this PR do?

Adds comprehensive e2e tests for ChartCard components on the insights page to verify that charts render correctly with their expected titles. This is a bare minimum test to see if tRPC calls for the charts fail or not.

Requested by: @eunjae-lee
Link to Devin run: https://app.devin.ai/sessions/88f67fafda5344e19b0ae5b4053bc593

Test Coverage

This PR adds three test scenarios:

Full chart rendering test - Verifies all 16 ChartCard components render with correct titles

How should this be tested?

Testing steps:

  1. Run the new tests: PLAYWRIGHT_HEADLESS=1 yarn e2e insights-chartcard.e2e.ts
  2. Verify all tests pass consistently across multiple runs
  3. Manually check that the chart titles in the test match actual rendered text on /insights
  4. Verify h2.text-emphasis selectors work correctly in the browser

Expected behavior:

  • All ChartCard components should be visible with correct titles

Mandatory Tasks

  • I have self-reviewed the code
  • N/A - No documentation changes required
  • I confirm automated tests are in place that prove my fix is effective or that my feature works ⚠️ Tests need verification due to local environment issues

- Test that all ChartCard components render with expected titles
- Test graceful handling of TRPC call failures
- Test ChartCard rendering for organization users
- Wait for page load and TRPC calls before checking chart presence
- Use text-based selectors to find ChartCard titles in h2 elements
- Remove page.waitForTimeout() calls to comply with ESLint rules

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 11, 2025

Walkthrough

  • Adds a Playwright E2E test verifying the /insights page renders 16 ChartCard panels by asserting expected h2 titles within panel-card elements.
  • Updates PanelCard to include data-testid="panel-card" on its root div.
  • Modifies HighestRatedMembersTable and LowestRatedMembersTable to always render ChartCard when isSuccess and data are present, removing the data.length > 0 guard (zero-length arrays now render an empty table within the card).
  • No changes to exported/public APIs.

Possibly related PRs

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "feat: add e2e tests for insights charts" is concise and directly summarizes the primary change—adding end-to-end tests for the insights ChartCard components—so it communicates the main intent clearly to reviewers.
Description Check ✅ Passed The PR description is directly related to the changeset and documents the purpose, test scenarios, how to run the tests, a Devin run link, and the note that tests need verification, so it meets the lenient requirement for this check.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 94a5008 and fb270b7.

📒 Files selected for processing (4)
  • apps/web/playwright/insights.e2e.ts (1 hunks)
  • packages/features/insights/components/booking/HighestRatedMembersTable.tsx (1 hunks)
  • packages/features/insights/components/booking/LowestRatedMembersTable.tsx (1 hunks)
  • packages/ui/components/card/PanelCard.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/web/playwright/insights.e2e.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/web/playwright/insights.e2e.ts
  • packages/ui/components/card/PanelCard.tsx
  • packages/features/insights/components/booking/HighestRatedMembersTable.tsx
  • packages/features/insights/components/booking/LowestRatedMembersTable.tsx
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/web/playwright/insights.e2e.ts
  • packages/ui/components/card/PanelCard.tsx
  • packages/features/insights/components/booking/HighestRatedMembersTable.tsx
  • packages/features/insights/components/booking/LowestRatedMembersTable.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • packages/ui/components/card/PanelCard.tsx
  • packages/features/insights/components/booking/HighestRatedMembersTable.tsx
  • packages/features/insights/components/booking/LowestRatedMembersTable.tsx
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: Detect changes
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Detect changes
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Detect changes
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Check for E2E label
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Check for E2E label
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Check for E2E label
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Check for E2E label
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
packages/ui/components/card/PanelCard.tsx (1)

48-48: LGTM! Good addition for test support.

The data-testid="panel-card" attribute enables proper E2E testing of panel cards without affecting the component's functionality or visual appearance.

apps/web/playwright/insights.e2e.ts (1)

241-274: LGTM! Comprehensive E2E test for ChartCard rendering.

The test properly sets up the required data (users, teams, memberships) and verifies all 16 expected ChartCard components are rendered with their correct titles. The test leverages the new data-testid="panel-card" attribute and h2 selectors to accurately locate and validate each chart component.

packages/features/insights/components/booking/LowestRatedMembersTable.tsx (1)

30-34: LGTM! Consistent rendering behavior for empty data.

Removing the data.length > 0 check ensures the ChartCard always renders when data is available, providing a consistent UI even with empty results. This aligns with the E2E test expectation of always having 16 ChartCard components visible.

packages/features/insights/components/booking/HighestRatedMembersTable.tsx (1)

30-34: LGTM! Consistent rendering behavior for empty data.

Similar to LowestRatedMembersTable, this change ensures the ChartCard renders unconditionally when data exists, maintaining consistent UI behavior and supporting the E2E test requirements.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/1757611521-insights-chartcard-e2e-tests

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

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

@keithwillcode keithwillcode added consumer core area: core, team members only labels Sep 11, 2025
@vercel
Copy link

vercel bot commented Sep 11, 2025

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

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 15, 2025 7:59am
cal-eu Ignored Ignored Sep 15, 2025 7:59am

@eunjae-lee eunjae-lee changed the title feat: add e2e tests for insights ChartCard components feat: add e2e tests for insights charts Sep 12, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2025

E2E results are ready!

@eunjae-lee eunjae-lee marked this pull request as ready for review September 12, 2025 13:16
@eunjae-lee eunjae-lee requested review from a team as code owners September 12, 2025 13:16
@dosubot dosubot bot added automated-tests area: unit tests, e2e tests, playwright insights area: insights, analytics labels Sep 12, 2025
@eunjae-lee eunjae-lee merged commit 0332118 into main Sep 15, 2025
41 checks passed
@eunjae-lee eunjae-lee deleted the devin/1757611521-insights-chartcard-e2e-tests branch September 15, 2025 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated-tests area: unit tests, e2e tests, playwright consumer core area: core, team members only insights area: insights, analytics ready-for-e2e size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants