Merge pull request #785 from pelias/refactor-service-tests

refactored search service tests to inline mock callbacks

controller/place.js

@@ -53,6 +53,12 @@ function setup( apiConfig, esclient ){
           return;
         }
 
+        // if execution has gotten this far then one of three things happened:
+        // - the request didn't time out
+        // - maxRetries has been hit so we're giving up
+        // - another error occurred
+        // in either case, handle the error or results
+
         // error handler
         if( err ){
           if (_.isObject(err) && err.message) {

controller/search.js

@@ -80,6 +80,12 @@ function setup( apiConfig, esclient, query ){
           return;
         }
 
+        // if execution has gotten this far then one of three things happened:
+        // - the request didn't time out
+        // - maxRetries has been hit so we're giving up
+        // - another error occurred
+        // in either case, handle the error or results
+
         // error handler
         if( err ){
           if (_.isObject(err) && err.message) {

service/search.js

@@ -30,7 +30,6 @@ function service( esclient, cmd, cb ){
     };
 
     if( data && data.hits && data.hits.total && Array.isArray(data.hits.hits)){
-
       docs = data.hits.hits.map( function( hit ){
 
         meta.scores.push(hit._score);

test/unit/mock/backend.js

@@ -1,91 +0,0 @@
-
-var responses = {};
-responses['client/search/ok/1'] = function( cmd, cb ){
-  return cb( undefined, searchEnvelope([{
-    _id: 'myid1',
-    _type: 'mytype1',
-    _score: 10,
-    matched_queries: ['query 1', 'query 2'],
-    _source: {
-      value: 1,
-      center_point: { lat: 100.1, lon: -50.5 },
-      name: { default: 'test name1' },
-      parent: { country: ['country1'], region: ['state1'], county: ['city1'] }
-    }
-  }, {
-    _id: 'myid2',
-    _type: 'mytype2',
-    _score: 20,
-    matched_queries: ['query 3'],
-    _source: {
-      value: 2,
-      center_point: { lat: 100.2, lon: -51.5 },
-      name: { default: 'test name2' },
-      parent: { country: ['country2'], region: ['state2'], county: ['city2'] }
-    }
-  }]));
-};
-responses['client/search/fail/1'] = function( cmd, cb ){
-  return cb( 'an elasticsearch error occurred' );
-};
-
-responses['client/mget/ok/1'] = function( cmd, cb ){
-  return cb( undefined, mgetEnvelope([{
-    _id: 'myid1',
-    _type: 'mytype1',
-    _score: 10,
-    found: true,
-    _source: {
-      value: 1,
-      center_point: { lat: 100.1, lon: -50.5 },
-      name: { default: 'test name1' },
-      parent: { country: ['country1'], region: ['state1'], county: ['city1'] }
-    }
-  }, {
-    _id: 'myid2',
-    _type: 'mytype2',
-    _score: 20,
-    found: true,
-    _source: {
-      value: 2,
-      center_point: { lat: 100.2, lon: -51.5 },
-      name: { default: 'test name2' },
-      parent: { country: ['country2'], region: ['state2'], county: ['city2'] }
-    }
-  }]));
-};
-responses['client/mget/fail/1'] = responses['client/search/fail/1'];
-
-function setup( key, cmdCb ){
-  function backend( a, b ){
-    return {
-      mget: function( cmd, cb ){
-        if( 'function' === typeof cmdCb ){ cmdCb( cmd ); }
-        return responses[key.indexOf('mget') === -1 ? 'client/mget/ok/1' : key].apply( this, arguments );
-      },
-      suggest: function( cmd, cb ){
-        if( 'function' === typeof cmdCb ){ cmdCb( cmd ); }
-        return responses[key].apply( this, arguments );
-      },
-      search: function( cmd, cb ){
-        if( 'function' === typeof cmdCb ){ cmdCb( cmd ); }
-        return responses[key].apply( this, arguments );
-      }
-    };
-  }
-  return backend();
-}
-
-function mgetEnvelope( options ){
-  return { docs: options };
-}
-
-function suggestEnvelope( options1, options2 ){
-  return { 0: [{ options: options1 }], 1: [{ options: options2 }]};
-}
-
-function searchEnvelope( options ){
-  return { hits: { total: options.length, hits: options } };
-}
-
-module.exports = setup;

test/unit/service/mget.js

@@ -1,13 +1,9 @@
-
-var service = require('../../../service/mget'),
-    mockBackend = require('../mock/backend');
-
 const proxyquire = require('proxyquire').noCallThru();
 
 module.exports.tests = {};
 
-module.exports.tests.interface = function(test, common) {
+module.exports.tests.interface = (test, common) => {
-  test('valid interface', function(t) {
+  test('valid interface', (t) => {
     var service = proxyquire('../../../service/mget', {
       'pelias-logger': {
         get: (section) => {
@@ -22,82 +18,243 @@ module.exports.tests.interface = function(test, common) {
   });
 };
 
-// functionally test service
+module.exports.tests.error_conditions = (test, common) => {
-module.exports.tests.functional_success = function(test, common) {
+  test('esclient.mget returning error should log and pass it on', (t) => {
-
+    const errorMessages = [];
-  var expected = [
+
-    {
+    const service = proxyquire('../../../service/mget', {
-      _id: 'myid1', _type: 'mytype1',
+      'pelias-logger': {
-      value: 1,
+        get: () => {
-      center_point: { lat: 100.1, lon: -50.5 },
+          return {
-      name: { default: 'test name1' },
+            error: (msg) => {
-      parent: { country: ['country1'], region: ['state1'], county: ['city1'] }
+              errorMessages.push(msg);
-    },
+            }
-    {
+          };
-      _id: 'myid2', _type: 'mytype2',
+        }
-      value: 2,
+      }
-      center_point: { lat: 100.2, lon: -51.5 },
-      name: { default: 'test name2' },
-      parent: { country: ['country2'], region: ['state2'], county: ['city2'] }
-    }
-  ];
-
-  test('valid query', function(t) {
-    var backend = mockBackend( 'client/mget/ok/1', function( cmd ){
-      t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'a' } ] } }, 'correct backend command');
     });
-    service( backend, [ { _id: 123, _index: 'pelias', _type: 'a' } ], function(err, data) {
+
-      t.true(Array.isArray(data), 'returns an array');
+    const expectedCmd = {
-      data.forEach(function(d) {
+      body: {
-        t.true(typeof d === 'object', 'valid object');
+        docs: 'this is the query'
-      });
+      }
-      t.deepEqual(data, expected, 'values correctly mapped');
+    };
+
+    const esclient = {
+      mget: (cmd, callback) => {
+        t.deepEquals(cmd, expectedCmd);
+
+        const err = 'this is an error';
+        const data = {
+          docs: [
+            {
+              found: true,
+              _id: 'doc id',
+              _type: 'doc type',
+              _source: {}
+            }
+          ]
+        };
+
+        callback('this is an error', data);
+
+      }
+    };
+
+    const next = (err, docs) => {
+      t.equals(err, 'this is an error', 'err should have been passed on');
+      t.equals(docs, undefined);
+
+      t.ok(errorMessages.find((msg) => {
+        return msg === `elasticsearch error ${err}`;
+      }));
       t.end();
-    });
+    };
-  });
+
+    service(esclient, 'this is the query', next);
 
+  });
 };
 
-// functionally test service
+module.exports.tests.success_conditions = (test, common) => {
-module.exports.tests.functional_failure = function(test, common) {
+  test('esclient.mget returning data.docs should filter and map', (t) => {
+    const errorMessages = [];
 
-  test('invalid query', function(t) {
+    const service = proxyquire('../../../service/mget', {
-    var invalid_queries = [
+      'pelias-logger': {
-      { _id: 123, _index: 'pelias' },
+        get: () => {
-      { _id: 123, _type: 'a' },
+          return {
-      { _index: 'pelias', _type: 'a' },
+            error: (msg) => {
-      { }
+              errorMessages.push(msg);
-    ];
+            }
-
+          };
-    var backend = mockBackend( 'client/mget/fail/1', function( cmd ){
+        }
-      t.notDeepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'a' } ] } }, 'incorrect backend command');
+      }
     });
-    invalid_queries.forEach(function(query) {
+
-      // mock out pelias-logger so we can assert what's being logged
+    const expectedCmd = {
-      var service = proxyquire('../../../service/mget', {
+      body: {
-        'pelias-logger': {
+        docs: 'this is the query'
-          get: () => {
+      }
-            return {
+    };
-              error: (msg) => {
+
-                t.equal(msg, 'elasticsearch error an elasticsearch error occurred');
+    const esclient = {
+      mget: (cmd, callback) => {
+        t.deepEquals(cmd, expectedCmd);
+
+        const data = {
+          docs: [
+            {
+              found: true,
+              _id: 'doc id 1',
+              _type: 'doc type 1',
+              _source: {
+                random_key: 'value 1'
               }
-            };
+            },
-          }
+            {
+              found: false,
+              _id: 'doc id 2',
+              _type: 'doc type 2',
+              _source: {}
+            },
+            {
+              found: true,
+              _id: 'doc id 3',
+              _type: 'doc type 3',
+              _source: {
+                random_key: 'value 3'
+              }
+            }
+          ]
+        };
+
+        callback(undefined, data);
+
+      }
+    };
+
+    const expectedDocs = [
+      {
+        _id: 'doc id 1',
+        _type: 'doc type 1',
+        random_key: 'value 1'
+      },
+      {
+        _id: 'doc id 3',
+        _type: 'doc type 3',
+        random_key: 'value 3'
+      }
+
+    ];
+
+    const next = (err, docs) => {
+      t.equals(err, null);
+      t.deepEquals(docs, expectedDocs);
+
+      t.equals(errorMessages.length, 0, 'no errors should have been logged');
+      t.end();
+    };
+
+    service(esclient, 'this is the query', next);
+
+  });
+
+  test('esclient.mget callback with falsy data should return empty array', (t) => {
+    const errorMessages = [];
+
+    const service = proxyquire('../../../service/mget', {
+      'pelias-logger': {
+        get: () => {
+          return {
+            error: (msg) => {
+              errorMessages.push(msg);
+            }
+          };
         }
+      }
+    });
+
+    const expectedCmd = {
+      body: {
+        docs: 'this is the query'
+      }
+    };
+
+    const esclient = {
+      mget: (cmd, callback) => {
+        t.deepEquals(cmd, expectedCmd);
+
+        callback(undefined, undefined);
+
+      }
+    };
+
+    const expectedDocs = [];
+
+    const next = (err, docs) => {
+      t.equals(err, null);
+      t.deepEquals(docs, expectedDocs);
+
+      t.equals(errorMessages.length, 0, 'no errors should have been logged');
+      t.end();
+    };
+
+    service(esclient, 'this is the query', next);
 
-      });
+  });
+
+  test('esclient.mget callback with non-array data.docs should return empty array', (t) => {
+    const errorMessages = [];
 
-      service( backend, [ query ], function(err, data) {
+    const service = proxyquire('../../../service/mget', {
-        t.equal(err, 'an elasticsearch error occurred','error passed to errorHandler');
+      'pelias-logger': {
-        t.equal(data, undefined, 'data is undefined');
+        get: () => {
-      });
+          return {
+            error: (msg) => {
+              errorMessages.push(msg);
+            }
+          };
+        }
+      }
     });
-    t.end();
+
+    const expectedCmd = {
+      body: {
+        docs: 'this is the query'
+      }
+    };
+
+    const esclient = {
+      mget: (cmd, callback) => {
+        t.deepEquals(cmd, expectedCmd);
+
+        const data = {
+          docs: 'this isn\'t an array'
+        };
+
+        callback(undefined, data);
+
+      }
+    };
+
+    const expectedDocs = [];
+
+    const next = (err, docs) => {
+      t.equals(err, null);
+      t.deepEquals(docs, expectedDocs);
+
+      t.equals(errorMessages.length, 0, 'no errors should have been logged');
+      t.end();
+    };
+
+    service(esclient, 'this is the query', next);
+
   });
 
 };
 
-module.exports.all = function (tape, common) {
+module.exports.all = (tape, common) => {
 
   function test(name, testFunction) {
     return tape('SERVICE /mget ' + name, testFunction);

test/unit/service/search.js

@@ -1,16 +1,10 @@
-
-var service = require('../../../service/search'),
-    mockBackend = require('../mock/backend');
-
 const proxyquire = require('proxyquire').noCallThru();
 
-var example_valid_es_query = { body: { a: 'b' }, index: 'pelias' };
-
 module.exports.tests = {};
 
-module.exports.tests.interface = function(test, common) {
+module.exports.tests.interface = (test, common) => {
-  test('valid interface', function(t) {
+  test('valid interface', (t) => {
-    var service = proxyquire('../../../service/mget', {
+    var service = proxyquire('../../../service/search', {
       'pelias-logger': {
         get: (section) => {
           t.equal(section, 'api');
@@ -24,89 +18,344 @@ module.exports.tests.interface = function(test, common) {
   });
 };
 
-// functionally test service
+module.exports.tests.error_conditions = (test, common) => {
-module.exports.tests.functional_success = function(test, common) {
+  test('esclient.search returning error should log and pass it on', (t) => {
-
+    const errorMessages = [];
-  var expected = [
+
-    {
+    const service = proxyquire('../../../service/search', {
-      _id: 'myid1', _type: 'mytype1',
+      'pelias-logger': {
-      _score: 10,
+        get: () => {
-      _matched_queries: ['query 1', 'query 2'],
+          return {
-      value: 1,
+            error: (msg) => {
-      center_point: { lat: 100.1, lon: -50.5 },
+              errorMessages.push(msg);
-      name: { default: 'test name1' },
+            }
-      parent: { country: ['country1'], region: ['state1'], county: ['city1'] }
+          };
-    },
+        }
-    {
+      }
-      _id: 'myid2', _type: 'mytype2',
-      _score: 20,
-      _matched_queries: ['query 3'],
-      value: 2,
-      center_point: { lat: 100.2, lon: -51.5 },
-      name: { default: 'test name2' },
-      parent: { country: ['country2'], region: ['state2'], county: ['city2'] }
-    }
-  ];
-
-  var expectedMeta = {
-    scores: [10, 20]
-  };
-
-  test('valid ES query', function(t) {
-    var backend = mockBackend( 'client/search/ok/1', function( cmd ){
-      t.deepEqual(cmd, example_valid_es_query, 'no change to the command');
     });
-    service( backend, example_valid_es_query, function(err, data, meta) {
+
-      t.true(Array.isArray(data), 'returns an array');
+    const esclient = {
-      data.forEach(function(d) {
+      search: (cmd, callback) => {
-        t.true(typeof d === 'object', 'valid object');
+        t.deepEquals(cmd, 'this is the query');
-      });
+
-      t.deepEqual(data, expected, 'values correctly mapped');
+        const err = 'this is an error';
-      t.deepEqual(meta, expectedMeta, 'meta data correctly mapped');
+        const data = {
+          docs: [
+            {
+              found: true,
+              _id: 'doc id',
+              _type: 'doc type',
+              _source: {}
+            }
+          ]
+        };
+
+        callback('this is an error', data);
+
+      }
+    };
+
+    const next = (err, docs) => {
+      t.equals(err, 'this is an error', 'err should have been passed on');
+      t.equals(docs, undefined);
+
+      t.ok(errorMessages.find((msg) => {
+        return msg === `elasticsearch error ${err}`;
+      }));
       t.end();
-    });
+    };
-  });
 
+    service(esclient, 'this is the query', next);
+
+  });
 };
 
-// functionally test service
+module.exports.tests.success_conditions = (test, common) => {
-module.exports.tests.functional_failure = function(test, common) {
+  test('esclient.search returning data.docs should filter and map', (t) => {
+    const errorMessages = [];
+
+    const service = proxyquire('../../../service/search', {
+      'pelias-logger': {
+        get: () => {
+          return {
+            error: (msg) => {
+              errorMessages.push(msg);
+            }
+          };
+        }
+      }
+    });
+
+    const esclient = {
+      search: (cmd, callback) => {
+        t.deepEquals(cmd, 'this is the query');
+
+        const data = {
+          hits: {
+            total: 17,
+            hits: [
+              {
+                _score: 'score 1',
+                _id: 'doc id 1',
+                _type: 'doc type 1',
+                matched_queries: 'matched_queries 1',
+                _source: {
+                  random_key: 'value 1'
+                }
+              },
+              {
+                _score: 'score 2',
+                _id: 'doc id 2',
+                _type: 'doc type 2',
+                matched_queries: 'matched_queries 2',
+                _source: {
+                  random_key: 'value 2'
+                }
+              }
+            ]
+          }
+        };
+
+        callback(undefined, data);
+
+      }
+    };
 
-  test('invalid ES query', function(t) {
+    const expectedDocs = [
-    var invalid_queries = [
+      {
-      {  },
+        _score: 'score 1',
-      { foo: 'bar' }
+        _id: 'doc id 1',
+        _type: 'doc type 1',
+        random_key: 'value 1',
+        _matched_queries: 'matched_queries 1'
+      },
+      {
+        _score: 'score 2',
+        _id: 'doc id 2',
+        _type: 'doc type 2',
+        random_key: 'value 2',
+        _matched_queries: 'matched_queries 2'
+      }
     ];
 
-    var backend = mockBackend( 'client/search/fail/1', function( cmd ){
+    const expectedMeta = {
-      t.notDeepEqual(cmd, example_valid_es_query, 'incorrect backend command');
+      scores: ['score 1', 'score 2']
+    };
+
+    const next = (err, docs, meta) => {
+      t.equals(err, null);
+      t.deepEquals(docs, expectedDocs);
+      t.deepEquals(meta, expectedMeta);
+
+      t.equals(errorMessages.length, 0, 'no errors should have been logged');
+      t.end();
+    };
+
+    service(esclient, 'this is the query', next);
+
+  });
+
+  test('esclient.search returning falsy data should return empty docs and meta', (t) => {
+    const errorMessages = [];
+
+    const service = proxyquire('../../../service/search', {
+      'pelias-logger': {
+        get: () => {
+          return {
+            error: (msg) => {
+              errorMessages.push(msg);
+            }
+          };
+        }
+      }
     });
-    invalid_queries.forEach(function(query) {
+
-      // mock out pelias-logger so we can assert what's being logged
+    const esclient = {
-      var service = proxyquire('../../../service/search', {
+      search: (cmd, callback) => {
-        'pelias-logger': {
+        t.deepEquals(cmd, 'this is the query');
-          get: () => {
+
-            return {
+        callback(undefined, undefined);
-              error: (msg) => {
+
-                t.equal(msg, 'elasticsearch error an elasticsearch error occurred');
+      }
-              }
+    };
-            };
+
+    const expectedDocs = [];
+    const expectedMeta = { scores: [] };
+
+    const next = (err, docs, meta) => {
+      t.equals(err, null);
+      t.deepEquals(docs, expectedDocs);
+      t.deepEquals(meta, expectedMeta);
+
+      t.equals(errorMessages.length, 0, 'no errors should have been logged');
+      t.end();
+    };
+
+    service(esclient, 'this is the query', next);
+
+  });
+
+  test('esclient.search returning falsy data.hits should return empty docs and meta', (t) => {
+    const errorMessages = [];
+
+    const service = proxyquire('../../../service/search', {
+      'pelias-logger': {
+        get: () => {
+          return {
+            error: (msg) => {
+              errorMessages.push(msg);
+            }
+          };
+        }
+      }
+    });
+
+    const esclient = {
+      search: (cmd, callback) => {
+        t.deepEquals(cmd, 'this is the query');
+
+        const data = {
+          hits: {
+            total: 17
           }
+        };
+
+        callback(undefined, data);
+
+      }
+    };
+
+    const expectedDocs = [];
+    const expectedMeta = { scores: [] };
+
+    const next = (err, docs, meta) => {
+      t.equals(err, null);
+      t.deepEquals(docs, expectedDocs);
+      t.deepEquals(meta, expectedMeta);
+
+      t.equals(errorMessages.length, 0, 'no errors should have been logged');
+      t.end();
+    };
+
+    service(esclient, 'this is the query', next);
+
+  });
+
+  test('esclient.search returning falsy data.hits.total should return empty docs and meta', (t) => {
+    const errorMessages = [];
+
+    const service = proxyquire('../../../service/search', {
+      'pelias-logger': {
+        get: () => {
+          return {
+            error: (msg) => {
+              errorMessages.push(msg);
+            }
+          };
         }
+      }
+    });
+
+    const esclient = {
+      search: (cmd, callback) => {
+        t.deepEquals(cmd, 'this is the query');
+
+        const data = {
+          hits: {
+            hits: [
+              {
+                _score: 'score 1',
+                _id: 'doc id 1',
+                _type: 'doc type 1',
+                matched_queries: 'matched_queries 1',
+                _source: {
+                  random_key: 'value 1'
+                }
+              },
+              {
+                _score: 'score 2',
+                _id: 'doc id 2',
+                _type: 'doc type 2',
+                matched_queries: 'matched_queries 2',
+                _source: {
+                  random_key: 'value 2'
+                }
+              }
+            ]
+          }
+        };
 
-      });
+        callback(undefined, data);
 
-      service( backend, [ query ], function(err, data) {
+      }
-        t.equal(err, 'an elasticsearch error occurred','error passed to errorHandler');
+    };
-        t.equal(data, undefined, 'data is undefined');
+
-      });
+    const expectedDocs = [];
+    const expectedMeta = { scores: [] };
+
+    const next = (err, docs, meta) => {
+      t.equals(err, null);
+      t.deepEquals(docs, expectedDocs);
+      t.deepEquals(meta, expectedMeta);
+
+      t.equals(errorMessages.length, 0, 'no errors should have been logged');
+      t.end();
+    };
+
+    service(esclient, 'this is the query', next);
+
+  });
+
+  test('esclient.search returning non-array data.hits.hits should return empty docs and meta', (t) => {
+    const errorMessages = [];
+
+    const service = proxyquire('../../../service/search', {
+      'pelias-logger': {
+        get: () => {
+          return {
+            error: (msg) => {
+              errorMessages.push(msg);
+            }
+          };
+        }
+      }
     });
-    t.end();
+
+    const esclient = {
+      search: (cmd, callback) => {
+        t.deepEquals(cmd, 'this is the query');
+
+        const data = {
+          hits: {
+            total: 17,
+            hits: 'this isn\'t an array'
+          }
+        };
+
+        callback(undefined, data);
+
+      }
+    };
+
+    const expectedDocs = [];
+    const expectedMeta = { scores: [] };
+
+    const next = (err, docs, meta) => {
+      t.equals(err, null);
+      t.deepEquals(docs, expectedDocs);
+      t.deepEquals(meta, expectedMeta);
+
+      t.equals(errorMessages.length, 0, 'no errors should have been logged');
+      t.end();
+    };
+
+    service(esclient, 'this is the query', next);
+
   });
 
 };
 
-module.exports.all = function (tape, common) {
+module.exports.all = (tape, common) => {
 
   function test(name, testFunction) {
     return tape('SERVICE /search ' + name, testFunction);