From d35544cdf4861bfb79b04c89ee5ff6c8312317c5 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 17:21:16 -0400 Subject: [PATCH] Remove loops and many global checks from sanitiser tests Modifying these sanitiser tests became extremely hard because almost all of them were looping over lots of individual test cases, which places assumptions about the common behavior of potentialy very different test cases, as well as making assertions about huge swaths of output when only a small amount of that output was really under test. Hopefully these changes will make our tests easier to modify, and not really lose any ability to catch bugs. --- test/unit/sanitiser/_input.js | 4 +- test/unit/sanitiser/search.js | 91 +++++++++++++---------------------- 2 files changed, 36 insertions(+), 59 deletions(-) diff --git a/test/unit/sanitiser/_input.js b/test/unit/sanitiser/_input.js index 98eab084..378b8140 100644 --- a/test/unit/sanitiser/_input.js +++ b/test/unit/sanitiser/_input.js @@ -7,7 +7,7 @@ var input = require('../../../sanitiser/_input'), 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], nonAddressLayers = [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin' ], - defaultParsed= { target_layer: nonAddressLayers }, + defaultParsed= { }, defaultClean = { input: 'test', layers: allLayers, size: 10, @@ -26,4 +26,4 @@ module.exports = { defaultParsed: defaultParsed, defaultClean : defaultClean, getTargetLayers: getTargetLayers -}; \ No newline at end of file +}; diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 1f37fa13..f6c050cf 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -34,13 +34,10 @@ module.exports.tests.interface = function(test, common) { }); }; -module.exports.tests.sanitize_input = function(test, common) { - var inputs = { - invalid: [ '', 100, null, undefined, new Date() ], - valid: [ 'a', 'aa', 'aaaaaaaa' ] - }; +module.exports.tests.sanitize_invalid_input = function(test, common) { test('invalid input', function(t) { - inputs.invalid.forEach( function( input ){ + var invalid = [ '', 100, null, undefined, new Date() ]; + invalid.forEach( function( input ){ sanitize({ input: input }, function( err, clean ){ t.equal(err, 'invalid param \'input\': text length, must be >0', input + ' is an invalid input'); t.equal(clean, undefined, 'clean not set'); @@ -48,16 +45,26 @@ module.exports.tests.sanitize_input = function(test, common) { }); t.end(); }); - test('valid input', function(t) { - inputs.valid.forEach( function( input ){ - sanitize({ input: input }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.input = input; - expected.parsed_input.target_layer = _input.getTargetLayers(input); +}; - t.equal(err, undefined, 'no error'); - t.deepEqual(clean, expected, 'clean set correctly (' + input + ')'); - }); +module.exports.tests.sanitise_valid_input = function(test, common) { + test('valid short input', function(t) { + sanitize({ input: 'a' }, function( err, clean ){ + t.equal(err, undefined, 'no error'); + }); + t.end(); + }); + + test('valid not-quite-as-short input', function(t) { + sanitize({ input: 'aa' }, function( err, clean ){ + t.equal(err, undefined, 'no error'); + }); + t.end(); + }); + + test('valid longer input', function(t) { + sanitize({ input: 'aaaaaaaa' }, function( err, clean ){ + t.equal(err, undefined, 'no error'); }); t.end(); }); @@ -75,8 +82,6 @@ module.exports.tests.sanitize_input_with_delim = function(test, common) { expected.parsed_input = parser(input); t.equal(err, undefined, 'no error'); t.equal(clean.parsed_input.name, expected.parsed_input.name, 'clean name set correctly'); - t.equal(clean.parsed_input.admin_parts, expected.parsed_input.admin_parts, 'clean admin_parts set correctly'); - t.deepEqual(clean, expected, 'clean set correctly (' + input + ')'); }); }); @@ -103,10 +108,8 @@ module.exports.tests.sanitize_lat = function(test, common) { sanitize({ input: 'test', lat: lat, lon: 0 }, function( err, clean ){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lat = parseFloat( lat ); - expected.lon = 0; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (' + lat + ')'); + t.deepEqual(clean.lat, expected.lat, 'clean lat set correctly (' + lat + ')'); }); }); t.end(); @@ -122,10 +125,8 @@ module.exports.tests.sanitize_lon = function(test, common) { sanitize({ input: 'test', lat: 0, lon: lon }, function( err, clean ){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lon = parseFloat( lon ); - expected.lat = 0; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (' + lon + ')'); + t.deepEqual(clean.lon, expected.lon, 'clean set correctly (' + lon + ')'); }); }); t.end(); @@ -133,34 +134,27 @@ module.exports.tests.sanitize_lon = function(test, common) { }; module.exports.tests.sanitize_optional_geo = function(test, common) { - test('no lat/lon', function(t) { + test('no lat/lon', function(t) { sanitize({ input: 'test' }, function( err, clean ){ - var expected = defaultClean; t.equal(err, undefined, 'no error'); t.equal(clean.lat, undefined, 'clean set without lat'); t.equal(clean.lon, undefined, 'clean set without lon'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set without lat/lon'); }); t.end(); }); - test('no lat', function(t) { + test('no lat', function(t) { sanitize({ input: 'test', lon: 0 }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.lon = 0; + var expected_lon = 0; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (without any lat)'); + t.deepEqual(clean.lon, expected_lon, 'clean set correctly (without any lat)'); }); t.end(); }); - test('no lon', function(t) { + test('no lon', function(t) { sanitize({ input: 'test', lat: 0 }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.lat = 0; + var expected_lat = 0; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (without any lon)'); + t.deepEqual(clean.lat, expected_lat, 'clean set correctly (without any lon)'); }); t.end(); }); @@ -168,8 +162,6 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { module.exports.tests.sanitize_bbox = function(test, common) { var bboxes = { - invalid_coordinates: [ - ], invalid: [ '91;-181,-91,181', // invalid - semicolon between coordinates 'these,are,not,numbers', @@ -196,22 +188,11 @@ module.exports.tests.sanitize_bbox = function(test, common) { ] }; - test('invalid bbox coordinates', function(t) { - bboxes.invalid_coordinates.forEach( function( bbox ){ - sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ - t.ok(err.match(/Invalid (lat|lon)/), bbox + ' is invalid'); - t.equal(clean, undefined, 'clean not set'); - }); - }); - t.end(); - }); test('invalid bbox', function(t) { bboxes.invalid.forEach( function( bbox ){ sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'falling back on 50km distance from centroid'); + t.equal(clean.bbox, undefined, 'falling back on 50km distance from centroid'); }); }); t.end(); @@ -219,19 +200,17 @@ module.exports.tests.sanitize_bbox = function(test, common) { test('valid bbox', function(t) { bboxes.valid.forEach( function( bbox ){ sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); var bboxArray = bbox.split(',').map(function(i) { return parseInt(i); }); - expected.bbox = { + var expected_bbox = { right: Math.max(bboxArray[0], bboxArray[2]), top: Math.max(bboxArray[1], bboxArray[3]), left: Math.min(bboxArray[0], bboxArray[2]), bottom: Math.min(bboxArray[1], bboxArray[3]) }; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (' + bbox + ')'); + t.deepEqual(clean.bbox, expected_bbox, 'clean set correctly (' + bbox + ')'); }); }); t.end(); @@ -422,8 +401,6 @@ module.exports.tests.middleware_success = function(test, common) { var req = { query: { input: 'test' }}; var next = function( message ){ t.equal(message, undefined, 'no error message set'); - req.clean.parsed_input = parser('test'); - t.deepEqual(req.clean, defaultClean); t.end(); }; middleware( req, undefined, next );