From 4992eaf8bd0c4ee4c8a314b8043e782f5b127b56 Mon Sep 17 00:00:00 2001 From: dmudro Date: Tue, 19 Nov 2019 21:49:32 +0100 Subject: [PATCH 1/4] crude unit tests for counter visualisation utils --- .../app/visualizations/counter/utils.test.js | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 client/app/visualizations/counter/utils.test.js diff --git a/client/app/visualizations/counter/utils.test.js b/client/app/visualizations/counter/utils.test.js new file mode 100644 index 0000000000..e9fcfe93b9 --- /dev/null +++ b/client/app/visualizations/counter/utils.test.js @@ -0,0 +1,131 @@ +import { getCounterData } from './utils'; + +let dummy; + +describe('Visualizations -> Counter -> Utils', () => { + beforeEach(() => { + dummy = { + rows: [ + { city: 'New York City', population: 18604000 }, + { city: 'Shangai', population: 24484000 }, + { city: 'Tokyo', population: 38140000 }, + ], + result: { + counterLabel: undefined, + counterValue: '', + // TODO: consider default value for showTrend + // showTrend: false, + targetValue: null, + counterValueTooltip: '', + targetValueTooltip: '', + }, + }; + }); + + describe('getCounterData()', () => { + // test('no input returns empty result object', () => { + // const result = getCounterData(); + // expect(result).toEqual({}); + // }); + + test('empty rows returns empty result object', () => { + const result = getCounterData([]); + expect(result).toEqual({}); + }); + + describe('"Count rows" option is disabled', () => { + test('No target and counter values return empty result', () => { + const result = getCounterData(dummy.rows, {}, 'Vizualization Name'); + expect(result).toEqual({ + ...dummy.result, + counterLabel: 'Vizualization Name', + showTrend: false, + }); + }); + + test('"Counter label" overrides vizualization name', () => { + const result = getCounterData( + dummy.rows, + { counterLabel: 'Counter Label' }, + 'Vizualization Name', + ); + expect(result).toEqual({ + ...dummy.result, + counterLabel: 'Counter Label', + showTrend: false, + }); + }); + + test('"Counter Value Column Name" must be set to a correct non empty value', () => { + const result = getCounterData(dummy.rows, { rowNumber: 3 }); + expect(result).toEqual({ + ...dummy.result, + showTrend: false, + }); + + const result2 = getCounterData(dummy.rows, { counterColName: 'missingColumn' }); + expect(result2).toEqual({ + ...dummy.result, + showTrend: false, + }); + }); + + test('"Counter Value Column Name" uses correct column', () => { + const result = getCounterData(dummy.rows, { + counterColName: 'population', + }); + expect(result).toEqual({ + ...dummy.result, + counterValue: '18,604,000.000', + counterValueTooltip: '18,604,000', + showTrend: false, + }); + }); + + test('Counter and target values return correct result including trend', () => { + const result = getCounterData(dummy.rows, { + rowNumber: 1, + counterColName: 'population', + targetRowNumber: 2, + targetColName: 'population', + }); + expect(result).toEqual({ + ...dummy.result, + counterValue: '18,604,000.000', + counterValueTooltip: '18,604,000', + targetValue: '24484000', + targetValueTooltip: '24,484,000', + showTrend: true, + trendPositive: false, + }); + + const result2 = getCounterData(dummy.rows, { + rowNumber: 2, + counterColName: 'population', + targetRowNumber: 1, + targetColName: 'population', + }); + expect(result2).toEqual({ + ...dummy.result, + counterValue: '24,484,000.000', + counterValueTooltip: '24,484,000', + targetValue: '18604000', + targetValueTooltip: '18,604,000', + showTrend: true, + trendPositive: true, + }); + }); + }); + + test('"Count Rows" option is enabled', () => { + const result = getCounterData(dummy.rows, { countRow: true }); + + expect(result).toEqual({ + ...dummy.result, + counterValue: '3.000', + counterValueTooltip: '3', + showTrend: false, + }); + }); + }); +}); From 4f42b0a70c95654708cc2532770ef840be31e194 Mon Sep 17 00:00:00 2001 From: dmudro Date: Thu, 21 Nov 2019 13:15:23 +0100 Subject: [PATCH 2/4] improve type safety with default param values for getCounterData() --- client/app/visualizations/counter/utils.js | 10 +++------- client/app/visualizations/counter/utils.test.js | 12 +++++------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/client/app/visualizations/counter/utils.js b/client/app/visualizations/counter/utils.js index 8a4bd1b0f6..23857e11f0 100644 --- a/client/app/visualizations/counter/utils.js +++ b/client/app/visualizations/counter/utils.js @@ -70,7 +70,7 @@ function formatTooltip(value, formatString) { return toString(value); } -export function getCounterData(rows, options, visualizationName) { +export function getCounterData(rows = [], options = {}, visualizationName = '') { const result = {}; const rowsCount = rows.length; @@ -79,13 +79,8 @@ export function getCounterData(rows, options, visualizationName) { const targetRowNumber = getRowNumber(options.targetRowNumber, rowsCount); const counterColName = options.counterColName; const targetColName = options.targetColName; - const counterLabel = options.counterLabel; - if (counterLabel) { - result.counterLabel = counterLabel; - } else { - result.counterLabel = visualizationName; - } + result.counterLabel = options.counterLabel || visualizationName; if (options.countRow) { result.counterValue = rowsCount; @@ -94,6 +89,7 @@ export function getCounterData(rows, options, visualizationName) { } result.showTrend = false; + if (targetColName) { result.targetValue = rows[targetRowNumber][targetColName]; diff --git a/client/app/visualizations/counter/utils.test.js b/client/app/visualizations/counter/utils.test.js index e9fcfe93b9..6f49796166 100644 --- a/client/app/visualizations/counter/utils.test.js +++ b/client/app/visualizations/counter/utils.test.js @@ -11,10 +11,8 @@ describe('Visualizations -> Counter -> Utils', () => { { city: 'Tokyo', population: 38140000 }, ], result: { - counterLabel: undefined, + counterLabel: '', counterValue: '', - // TODO: consider default value for showTrend - // showTrend: false, targetValue: null, counterValueTooltip: '', targetValueTooltip: '', @@ -23,10 +21,10 @@ describe('Visualizations -> Counter -> Utils', () => { }); describe('getCounterData()', () => { - // test('no input returns empty result object', () => { - // const result = getCounterData(); - // expect(result).toEqual({}); - // }); + test('no input returns empty result object', () => { + const result = getCounterData(); + expect(result).toEqual({}); + }); test('empty rows returns empty result object', () => { const result = getCounterData([]); From 8956a46e3d99c4cd238b44eeff76319405b7ade7 Mon Sep 17 00:00:00 2001 From: dmudro Date: Fri, 22 Nov 2019 00:32:05 +0100 Subject: [PATCH 3/4] fix count rows never shows zero --- client/app/visualizations/counter/utils.js | 8 +-- .../app/visualizations/counter/utils.test.js | 55 ++++++++++++++++--- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/client/app/visualizations/counter/utils.js b/client/app/visualizations/counter/utils.js index 23857e11f0..205cf9a3a4 100644 --- a/client/app/visualizations/counter/utils.js +++ b/client/app/visualizations/counter/utils.js @@ -72,11 +72,9 @@ function formatTooltip(value, formatString) { export function getCounterData(rows = [], options = {}, visualizationName = '') { const result = {}; - const rowsCount = rows.length; - if (rowsCount > 0) { - const rowNumber = getRowNumber(options.rowNumber, rowsCount); - const targetRowNumber = getRowNumber(options.targetRowNumber, rowsCount); + + if (rowsCount > 0 || options.countRow) { const counterColName = options.counterColName; const targetColName = options.targetColName; @@ -85,12 +83,14 @@ export function getCounterData(rows = [], options = {}, visualizationName = '') if (options.countRow) { result.counterValue = rowsCount; } else if (counterColName) { + const rowNumber = getRowNumber(options.rowNumber, rowsCount); result.counterValue = rows[rowNumber][counterColName]; } result.showTrend = false; if (targetColName) { + const targetRowNumber = getRowNumber(options.targetRowNumber, rowsCount); result.targetValue = rows[targetRowNumber][targetColName]; if (Number.isFinite(result.counterValue) && isFinite(result.targetValue)) { diff --git a/client/app/visualizations/counter/utils.test.js b/client/app/visualizations/counter/utils.test.js index 6f49796166..be5bc65918 100644 --- a/client/app/visualizations/counter/utils.test.js +++ b/client/app/visualizations/counter/utils.test.js @@ -115,14 +115,53 @@ describe('Visualizations -> Counter -> Utils', () => { }); }); - test('"Count Rows" option is enabled', () => { - const result = getCounterData(dummy.rows, { countRow: true }); - - expect(result).toEqual({ - ...dummy.result, - counterValue: '3.000', - counterValueTooltip: '3', - showTrend: false, + describe('"Count rows" option is enabled', () => { + beforeEach(() => { + dummy.result = { + ...dummy.result, + counterValue: '3.000', + counterValueTooltip: '3', + showTrend: false, + }; + }); + + test('Rows are counted correctly', () => { + const result = getCounterData(dummy.rows, { countRow: true }); + + expect(result).toEqual(dummy.result); + }); + + test('Counter value is ignored', () => { + const result = getCounterData(dummy.rows, { + countRow: true, + rowNumber: 3, + counterColName: 'population', + }); + expect(result).toEqual(dummy.result); + }); + + test('Target value and trend are computed correctly', () => { + const result = getCounterData(dummy.rows, { + countRow: true, + targetRowNumber: 2, + targetColName: 'population', + }); + expect(result).toEqual({ + ...dummy.result, + targetValue: '24484000', + targetValueTooltip: '24,484,000', + showTrend: true, + trendPositive: false, + }); + }); + + test('Empty rows return counter value 0', () => { + const result = getCounterData([], { countRow: true }); + expect(result).toEqual({ + ...dummy.result, + counterValue: '0.000', + counterValueTooltip: '0', + }); }); }); }); From 213a095a27a4f66ba082b098c205e593f9be57f6 Mon Sep 17 00:00:00 2001 From: dmudro Date: Fri, 29 Nov 2019 17:09:50 +0100 Subject: [PATCH 4/4] remove default values for getCounterData() params --- client/app/visualizations/counter/utils.js | 2 +- .../app/visualizations/counter/utils.test.js | 112 +++++++++++------- 2 files changed, 69 insertions(+), 45 deletions(-) diff --git a/client/app/visualizations/counter/utils.js b/client/app/visualizations/counter/utils.js index 205cf9a3a4..73f7001a06 100644 --- a/client/app/visualizations/counter/utils.js +++ b/client/app/visualizations/counter/utils.js @@ -70,7 +70,7 @@ function formatTooltip(value, formatString) { return toString(value); } -export function getCounterData(rows = [], options = {}, visualizationName = '') { +export function getCounterData(rows, options, visualizationName) { const result = {}; const rowsCount = rows.length; diff --git a/client/app/visualizations/counter/utils.test.js b/client/app/visualizations/counter/utils.test.js index be5bc65918..99a515ac2f 100644 --- a/client/app/visualizations/counter/utils.test.js +++ b/client/app/visualizations/counter/utils.test.js @@ -10,8 +10,10 @@ describe('Visualizations -> Counter -> Utils', () => { { city: 'Shangai', population: 24484000 }, { city: 'Tokyo', population: 38140000 }, ], + options: {}, + visualisationName: 'Visualisation Name', result: { - counterLabel: '', + counterLabel: 'Visualisation Name', counterValue: '', targetValue: null, counterValueTooltip: '', @@ -21,22 +23,11 @@ describe('Visualizations -> Counter -> Utils', () => { }); describe('getCounterData()', () => { - test('no input returns empty result object', () => { - const result = getCounterData(); - expect(result).toEqual({}); - }); - - test('empty rows returns empty result object', () => { - const result = getCounterData([]); - expect(result).toEqual({}); - }); - describe('"Count rows" option is disabled', () => { test('No target and counter values return empty result', () => { - const result = getCounterData(dummy.rows, {}, 'Vizualization Name'); + const result = getCounterData(dummy.rows, dummy.options, dummy.visualisationName); expect(result).toEqual({ ...dummy.result, - counterLabel: 'Vizualization Name', showTrend: false, }); }); @@ -45,7 +36,7 @@ describe('Visualizations -> Counter -> Utils', () => { const result = getCounterData( dummy.rows, { counterLabel: 'Counter Label' }, - 'Vizualization Name', + dummy.visualisationName, ); expect(result).toEqual({ ...dummy.result, @@ -55,13 +46,21 @@ describe('Visualizations -> Counter -> Utils', () => { }); test('"Counter Value Column Name" must be set to a correct non empty value', () => { - const result = getCounterData(dummy.rows, { rowNumber: 3 }); + const result = getCounterData( + dummy.rows, + { rowNumber: 3 }, + dummy.visualisationName, + ); expect(result).toEqual({ ...dummy.result, showTrend: false, }); - const result2 = getCounterData(dummy.rows, { counterColName: 'missingColumn' }); + const result2 = getCounterData( + dummy.rows, + { counterColName: 'missingColumn' }, + dummy.visualisationName, + ); expect(result2).toEqual({ ...dummy.result, showTrend: false, @@ -69,9 +68,11 @@ describe('Visualizations -> Counter -> Utils', () => { }); test('"Counter Value Column Name" uses correct column', () => { - const result = getCounterData(dummy.rows, { - counterColName: 'population', - }); + const result = getCounterData( + dummy.rows, + { counterColName: 'population' }, + dummy.visualisationName, + ); expect(result).toEqual({ ...dummy.result, counterValue: '18,604,000.000', @@ -81,12 +82,16 @@ describe('Visualizations -> Counter -> Utils', () => { }); test('Counter and target values return correct result including trend', () => { - const result = getCounterData(dummy.rows, { - rowNumber: 1, - counterColName: 'population', - targetRowNumber: 2, - targetColName: 'population', - }); + const result = getCounterData( + dummy.rows, + { + rowNumber: 1, + counterColName: 'population', + targetRowNumber: 2, + targetColName: 'population', + }, + dummy.visualisationName, + ); expect(result).toEqual({ ...dummy.result, counterValue: '18,604,000.000', @@ -97,12 +102,16 @@ describe('Visualizations -> Counter -> Utils', () => { trendPositive: false, }); - const result2 = getCounterData(dummy.rows, { - rowNumber: 2, - counterColName: 'population', - targetRowNumber: 1, - targetColName: 'population', - }); + const result2 = getCounterData( + dummy.rows, + { + rowNumber: 2, + counterColName: 'population', + targetRowNumber: 1, + targetColName: 'population', + }, + dummy.visualisationName, + ); expect(result2).toEqual({ ...dummy.result, counterValue: '24,484,000.000', @@ -126,26 +135,37 @@ describe('Visualizations -> Counter -> Utils', () => { }); test('Rows are counted correctly', () => { - const result = getCounterData(dummy.rows, { countRow: true }); - + const result = getCounterData( + dummy.rows, + { countRow: true }, + dummy.visualisationName, + ); expect(result).toEqual(dummy.result); }); test('Counter value is ignored', () => { - const result = getCounterData(dummy.rows, { - countRow: true, - rowNumber: 3, - counterColName: 'population', - }); + const result = getCounterData( + dummy.rows, + { + countRow: true, + rowNumber: 3, + counterColName: 'population', + }, + dummy.visualisationName, + ); expect(result).toEqual(dummy.result); }); test('Target value and trend are computed correctly', () => { - const result = getCounterData(dummy.rows, { - countRow: true, - targetRowNumber: 2, - targetColName: 'population', - }); + const result = getCounterData( + dummy.rows, + { + countRow: true, + targetRowNumber: 2, + targetColName: 'population', + }, + dummy.visualisationName, + ); expect(result).toEqual({ ...dummy.result, targetValue: '24484000', @@ -156,7 +176,11 @@ describe('Visualizations -> Counter -> Utils', () => { }); test('Empty rows return counter value 0', () => { - const result = getCounterData([], { countRow: true }); + const result = getCounterData( + [], + { countRow: true }, + dummy.visualisationName, + ); expect(result).toEqual({ ...dummy.result, counterValue: '0.000',