From c04fd046598b0b262c77dda61b756c4456ed9793 Mon Sep 17 00:00:00 2001 From: Teal Hobson-Lowther Date: Thu, 4 Oct 2018 10:35:01 -0700 Subject: [PATCH 1/4] fix regression due to poor array access --- validators/headerFields.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/validators/headerFields.js b/validators/headerFields.js index e1a344dd0..5cde3d7ef 100644 --- a/validators/headerFields.js +++ b/validators/headerFields.js @@ -34,16 +34,14 @@ var headerFields = function headerFields(headers) { } } - for (let file in allIssues39Dict) { - if (allIssues39Dict.hasOwnProperty(file)) { - const firstIssue = allIssues39Dict[file][0] - let evidence = '' - for (var issue of allIssues39Dict[file]) { - evidence = evidence + ' ' + allIssues39Dict[file][issue].reason - } - firstIssue.reason = evidence - finalIssues.push(firstIssue) + for (let file of Object.keys(allIssues39Dict)) { + const firstIssue = allIssues39Dict[file][0] + let evidence = '' + for (var issue of allIssues39Dict[file]) { + evidence = evidence + ' ' + issue.reason } + firstIssue.reason = evidence + finalIssues.push(firstIssue) } return finalIssues From 0453d3fdc2ab7712ba3214bae67b21fbc380f600 Mon Sep 17 00:00:00 2001 From: Teal Hobson-Lowther Date: Thu, 4 Oct 2018 10:38:58 -0700 Subject: [PATCH 2/4] abyssmal function declarations removed --- validators/headerFields.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/validators/headerFields.js b/validators/headerFields.js index 5cde3d7ef..07d6d3aac 100644 --- a/validators/headerFields.js +++ b/validators/headerFields.js @@ -12,7 +12,7 @@ var Issue = utils.issues.Issue * arrays more arguments will need to be added to headerField. */ -var headerFields = function headerFields(headers) { +const headerFields = headers => { var finalIssues = [] var allIssues39Dict = {} var fields = ['dim', 'pixdim'] @@ -56,7 +56,7 @@ var headerFields = function headerFields(headers) { * dimensionality of similar anatomy/functional/dwi headers are being compared. */ -var headerField = function headerField(headers, field) { +const headerField = (headers, field) => { var nifti_types = {} var issues = {} for (var header_index = 0; header_index < headers.length; header_index++) { @@ -247,7 +247,7 @@ var headerField = function headerField(headers, field) { * errors that cause resolutions to be slightly different. Returns true if * the two headers are signifigantly different */ -function headerFieldCompare(header1, header2) { +const headerFieldCompare = (header1, header2) => { var hdr1 = header1.split(',') var hdr2 = header2.split(',') if (hdr1.length !== hdr2.length) { From fb28fc6363b618c0e96f12418a4fa8d75f6bd30d Mon Sep 17 00:00:00 2001 From: Teal Hobson-Lowther Date: Thu, 4 Oct 2018 10:42:45 -0700 Subject: [PATCH 3/4] move issue 39 collection into testable function --- validators/headerFields.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/validators/headerFields.js b/validators/headerFields.js index 07d6d3aac..ea89f7da0 100644 --- a/validators/headerFields.js +++ b/validators/headerFields.js @@ -34,6 +34,13 @@ const headerFields = headers => { } } + finalIssues = finalIssues.concat(collect39Issues(allIssues39Dict)) + + return finalIssues +} + +const collect39Issues = allIssues39Dict => { + const finalIssues = [] for (let file of Object.keys(allIssues39Dict)) { const firstIssue = allIssues39Dict[file][0] let evidence = '' @@ -43,7 +50,6 @@ const headerFields = headers => { firstIssue.reason = evidence finalIssues.push(firstIssue) } - return finalIssues } From 661ee6b51d7cb6a2d8a94ada0a3affa7e44c0afd Mon Sep 17 00:00:00 2001 From: Teal Hobson-Lowther Date: Thu, 4 Oct 2018 11:11:17 -0700 Subject: [PATCH 4/4] unit tests for collect39Issues --- validators/__tests__/headerFields.spec.js | 65 +++++++++++++++++++++++ validators/headerFields.js | 1 + 2 files changed, 66 insertions(+) create mode 100644 validators/__tests__/headerFields.spec.js diff --git a/validators/__tests__/headerFields.spec.js b/validators/__tests__/headerFields.spec.js new file mode 100644 index 000000000..23a18b686 --- /dev/null +++ b/validators/__tests__/headerFields.spec.js @@ -0,0 +1,65 @@ +const assert = require('chai').assert +const { collect39Issues } = require('../headerFields') + +describe('headerFields', () => { + describe('collect39Issues()', () => { + it('should return an empty array if there are no files in allIssues39Dict', () => { + const allIssues39Dict = {} + const issues = collect39Issues(allIssues39Dict) + assert.isArray(issues) + assert.lengthOf(issues, 0) + }) + it('should return one issue per file in the allIssues39Dict', () => { + const allIssues39Dict = { + file_1: [ + { + code: 39, + reason: 'for some reason', + }, + { + code: 39, + reason: 'for some other reason', + }, + ], + } + const issues = collect39Issues(allIssues39Dict) + assert.isArray(issues) + assert.lengthOf(issues, 1) + }) + it('should return one issue for each file with code 39 issues', () => { + const allIssues39Dict = { + file_1: [ + { + code: 39, + reason: 'reason1', + }, + ], + file_2: [ + { + code: 39, + reason: 'reason2', + }, + ], + } + const issues = collect39Issues(allIssues39Dict) + assert.lengthOf(issues, 2) + }) + it('constructs a combined reason string from each issue.reason of a file', () => { + const allIssues39Dict = { + file_1: [ + { + code: 39, + reason: 'reason1', + }, + { + code: 39, + reason: 'reason2', + }, + ], + } + const issues = collect39Issues(allIssues39Dict) + assert.lengthOf(issues, 1) + assert(issues[0].reason == ' reason1 reason2') + }) + }) +}) diff --git a/validators/headerFields.js b/validators/headerFields.js index ea89f7da0..9dd532996 100644 --- a/validators/headerFields.js +++ b/validators/headerFields.js @@ -276,3 +276,4 @@ const headerFieldCompare = (header1, header2) => { } module.exports = headerFields +module.exports.collect39Issues = collect39Issues