Skip to content

test(lcms): disperse PR #232 review tests (B1–B7) into per-subject suites - #314

Merged
PiTrem merged 1 commit into
231-display-lcms-datafrom
test/disperse-pr232-blocking
Jun 29, 2026
Merged

test(lcms): disperse PR #232 review tests (B1–B7) into per-subject suites#314
PiTrem merged 1 commit into
231-display-lcms-datafrom
test/disperse-pr232-blocking

Conversation

@PiTrem

@PiTrem PiTrem commented Jun 29, 2026

Copy link
Copy Markdown
Member

Stacked on #313 (base = fix/cv-density-submit-factor). Will retarget to 231-display-lcms-data once #313 merges. Tests only — no behaviour change.

What

The B1–B7 review regression tests were carried in a single PR-named file, src/__tests__/units/pr232_review_blocking.test.tsx, which served its purpose as the red→green proof for #232's blockers. As a permanent fixture it fought the repo's subject-based test layout. This PR moves each test into the suite that owns the code it guards and deletes the combined file.

Finding Now lives in
B1 — peak-add on a non-LCMS layout dispatches EDITPEAK.ADD_POSITIVE sagas/saga_ui_lcms.test.tsx (alongside the existing PEAK_ADD LCMS cases)
B2 — getArea on cumulative-k (NMR/CV) data helpers/integration.test.tsx
B3 — getAbsoluteArea from raw y helpers/integration.test.tsx
B4 — Convert2Peak honours stored LC/MS peaks + offset helpers/chem.test.tsx
B7 — convertThresEndPts empty-on-cleared precondition helpers/chem.test.tsx
B5 — CV current-density chart factor components/d3_multi/multi_focus.test.js (new)
B7 — drawBar empty-endpoint guard components/d3_line_rect.test.js

Two deliberate changes from a straight move

  • B6 dropped, not moved. Its test only re-implemented the source access forms inline (ViewerLineRect isn't exported), so it exercised no real component code — a tautology that gives false confidence in a permanent suite. Removed and tracked as a follow-up: it needs a testable seam (export ViewerLineRect, or extract the uvvis-seed builder) before it can guard the real componentDidUpdate guard.
  • B5 submit/export factor not duplicated. computeCvYScaleFactor is already covered in components/cmd_bar/r05_submit_btn.test.js (added in fix(cv): apply CV current-density factor once on submit/export (B5 remainder, for #232) #313). The B5 test moved here covers the chart factor (multi_focus.computeYTransformFactor), the other call site.

Test

All moved assertions stay green on the branch head:

helpers/integration.test.tsx · sagas/saga_ui_lcms.test.tsx · helpers/chem.test.tsx
components/d3_multi/multi_focus.test.js · components/d3_line_rect.test.js
→ Test Suites: 5 passed, Tests: 91 passed

refs #232

@PiTrem
PiTrem requested a review from headri June 29, 2026 11:05
@PiTrem
PiTrem changed the base branch from fix/cv-density-submit-factor to 231-display-lcms-data June 29, 2026 11:32
…ites

The B1–B7 review regression tests lived in one PR-named file
(pr232_review_blocking.test.tsx). Move each into the test file that owns the
code it guards, matching the repo's subject-based test layout:

- B1 peak-add on a non-LCMS layout      -> sagas/saga_ui_lcms.test.tsx
- B2 getArea on cumulative-k data       -> helpers/integration.test.tsx
- B3 getAbsoluteArea from raw y          -> helpers/integration.test.tsx
- B4 Convert2Peak honours stored peaks   -> helpers/chem.test.tsx
- B7 convertThresEndPts precondition     -> helpers/chem.test.tsx
- B5 CV current-density chart factor     -> components/d3_multi/multi_focus.test.js (new)
- B7 drawBar empty-endpoint guard        -> components/d3_line_rect.test.js

B6 (UV-Vis update-path guard) is dropped: its test only re-implemented the
source inline (ViewerLineRect is not exported), so it guarded nothing in the
real component. Tracked as a follow-up needing a testable seam.

B5's submit/export factor (computeCvYScaleFactor) is already covered in
components/cmd_bar/r05_submit_btn.test.js, so it is not duplicated here.

No behaviour change — tests only. All moved assertions stay green.

refs #232
@PiTrem
PiTrem force-pushed the test/disperse-pr232-blocking branch from 2d6e367 to 887b289 Compare June 29, 2026 11:34
@PiTrem
PiTrem merged commit f9c8eb5 into 231-display-lcms-data Jun 29, 2026
2 checks passed
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.

1 participant