Skip to content

Rewrite lane and book-detail test suites with React Testing Library#319

Open
jonathangreen wants to merge 1 commit into
mainfrom
rtl-lanes-books
Open

Rewrite lane and book-detail test suites with React Testing Library#319
jonathangreen wants to merge 1 commit into
mainfrom
rtl-lanes-books

Conversation

@jonathangreen

@jonathangreen jonathangreen commented Jul 13, 2026

Copy link
Copy Markdown
Member

Description

Rewrites the lane-editing and book-detail 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/):

  • Lanes
  • LanesSidebar
  • LaneEditor
  • Lane
  • ClassificationsForm
  • BookCoverEditor
  • GenreForm
  • Contributors
  • EditorField
  • CatalogPage

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

  • The lane suites (Lanes, LanesSidebar, LaneEditor, Lane) and the book-detail forms previously mounted under enzyme; state()/instance() reads become DOM assertions and userEvent interactions.
  • Lane merges into an existing jest suite rather than adding a new file.
  • Enzyme live-instance-method spies become observable behavior instead of assertions on class internals, with tiny test subclasses where needed to drive branches the base method short-circuits.
  • 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: 10 suites / 111 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 22s —— View job


Code Review

  • Read the PR diff and changed test files
  • Compare new RTL suites against the deleted enzyme suites for coverage gaps
  • Assess correctness of test logic (assertions, mocks, drag boundaries)
  • Post review

Summary

This is a clean, well-executed enzyme → RTL migration. I traced each new suite against the component it exercises (LanesSidebar, Lanes, Lane, LaneEditor, BookCoverEditor, ClassificationsForm, GenreForm, Contributors, EditorField, CatalogPage) and the assertions line up with real behavior — the drag-boundary isDropDisabled matrix, the reorder payload handed to changeLaneOrder, the exact changeCoverLink.href on save, and the FormData contents all check out. The three earlier greptile P1 concerns (loose lane-order payload, loose cover-save URL, missing top-level→sublane drag boundary) are all resolved in the current revision — the new "keeps a dragged top-level lane in the top-level list" test is the mirrored boundary case that was requested, and it correctly asserts top stays enabled while both parent droppables are disabled. The mock strategy is well-documented (marker Link, hand-driven react-beautiful-dnd handlers, MockLaneCustomListsEditor as a class so the imperative ref API resolves), and the reorder flow reaching into ref.current for drag/state is a justified escape hatch since jsdom can't perform a real drag. I found no blocking issues. On the shared genreData fixture imported from src/components/__tests__/ by ClassificationsForm/GenreForm — agreed this is fine to defer; just make sure that fixture moves alongside these imports when the follow-up deletes the legacy __tests__ dir, otherwise these two suites break.

Nice work — no changes requested.
· rtl-lanes-books

@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown

Greptile Summary

This PR rewrites lane and book-detail component tests with React Testing Library. The main changes are:

  • Removed legacy Enzyme suites from src/components/__tests__/.
  • Added RTL coverage under tests/jest/components/.
  • Migrated lane editing, drag/drop, catalog, cover, contributor, genre, and classification form tests.

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/Lanes.test.tsx Migrates lane tests to RTL and checks the reordered lane payload on save.
tests/jest/components/BookCoverEditor.test.tsx Migrates cover editor tests to RTL and checks the exact cover save endpoint.
tests/jest/components/LanesSidebar.test.tsx Migrates sidebar tests to RTL and covers drag-start drop-boundary behavior.

Reviews (5): Last reviewed commit: "Rewrite lane and book-detail test suites..." | Re-trigger Greptile

Comment thread tests/jest/components/Lanes.test.tsx
Comment thread tests/jest/components/BookCoverEditor.test.tsx
Comment thread tests/jest/components/LanesSidebar.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 90.36%. Comparing base (9fdb519) to head (fdb4fce).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #319      +/-   ##
==========================================
+ Coverage   90.14%   90.36%   +0.22%     
==========================================
  Files         205      205              
  Lines        7610     7610              
  Branches     1664     1664              
==========================================
+ Hits         6860     6877      +17     
+ Misses        437      422      -15     
+ Partials      313      311       -2     
Flag Coverage Δ
jest 90.36% <ø> (+0.22%) ⬆️

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

Copy link
Copy Markdown
Member Author

The Claude code review bots minor comment:

These two new unit suites import the shared genreData fixture from src/components/__tests__/

Is valid, but I'm going to handle cleaning this up in a follow up PR after these get merged, when I delete the old __tests__ dir.

@jonathangreen jonathangreen force-pushed the rtl-lanes-books branch 3 times, most recently from 04b38e1 to d1cfd54 Compare July 13, 2026 14:36
Comment thread tests/jest/components/LanesSidebar.test.tsx Outdated
@jonathangreen jonathangreen requested a review from a team July 13, 2026 14:57
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