Skip to content

Commit 79d0ee1

Browse files
author
M.D
authored
fix(ellipticalROI): give default values for cachedStats values to avoid undefined error (#1537)
* [bugfix] give default values for cachedStats values to avoid undefined error * coding rule fix * added new tests to increaes coverage * test refactor
1 parent 925a3dd commit 79d0ee1

File tree

4 files changed

+162
-4
lines changed

4 files changed

+162
-4
lines changed

src/tools/annotation/EllipticalRoiTool.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ function _getUnit(modality, showHounsfieldUnits) {
355355
function _createTextBoxContent(
356356
context,
357357
isColorImage,
358-
{ area, mean, stdDev, min, max, meanStdDevSUV } = {},
358+
{ area = 0, mean = 0, stdDev = 0, min = 0, max = 0, meanStdDevSUV = 0 } = {},
359359
modality,
360360
hasPixelSpacing,
361361
options = {}

src/tools/annotation/EllipticalRoiTool.test.js

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,27 @@
11
import EllipticalRoiTool from './EllipticalRoiTool.js';
22
import { getToolState } from './../../stateManagement/toolState.js';
33
import { getLogger } from '../../util/logger.js';
4+
import getNewContext from '../../drawing/getNewContext.js';
5+
import drawEllipse from '../../drawing/drawEllipse.js';
6+
7+
/* ~ Setup
8+
* To mock properly, Jest needs jest.mock('moduleName') to be in the
9+
* same scope as the require/import statement.
10+
*/
11+
import external from '../../externalModules.js';
412

513
jest.mock('../../util/logger.js');
614
jest.mock('./../../stateManagement/toolState.js', () => ({
715
getToolState: jest.fn(),
816
}));
17+
jest.mock('../../drawing/drawEllipse', () => ({
18+
__esModule: true,
19+
default: jest.fn(),
20+
}));
21+
jest.mock('../../drawing/getNewContext', () => ({
22+
__esModule: true,
23+
default: jest.fn(),
24+
}));
925

1026
jest.mock('./../../importInternal.js', () => ({
1127
default: jest.fn(),
@@ -19,6 +35,7 @@ jest.mock('./../../externalModules.js', () => ({
1935
/* eslint-disable prettier/prettier */
2036
getPixels: () => [100, 100, 100, 100, 4, 5, 100, 3, 6],
2137
/* eslint-enable prettier/prettier */
38+
pixelToCanvas: jest.fn(),
2239
},
2340
}));
2441

@@ -215,6 +232,20 @@ describe('EllipticalRoiTool.js', () => {
215232
});
216233

217234
describe('renderToolData', () => {
235+
beforeAll(() => {
236+
getNewContext.mockReturnValue({
237+
save: jest.fn(),
238+
restore: jest.fn(),
239+
beginPath: jest.fn(),
240+
arc: jest.fn(),
241+
stroke: jest.fn(),
242+
fillRect: jest.fn(),
243+
fillText: jest.fn(),
244+
measureText: jest.fn(() => ({ width: 1 })),
245+
});
246+
external.cornerstone.pixelToCanvas.mockImplementation((comp, val) => val);
247+
});
248+
218249
it('returns undefined when no toolData exists for the tool', () => {
219250
const instantiatedTool = new EllipticalRoiTool();
220251
const mockEvent = {
@@ -228,5 +259,55 @@ describe('EllipticalRoiTool.js', () => {
228259

229260
expect(renderResult).toBe(undefined);
230261
});
262+
263+
describe('draw ellipse with color', () => {
264+
const defaulColor = 'white';
265+
const mockEvent = {
266+
detail: {
267+
element: {},
268+
canvasContext: {
269+
canvas: {},
270+
},
271+
image: {},
272+
viewport: {},
273+
},
274+
};
275+
const instantiatedTool = new EllipticalRoiTool({
276+
configuration: {},
277+
});
278+
279+
const toolState = {
280+
data: [
281+
{
282+
visible: true,
283+
active: false,
284+
handles: {
285+
start: {
286+
x: 0,
287+
y: 0,
288+
},
289+
end: {
290+
x: 3,
291+
y: 3,
292+
},
293+
textBox: {},
294+
},
295+
},
296+
],
297+
};
298+
299+
const expectDraw = color => {
300+
expect(drawEllipse.mock.calls.length).toBe(1);
301+
};
302+
303+
it('should draw an ellipse with the inactive color', () => {
304+
toolState.data[0].active = false;
305+
getToolState.mockReturnValue(toolState);
306+
307+
instantiatedTool.renderToolData(mockEvent);
308+
309+
expectDraw(defaulColor);
310+
});
311+
});
231312
});
232313
});

src/tools/annotation/RectangleRoiTool.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ function _getUnit(modality, showHounsfieldUnits) {
478478
function _createTextBoxContent(
479479
context,
480480
isColorImage,
481-
{ area, mean, stdDev, min, max, meanStdDevSUV },
481+
{ area = 0, mean = 0, stdDev = 0, min = 0, max = 0, meanStdDevSUV = 0 } = {},
482482
modality,
483483
hasPixelSpacing,
484484
options = {}

src/tools/annotation/RectangleRoiTool.test.js

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,25 @@
11
import RectangleRoiTool from './RectangleRoiTool.js';
22
import { getToolState } from './../../stateManagement/toolState.js';
33
import { getLogger } from '../../util/logger.js';
4+
import getNewContext from '../../drawing/getNewContext.js';
5+
import drawRect from '../../drawing/drawRect.js';
6+
7+
/* ~ Setup
8+
* To mock properly, Jest needs jest.mock('moduleName') to be in the
9+
* same scope as the require/import statement.
10+
*/
11+
import external from '../../externalModules.js';
412

513
jest.mock('../../util/logger.js');
614
jest.mock('./../../stateManagement/toolState.js', () => ({
715
getToolState: jest.fn(),
816
}));
9-
10-
jest.mock('./../../importInternal.js', () => ({
17+
jest.mock('../../drawing/drawRect', () => ({
18+
__esModule: true,
19+
default: jest.fn(),
20+
}));
21+
jest.mock('../../drawing/getNewContext', () => ({
22+
__esModule: true,
1123
default: jest.fn(),
1224
}));
1325

@@ -19,6 +31,7 @@ jest.mock('./../../externalModules.js', () => ({
1931
/* eslint-enable prettier/prettier */
2032
getPixels: () => [100, 100, 100, 100, 4, 5, 100, 3, 6],
2133
/* eslint-enable prettier/prettier */
34+
pixelToCanvas: jest.fn(),
2235
},
2336
}));
2437

@@ -218,6 +231,20 @@ describe('RectangleRoiTool.js', () => {
218231
});
219232

220233
describe('renderToolData', () => {
234+
beforeAll(() => {
235+
getNewContext.mockReturnValue({
236+
save: jest.fn(),
237+
restore: jest.fn(),
238+
beginPath: jest.fn(),
239+
arc: jest.fn(),
240+
stroke: jest.fn(),
241+
fillRect: jest.fn(),
242+
fillText: jest.fn(),
243+
measureText: jest.fn(() => ({ width: 1 })),
244+
});
245+
external.cornerstone.pixelToCanvas.mockImplementation((comp, val) => val);
246+
});
247+
221248
it('returns undefined when no toolData exists for the tool', () => {
222249
const instantiatedTool = new RectangleRoiTool();
223250
const mockEvent = {
@@ -231,5 +258,55 @@ describe('RectangleRoiTool.js', () => {
231258

232259
expect(renderResult).toBe(undefined);
233260
});
261+
262+
describe('draw rectangle with color', () => {
263+
const defaulColor = 'white';
264+
const mockEvent = {
265+
detail: {
266+
element: {},
267+
canvasContext: {
268+
canvas: {},
269+
},
270+
image: {},
271+
viewport: {},
272+
},
273+
};
274+
const instantiatedTool = new RectangleRoiTool({
275+
configuration: {},
276+
});
277+
278+
const toolState = {
279+
data: [
280+
{
281+
visible: true,
282+
active: false,
283+
handles: {
284+
start: {
285+
x: 0,
286+
y: 0,
287+
},
288+
end: {
289+
x: 3,
290+
y: 3,
291+
},
292+
textBox: {},
293+
},
294+
},
295+
],
296+
};
297+
298+
const expectDraw = color => {
299+
expect(drawRect.mock.calls.length).toBe(1);
300+
};
301+
302+
it('should draw a rectangle with the inactive color', () => {
303+
toolState.data[0].active = false;
304+
getToolState.mockReturnValue(toolState);
305+
306+
instantiatedTool.renderToolData(mockEvent);
307+
308+
expectDraw(defaulColor);
309+
});
310+
});
234311
});
235312
});

0 commit comments

Comments
 (0)