Skip to content

Rewrite custom-list test suites with React Testing Library#318

Open
jonathangreen wants to merge 1 commit into
mainfrom
rtl-custom-lists
Open

Rewrite custom-list test suites with React Testing Library#318
jonathangreen wants to merge 1 commit into
mainfrom
rtl-custom-lists

Conversation

@jonathangreen

@jonathangreen jonathangreen commented Jul 13, 2026

Copy link
Copy Markdown
Member

Description

Rewrites the custom-list and list-entry editing test suites with React Testing Library.

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

  • CustomListEntriesEditor
  • CustomLists
  • LaneCustomListsEditor
  • PairedMenus
  • InputList

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

  • CustomListEntriesEditor was the largest suite in the whole migration (originally 1461 LOC).
  • CustomListEntriesEditor and LaneCustomListsEditor use react-beautiful-dnd, which can't drag in jsdom. DragDropContext is mocked to capture onDragStart/onDragEnd, which the tests then invoke with synthetic drag results — both suites reach 100% coverage this way.
  • Enzyme live-instance-method spies become observable behavior — asserting which entries/rows are removed vs. retained — instead of assertions on class internals.
  • 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 / 109 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 18s —— View job


Code Review — PR #318: Rewrite custom-list test suites with React Testing Library

  • Read the new RTL test files
  • Read the source components under test
  • Analyze test correctness & coverage fidelity vs. the removed enzyme suites
  • Post review

Summary

This is a clean, faithful test-only migration. I reviewed all five migrated suites against their source components (CustomListEntriesEditor, LaneCustomListsEditor, PairedMenus, InputList/ProtocolFormField, CustomLists) and against the enzyme suites they replace. The assertions re-express the old live-instance-method checks as observable DOM/behavior, the react-beautiful-dnd mock captures onDragStart/onDragEnd cleanly, and connected default exports are rendered where mapStateToProps/mapDispatchToProps need to stay covered. Codecov confirms the net effect (misses −6, coverage +0.06%), so fidelity is preserved. I found no actionable bugs — nothing to block merge.

A few things I specifically checked that are correct as written (so no change needed):

  • The greptile note about getByRole("button", { name: "Delete" }) at CustomListEntriesEditor.test.tsx:1039 is a genuine false alarm — the exact name "Delete" doesn't collide with the "Remove from list" entry buttons, and the TrashIcon SVG contributes no accessible text. The author already resolved it.
  • The numeric draggableId: 2 in LaneCustomListsEditor.test.tsx's remove-drag test (vs. the string "1" in the add-drag test) initially looked asymmetric, but it matches the component's two code paths (add() does parseInt, remove() filters numerically) and preserves the prior enzyme suite's exact convention — not a regression.
  • jest.mock("react-router")'s global Link stub in CustomLists.test.tsx doesn't disturb the pre-existing connected create-mode tests, since CustomListEditor doesn't use Link.

Note: I was unable to execute the suites myself (dependency install requires an approval I don't have in this run), so this is a static review; the author's reported 5 suites / 109 tests passing and the Codecov result are consistent with what I read.
· rtl-custom-lists

@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown

Greptile Summary

This PR rewrites the custom-list test suites with React Testing Library.

  • Removes the legacy Enzyme suites from src/components/__tests__/.
  • Adds RTL coverage under tests/jest/components/ for custom lists and list entry editing.
  • Covers drag-and-drop flows with mocked react-beautiful-dnd callbacks.

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/CustomListEntriesEditor.test.tsx Rewrites the large CustomListEntriesEditor suite with RTL coverage for rendering, controls, pagination, and drag-and-drop callbacks.
tests/jest/components/CustomLists.test.tsx Migrates CustomLists behavior coverage to RTL.
tests/jest/components/InputList.test.tsx Migrates InputList interaction coverage to RTL.
tests/jest/components/LaneCustomListsEditor.test.tsx Migrates LaneCustomListsEditor tests with mocked drag-and-drop behavior.
tests/jest/components/PairedMenus.test.tsx Migrates PairedMenus coverage to RTL.

Reviews (2): Last reviewed commit: "Rewrite custom-list test suites with Rea..." | Re-trigger Greptile

Comment thread tests/jest/components/CustomListEntriesEditor.test.tsx
@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 90.22%. Comparing base (9fdb519) to head (2313de7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #318      +/-   ##
==========================================
+ Coverage   90.14%   90.22%   +0.07%     
==========================================
  Files         205      205              
  Lines        7610     7610              
  Branches     1664     1664              
==========================================
+ Hits         6860     6866       +6     
+ Misses        437      431       -6     
  Partials      313      313              
Flag Coverage Δ
jest 90.22% <ø> (+0.07%) ⬆️

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.

@jonathangreen jonathangreen requested a review from a team July 13, 2026 13:50
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