Rewrite the HARD-tier and remaining merge tests with React Testing Library

[jonathangreen]

Description

The HARD-tier suites and the "remaining merge" cases, across two commits.

Rewrite more HARD-tier tests — 12 suites: the announcement/genre/contributor/paired-menu forms, ChangePasswordForm, NeighborhoodAnalyticsForm, TextWithEditMode, ToolTip, and three that merge into existing jest files (Lane, EditableInput, IndividualAdmins). Prototype stubs (EditableInput.prototype.getValue, TextWithEditMode) become real userEvent.type reading back through the component's ref API; state()/instance() reads become DOM assertions.

Rewrite the HARD-heavy and remaining merge tests — 20 more, the hardest of the migration: ServiceEditForm (1087 LOC), CustomListEntriesEditor (1461 LOC), Header, CatalogPage, EditorField, InputList, Lanes, LaneCustomListsEditor, LanesSidebar, LibraryRegistration, SelfTests, BookCoverEditor, ClassificationsForm, plus seven merges. Highlights:

• Live-instance-method spies become observable behavior (asserting submitted FormData, removed/retained rows), with tiny test subclasses to drive branches the base method short-circuits.
• react-beautiful-dnd (can't drag in jsdom) is handled by mocking DragDropContext to capture onDragStart/onDragEnd, then invoking them with synthetic drag results — CustomListEntriesEditor/LaneCustomListsEditor reach 100% this way.
• Connected panels render their connected default export so mapStateToProps/mapDispatchToProps stay covered.

userEvent usage follows the const user = userEvent.setup() convention across tests/jest/.

Motivation and Context

Part of the mocha/chai/enzyme → jest/RTL migration — the hardest suites, which enzyme mounted and which poked class internals. After this, the only legacy enzyme suites left are the deferred tab containers, whose panels are now all migrated with connected coverage.

How Has This Been Tested?

npx jest --selectProjects=unit — full suite green. Coverage rises to 95.21% (+75 lines over baseline) across the two commits; no previously-covered line lost. eslint + prettier clean.

Checklist:

• I have updated the documentation accordingly.
• All new and existing tests passed.

[claude]

Claude finished @jonathangreen's task in 25m 18s —— View job

---

Code Review

• Read the PR diff and understand the scope
• Review the high-risk suites (BookCoverEditor, CustomListEntriesEditor, ServiceEditForm, drag mocks)
• Check merged/modified suites for lost coverage or weakened assertions
• Post review

Summary

This is a large but clean test-only migration. I reviewed the two big suites (ServiceEditForm, CustomListEntriesEditor), the ten merged/modified files, and every newly-added suite — tracing load-bearing assertions against the real components rather than the mocks. The react-beautiful-dnd and CatalogLink mocks match the shapes the components actually read; the drag-handler tests drive real branch logic through captured callbacks; connected-export tests genuinely exercise mapStateToProps/mapDispatchToProps (e.g. SelfTests waits on "2.50s", which only appears if the swap branch runs); the test subclasses in ServiceEditForm assert DOM outcomes driven by real gating rather than re-asserting the override; and userEvent/findBy/waitFor are consistently awaited with no zero-iteration forEach no-ops. On the draggableStyle question raised in-thread: react-beautiful-dnd@^2 does expose it top-level on provided, so the mock is correct. I have one minor, non-blocking suggestion below.

Details

Minor: tests/jest/components/BookCoverEditor.test.tsx:379-390

The "saves the cover" test asserts cover_url, title_position, rights_status, and rights_explanation in the posted payload but never asserts anything about cover_file. save() unconditionally does data.append("cover_file", imageFormData.get("cover_file")) (BookCoverEditor.tsx:298), and while the shim can't reproduce a File value, it does record the key — so deleting that append line would slip through the current test undetected. Adding a presence check closes that gap without depending on file semantics:

expect(editCall[1].body.has("cover_file")).toBe(true);

circulation-admin/tests/jest/components/BookCoverEditor.test.tsx

Lines 379 to 390 in 4e4fdfa

	const editCall = fetchSpy.mock.calls.find(([u]) =>
	String(u).includes("change_cover")
	);
	expect(editCall).toBeTruthy();
	expect(editCall[1].method).toBe("POST");
	expect(editCall[1].body.get("cover_url")).toBe("http://example.com");
	expect(editCall[1].body.get("title_position")).toBe("center");
	expect(editCall[1].body.get("rights_status")).toBe(
	"http://creativecommons.org/licenses/by/4.0/"
	);
	expect(editCall[1].body.get("rights_explanation")).toBe("explanation");

[greptile-apps]

Greptile Summary

This PR rewrites the HARD-tier and remaining merge tests with React Testing Library. The main changes are:

• Replaces Enzyme component tests under src/components/__tests__ with Jest tests under tests/jest/components.
• Moves class-instance and state assertions toward DOM and user-event assertions.
• Adds connected-component coverage for several Redux-backed panels.
• Uses mocked drag callbacks for react-beautiful-dnd paths that cannot run in jsdom.

Confidence Score: 4/5

This is close, but the uploaded-cover test gap should be fixed before merging.

• The BookCoverEditor test still does not assert the cover_file payload.
• The current shim can make file-upload saves pass with data that does not match the browser form submission.
• The drag-and-drop mock shape matches the installed library contract for the reviewed path.

tests/jest/components/BookCoverEditor.test.tsx

Important Files Changed

Filename	Overview
tests/jest/components/BookCoverEditor.test.tsx	Rewrites BookCoverEditor tests with connected Redux coverage, but the uploaded-cover save payload is still not exercised.
tests/jest/components/LaneCustomListsEditor.test.tsx	Rewrites LaneCustomListsEditor tests with mocked drag callbacks and RTL assertions for list add, remove, and ref behavior.

_{Reviews (7): Last reviewed commit: "Rewrite the HARD-heavy and remaining mer..." | Re-trigger Greptile}

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.90%. Comparing base (7014b93) to head (4e4fdfa).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #316      +/-   ##
==========================================
+ Coverage   90.19%   90.90%   +0.70%     
==========================================
  Files         205      205              
  Lines        7610     7610              
  Branches     1664     1664              
==========================================
+ Hits         6864     6918      +54     
+ Misses        437      394      -43     
+ Partials      309      298      -11     

Flag	Coverage Δ
jest	90.90% <ø> (+0.70%)	⬆️

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.

[greptile-apps]

Cover File Still Missing

This save test only exercises cover_url and never selects or asserts the cover_file field. The suite installs a FormData shim that omits file inputs, while a browser new FormData(imageForm) includes the uploaded cover as a File. A change that drops the uploaded cover from BookCoverEditor.save() would still pass this test because the posted body is only checked for URL, title position, and rights fields. Please add a file-upload save case or preserve File entries in the test shim so uploaded-cover submissions are asserted.

[jonathangreen]

I'm going to close this monster, and try to split it into a few smaller more reviewable PRs.