Skip to content

Rewrite service and collection configuration test suites with React Testing Library#320

Merged
jonathangreen merged 3 commits into
mainfrom
rtl-services
Jul 16, 2026
Merged

Rewrite service and collection configuration test suites with React Testing Library#320
jonathangreen merged 3 commits into
mainfrom
rtl-services

Conversation

@jonathangreen

@jonathangreen jonathangreen commented Jul 13, 2026

Copy link
Copy Markdown
Member

Description

Rewrites the service and collection configuration test suites with React Testing Library.

Migrated suites (old enzyme test in src/components/__tests__/ removed, RTL test added/expanded in tests/jest/components/):

  • ServiceEditForm
  • EditableConfigList
  • SelfTests
  • ProtocolFormField
  • Collections

Migration notes (carried over from #316, for the suites in this PR):

  • ServiceEditForm was one of the largest suites in the migration (originally 1087 LOC).
  • Enzyme live-instance-method spies become observable behavior — e.g. asserting the submitted FormData (via the shared FormData shim) rather than that a save method was called.
  • Tiny test subclasses drive branches that the base EditableConfigList method short-circuits (Collections extends EditableConfigList).
  • Where a suite covers a connected panel, it renders the connected default export so mapStateToProps/mapDispatchToProps stay covered.

This is one of four PRs splitting the oversized #316 into independently reviewable pieces, grouped by feature area. Each split branches from main and touches a disjoint set of component test files, so the four can be reviewed and merged in any order.

Motivation and Context

#316 rewrote the HARD-tier and remaining merge test suites from enzyme to React Testing Library in a single ~11k-line change, which is impractical to review. Splitting it by feature area keeps each PR at a reviewable size while preserving the same net result. These were among the hardest suites in the mocha/chai/enzyme → jest/RTL migration — enzyme mounted them and poked class internals, so the rewrites re-express those assertions as DOM and behavior checks.

How Has This Been Tested?

npx jest --selectProjects unit over the migrated suites on this branch: 5 suites / 134 tests pass. No application source changes — test files only.

Checklist:

  • All new and existing tests passed.

@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

Claude finished @jonathangreen's task in 4m 29s —— View job


Code Review

  • Read the new/modified RTL test files
  • Read the source components under test
  • Compare against deleted enzyme suites for coverage gaps
  • Post review findings

Summary

This is a clean, high-quality enzyme→RTL migration. The rewrites consistently replace instance-method spying with observable behavior — the FormData shim to assert what save/registerLibrary actually receive, tiny test subclasses to drive the base EditableConfigList/ServiceEditForm override hooks (isLibraryRemovalPermitted, canCreate/canDelete/canEdit), and rendering the connected default exports so mapStateToProps/mapDispatchToProps stay covered. Date formatting in SelfTests is verified to mirror the component's own local-time formatDate, so those assertions are not timezone-fragile. No skipped/focused tests, and Codecov shows all modified lines covered. The coverage gaps raised earlier in review were addressed (the hidden-variant it.each, the hidden-select contract) or tracked separately with the author's acknowledgment (the hidden color-picker update path → #322). I found no correctness issues; one very minor nit below.

Details

Nit: tests/jest/components/EditableConfigList.test.tsx:972

The footer link is located with container.querySelector("p"), which grabs the first <p> in the tree. It works today because the only <p> in this render is the footer, but it's the same broad-selector smell that was narrowed for the adjacent info-alert assertion in this file. Targeting the footer by its distinctive text (e.g. within(footer) off screen.getByText(/Problems with your things\?/)) keeps the two sibling assertions consistent and robust.

const footer = withSelfTests.container.querySelector("p");

@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown

Greptile Summary

This PR rewrites service and collection configuration tests with React Testing Library.

  • Removed the legacy Enzyme suites for service and collection configuration components.
  • Added RTL coverage for ServiceEditForm, EditableConfigList, SelfTests, ProtocolFormField, and Collections.
  • Expanded behavior-focused tests for hidden protocol fields, list/menu forwarding, form submission, collection deletion, and self-test flows.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
tests/jest/components/ProtocolFormField.test.tsx Adds RTL tests for protocol field rendering, hidden values, list/menu behavior, and forwarded props.
tests/jest/components/Collections.test.tsx Adds RTL tests for collection lists, deletion states, library registration, and library removal confirmation.
tests/jest/components/EditableConfigList.test.tsx Adds RTL tests for list rendering, permissions, create/edit/delete flows, messages, and self-test links.
tests/jest/components/SelfTests.test.tsx Adds RTL tests for self-test rendering, sorting, refresh behavior, errors, and run-test actions.
tests/jest/components/ServiceEditForm.test.tsx Adds RTL tests for service form setup, protocol settings, library associations, saving, clearing, and errors.

Reviews (7): Last reviewed commit: "Remove unused container binding in Proto..." | Re-trigger Greptile

Comment thread tests/jest/components/ProtocolFormField.test.tsx
Comment thread tests/jest/components/ProtocolFormField.test.tsx
Comment thread tests/jest/components/ProtocolFormField.test.tsx Outdated
@codecov-commenter

codecov-commenter commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.02%. Comparing base (c88bac5) to head (1b2980d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #320      +/-   ##
==========================================
+ Coverage   90.72%   91.02%   +0.30%     
==========================================
  Files         205      205              
  Lines        7610     7610              
  Branches     1664     1664              
==========================================
+ Hits         6904     6927      +23     
+ Misses        402      387      -15     
+ Partials      304      296       -8     
Flag Coverage Δ
jest 91.02% <ø> (+0.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread tests/jest/components/ProtocolFormField.test.tsx
Comment thread tests/jest/components/ProtocolFormField.test.tsx
@jonathangreen
jonathangreen force-pushed the rtl-services branch 3 times, most recently from e59dbeb to 88ed727 Compare July 13, 2026 15:05
Comment thread tests/jest/components/ProtocolFormField.test.tsx

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

Looks good.

A few minor comments below for consideration.

Comment thread tests/jest/components/ProtocolFormField.test.tsx Outdated
Comment thread tests/jest/components/EditableConfigList.test.tsx Outdated
jonathangreen and others added 3 commits July 16, 2026 10:29
Co-authored-by: Tim DiLauro <tdilauro@users.noreply.github.com>
The hidden-setting test destructured container from render() but made all
of its assertions through screen, tripping no-unused-vars under
--max-warnings 0.
@jonathangreen
jonathangreen enabled auto-merge (squash) July 16, 2026 13:34
@jonathangreen
jonathangreen merged commit 6bc1185 into main Jul 16, 2026
4 checks passed
@jonathangreen
jonathangreen deleted the rtl-services branch July 16, 2026 13:38
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.

3 participants