Skip to content

test(viewer): add dashboard and episode-viewer component tests#595

Open
WilliamBerryiii wants to merge 4 commits intomainfrom
feature/issue-218A-episode-viewer-tests
Open

test(viewer): add dashboard and episode-viewer component tests#595
WilliamBerryiii wants to merge 4 commits intomainfrom
feature/issue-218A-episode-viewer-tests

Conversation

@WilliamBerryiii
Copy link
Copy Markdown
Member

Description

Adds Vitest + React Testing Library coverage for the dataviewer frontend. Two stacked commits land together in this PR:

  1. 5ac1235 — vitest/happy-dom test infrastructure plus dashboard, app-shell, and viewer-display component tests (33 tests across 5 files, plus ViewerDisplayControls). Closes test(dataviewer): add tests for dashboard components #219.
  2. 1f6a15e — episode-viewer component tests (65 tests across 9 files), plus minor PlaybackControls aria-label additions to make controls testable. Closes #218A.

Total new coverage: 98 tests / 16 test files. Combined suite: 813/813 passing.

Closes #219
Closes #218A

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🏗️ Infrastructure change
  • ♻️ Refactoring (no functional changes)

Test-only addition; closest fit is Refactoring. The single source change is non-functional aria-label additions on PlaybackControls.tsx.

Component(s) Affected

  • infrastructure/ — Terraform, AKS, networking
  • workflows/ — GitHub Actions CI/CD
  • training/ — RL/IL training code
  • docs/ — Documentation
  • Other: data-management/viewer/frontend (test infrastructure + component tests)

Testing Performed

  • Manual testing completed
  • Unit tests added/updated
  • Integration tests added/updated
  • End-to-end tests verified

Test command:

cd data-management/viewer/frontend && npm run validate

Result: 81 test files / 813 tests passed in 28.64s · tsc clean · 0 lint errors / 39 pre-existing warnings.

Documentation Impact

  • No documentation changes needed
  • Documentation updated in this PR
  • Documentation will be updated in a follow-up PR

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas (n/a — tests are self-describing)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

WilliamBerryiii and others added 4 commits April 30, 2026 09:17
…codecov flags

- add Radix DOM shims (typed Observers) and test-utils QueryClient helpers
- split codecov vitest into vitest-{components,features,lib,state,app}
- multi-flag workflow upload; refresh copilot-instructions L448
- drop main.tsx from vitest-app; add StarRating/ProgressOverview proof tests

🧪 - Generated by Copilot
…ent tests

- add 5 dashboard component tests (ActivityFeed, AnnotatorLeaderboard, IssuesSummary, QualityDashboard, RatingDistribution)
- add DataviewerEpisodeViewer shell test and ViewerDisplayControls test
- disable vitest globals and wire jest-dom + cleanup in test/setup.ts
- closes #219

🧪 - Generated by Copilot
- add tests for CameraSelector, JointSelector, JointConfigDefaultsEditor
- add tests for PlaybackControls, Timeline, TrajectoryPlot, VideoPlayer
- add tests for joint-constants and trajectory-plot-utils helpers
- add aria-labels to PlaybackControls icon buttons for accessibility

closes #218

🧪 - Generated by Copilot
@WilliamBerryiii WilliamBerryiii requested a review from a team as a code owner May 1, 2026 17:31
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 1f6a15e.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.95%. Comparing base (c809d2f) to head (1f6a15e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #595      +/-   ##
==========================================
+ Coverage   65.16%   68.95%   +3.79%     
==========================================
  Files         251      263      +12     
  Lines       15597    16812    +1215     
  Branches     2193     2302     +109     
==========================================
+ Hits        10164    11593    +1429     
+ Misses       5142     4925     -217     
- Partials      291      294       +3     
Flag Coverage Δ *Carryforward flag
pester 83.13% <ø> (ø) Carriedforward from a7dd171
pytest-data-pipeline 100.00% <ø> (ø) Carriedforward from a7dd171
pytest-dataviewer 66.97% <ø> (+0.05%) ⬆️ Carriedforward from a7dd171
pytest-dm-tools 100.00% <ø> (ø) Carriedforward from a7dd171
pytest-evaluation 99.83% <ø> (?)
pytest-fuzz 4.91% <ø> (+0.01%) ⬆️ Carriedforward from a7dd171
pytest-inference 0.00% <ø> (ø) Carriedforward from a7dd171
pytest-training 82.14% <ø> (ø) Carriedforward from a7dd171
vitest 56.38% <100.00%> (+3.36%) ⬆️
vitest-app 56.38% <100.00%> (?)
vitest-components 56.38% <100.00%> (?)
vitest-features 56.38% <100.00%> (?)
vitest-lib 56.38% <100.00%> (?)
vitest-state 56.38% <100.00%> (?)

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...src/components/episode-viewer/PlaybackControls.tsx 100.00% <100.00%> (+42.85%) ⬆️

... and 38 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Collaborator

@katriendg katriendg left a comment

Choose a reason for hiding this comment

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

✅ Approved — Solid test coverage addition

98 new tests across 16 files with no regressions detected. The globals: false change is safe (all existing tests already import explicitly from vitest), aria-label additions improve both testability and accessibility, and the Response-based mock refactors are a strict improvement.


💡 Minor items — address at your discretion

These are all non-blocking style observations; feel free to ignore or address in follow-up:

1. Redundant cleanup() calls

setup.ts now registers a global afterEach(() => { cleanup() }), but 6 test files (StarRating, ActivityFeed, AnnotatorLeaderboard, IssuesSummary, ProgressOverview, QualityDashboard, RatingDistribution) still call cleanup() in their own afterEach. Harmless duplication — could remove the per-file ones for consistency.

2. Fragile CSS-class selectors

A few tests query by Tailwind utility classes (.animate-pulse, .font-medium, span.truncate.font-medium). These will break silently if class names change. Prefer role, aria-label, or data-testid where possible.

3. as never type casts on store mocks

Timeline.test.tsx, VideoPlayer.test.tsx, and TrajectoryPlot.test.tsx use as never to silence type mismatches on mock returns. A more precise cast (e.g., as ReturnType<typeof usePlaybackControls>) would preserve compile-time validation.

4. jsonResponse duplication

test-utils.tsx exports a nice jsonResponse helper, but api-client.test.ts and use-joint-config.test.ts maintain their own local versions. Could consolidate to one source.

5. Non-null assertion after expect guard (Timeline.test.tsx ~L183)

expect(playhead).not.toBeNull()
expect(playhead!.style.left).toBe('50%')

An if (playhead) narrowing or optional chaining would be more idiomatic TypeScript.


None of the above affect correctness or test reliability. Nice work on the coverage push! 🎉

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.

test(dataviewer): add tests for dashboard components

3 participants