Skip to content

Commit 887b289

Browse files
committed
test(lcms): disperse PR #232 review tests (B1–B7) into per-subject suites
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
1 parent 4f17463 commit 887b289

6 files changed

Lines changed: 134 additions & 188 deletions

File tree

src/__tests__/units/components/d3_line_rect.test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { isLcmsMsPageLoading } from '../../../components/d3_line_rect/index';
22
import { pickTicIndex } from '../../../components/d3_line_rect/multi_focus';
3+
import RectFocus from '../../../components/d3_line_rect/rect_focus';
34

45
describe('isLcmsMsPageLoading', () => {
56
const buildMzEntity = (polarity, pageValues) => ({
@@ -66,3 +67,19 @@ describe('isLcmsMsPageLoading', () => {
6667
expect(isLcmsMsPageLoading([], state)).toEqual(true);
6768
});
6869
});
70+
71+
// Review finding B7 (#232): drawBar reads tTrEndPts[0].y. Clearing the
72+
// threshold makes convertThresEndPts return [] (see chem.test.tsx) while the
73+
// MS bars are still present, so drawBar must guard the empty endpoint list
74+
// instead of crashing.
75+
describe('RectFocus.drawBar with an empty threshold-endpoint list (B7)', () => {
76+
it('does not crash when tTrEndPts is empty but bars exist', () => {
77+
const rf = Object.create(RectFocus.prototype);
78+
rf.bars = {}; // truthy → passes the `if (!this.bars)` guard
79+
rf.scales = { x: (v) => v, y: (v) => v }; // TfRescale reads focus.scales.{x,y}
80+
rf.updatePathCall = () => {}; // stub out the d3 path update
81+
rf.data = [{ x: 1, y: 2 }]; // bars present
82+
rf.tTrEndPts = []; // cleared threshold → empty endpoints
83+
expect(() => rf.drawBar()).not.toThrow();
84+
});
85+
});
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import MultiFocus from '../../../../components/d3_multi/multi_focus';
2+
import { LIST_LAYOUT } from '../../../../constants/list_layout';
3+
4+
// Review finding B5 (#232): the CV current-density factor on the chart
5+
// (computeYTransformFactor) must convert the electrode area to cm² exactly
6+
// once. Physically 100 mm² == 1 cm², so the factor for the two must match —
7+
// double-dividing by 100 for mm² made it 100x too small.
8+
// (computeYTransformFactor is pure — it ignores `this` — so it is called via
9+
// the prototype.)
10+
describe('MultiFocus.computeYTransformFactor — CV current density (B5)', () => {
11+
const compute = (cvSt) => MultiFocus.prototype.computeYTransformFactor.call(
12+
{}, LIST_LAYOUT.CYCLIC_VOLTAMMETRY, cvSt, { yUnit: 'A' },
13+
);
14+
15+
it('treats 100 mm² the same as 1 cm²', () => {
16+
const fMm2 = compute({ useCurrentDensity: true, areaValue: 100, areaUnit: 'mm²' });
17+
const fCm2 = compute({ useCurrentDensity: true, areaValue: 1, areaUnit: 'cm²' });
18+
expect(fMm2).toBeCloseTo(fCm2);
19+
expect(fMm2).toBeCloseTo(1.0); // double /100 for mm² gives 0.01
20+
});
21+
22+
it('gives A/cm² for a mm² area (factor = 100 / area_mm²)', () => {
23+
// 50 mm² == 0.5 cm² → factor 1 / 0.5 = 2
24+
expect(compute({ useCurrentDensity: true, areaValue: 50, areaUnit: 'mm²' })).toBeCloseTo(2.0);
25+
});
26+
27+
it('returns 1.0 when current density is off', () => {
28+
expect(compute({ useCurrentDensity: false })).toBeCloseTo(1.0);
29+
});
30+
});

src/__tests__/units/helpers/chem.test.tsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
ToFrequency, Convert2Scan, Convert2Thres, GetComparisons, Convert2DValue,
44
GetCyclicVoltaRatio, GetCyclicVoltaPeakSeparate, convertTopic,
55
Convert2MaxMinPeak, Feature2MaxMinPeak, GetCyclicVoltaShiftOffset, GetCyclicVoltaPreviousShift,
6+
convertThresEndPts,
67
} from "../../../helpers/chem";
78
import nmr1HJcamp from "../../fixtures/nmr1h_jcamp";
89
import aifJcamp1 from "../../fixtures/aif_jcamp_1";
@@ -193,6 +194,29 @@ describe('Test for chem helper', () => {
193194
const peaks = Convert2Peak(feature, threshold, offset)
194195
expect(peaks).toEqual([{x: 2, y: 2}, {x: -2, y: -2}])
195196
})
197+
198+
// Review finding B4 (#232): for an LC/MS feature carrying edited/stored
199+
// peaks, Convert2Peak must return those peaks with the offset applied,
200+
// rather than recomputing them from the raw data and dropping the offset.
201+
it('honours stored LC/MS peaks and applies the offset', () => {
202+
const feature = {
203+
operation: { layout: 'LC/MS' },
204+
data: [{ x: [10, 11, 12], y: [1, 5, 1] }],
205+
peaks: [{ x: 5, y: 100 }], // user-edited / stored peaks
206+
}
207+
const peaks = Convert2Peak(feature, 0, 2)
208+
expect(peaks).toEqual([{ x: 3, y: 100 }])
209+
})
210+
})
211+
212+
// Review finding B7 (#232) precondition: clearing the threshold input yields
213+
// an empty endpoint list (this is the state that made drawBar crash — see
214+
// d3_line_rect.test.js for the drawBar guard itself).
215+
describe('convertThresEndPts', () => {
216+
it('returns [] when the threshold is cleared', () => {
217+
const feature = { maxY: 100, maxX: 10, minX: 0, data: [{ x: [1, 2], y: [3, 4] }] }
218+
expect(convertThresEndPts(feature, '')).toEqual([])
219+
})
196220
})
197221

198222
describe('Feature2Peak', () => {

src/__tests__/units/helpers/integration.test.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,40 @@ describe('Test helper for integration', () => {
4242
expect(area).toEqual('0.52')
4343
})
4444
})
45+
46+
// Review finding B2 (#232): NMR/CV integration data carries a cumulative
47+
// running integral `k` (from calcXYK). The integral over [xL,xU] is the
48+
// difference k(xU)-k(xL); getArea must NOT trapezoidally integrate `k` again.
49+
describe('getArea on cumulative-k (NMR/CV) data — review B2', () => {
50+
// x ramp, constant normalised signal => k increases by 1 each sample.
51+
const data = [
52+
{ x: 0, y: 1, k: 0 },
53+
{ x: 1, y: 1, k: 1 },
54+
{ x: 2, y: 1, k: 2 },
55+
{ x: 3, y: 1, k: 3 },
56+
]
57+
58+
it('equals the cumulative difference k(xU)-k(xL)', () => {
59+
expect(getArea(0, 3, data)).toBeCloseTo(3) // double-integrating k gives 4.5
60+
})
61+
62+
it('is invariant to a constant baseline offset of the cumulative curve', () => {
63+
const shifted = data.map((p) => ({ ...p, k: p.k + 100 }))
64+
// The signal integral cannot depend on the integration constant of k.
65+
expect(getArea(0, 3, shifted)).toBeCloseTo(getArea(0, 3, data))
66+
})
67+
})
68+
69+
// Review finding B3 (#232): getAbsoluteArea must use the raw signal `y`
70+
// (baseline-subtracted), not the cumulative `k`, when data carries both.
71+
describe('getAbsoluteArea on data carrying both y and k — review B3', () => {
72+
const data = [
73+
{ x: 1, y: 1, k: 1 },
74+
{ x: 2, y: 2, k: 3 }, // peak in the raw signal y
75+
{ x: 3, y: 1, k: 4 },
76+
]
77+
it('computes the area from raw y (1.0), not from cumulative k (0.5)', () => {
78+
expect(getAbsoluteArea(0, 4, data)).toBeCloseTo(1.0)
79+
})
80+
})
4581
})

src/__tests__/units/pr232_review_blocking.test.tsx

Lines changed: 0 additions & 188 deletions
This file was deleted.

src/__tests__/units/sagas/saga_ui_lcms.test.tsx

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,33 @@ describe('saga_ui — clickUiTarget LCMS branches', () => {
258258
expect(value).toBeUndefined();
259259
});
260260

261+
// Review finding B1 (#232): on a NON-LCMS layout, PEAK_ADD must still add a
262+
// positive edit-peak (EDITPEAK.ADD_POSITIVE). The LCMS-only path must not
263+
// swallow peak-add for NMR/IR/MS layouts.
264+
it('PEAK_ADD on a non-LCMS layout dispatches EDITPEAK.ADD_POSITIVE', () => {
265+
const action = {
266+
type: UI.CLICK_TARGET,
267+
payload: { x: 5, y: 10 },
268+
sourceHint: null,
269+
onPeak: false,
270+
onPecker: false,
271+
voltammetryPeakIdx: 0,
272+
} as any;
273+
const iter = clickUiTarget(action);
274+
275+
const { value } = drainSelects(iter, [
276+
LIST_UI_SWEEP_TYPE.PEAK_ADD,
277+
{ curveIdx: 0 },
278+
{ uvvis: { selectedWaveLength: null, currentSpectrum: { peaks: [] } } },
279+
LIST_LAYOUT.H1, // a normal NMR layout — not LC/MS
280+
]);
281+
282+
expect(value).toEqual(put({
283+
type: EDITPEAK.ADD_POSITIVE,
284+
payload: { dataToAdd: action.payload, curveIdx: 0 },
285+
}));
286+
});
287+
261288
it('PEAK_DELETE on LCMS uses REMOVE_HPLCMS_PEAK for the selected wavelength', () => {
262289
const action = {
263290
type: UI.CLICK_TARGET,

0 commit comments

Comments
 (0)