wip: fixing up search tests

Author: zkat

lib/search/all-package-metadata.js

@@ -152,9 +152,6 @@ function createEntryUpdateStream (staleness, latest, opts) {
   }
   return npmFetch(uri, opts).then(res => {
     log.silly('all-package-metadata', 'request stream opened, code:', res.statusCode)
-    // NOTE - The stream returned by `request` seems to be very persnickety
-    //        and this is almost a magic incantation to get it to work.
-    //        Modify how `res` is used here at your own risk.
     let entryStream = ms.pipeline.obj(
       res.body,
       JSONStream.parse('*', (pkg, key) => {
@@ -199,15 +196,12 @@ function extractUpdated (entryStream, label, opts) {
       log.silly('all-package-metadata', 'got first stream entry for', label, latest)
       entryStream.removeListener('error', onErr)
       entryStream.removeListener('end', onEnd)
-      // Because `.once()` unpauses the stream, we re-pause it after the first
-      // entry so we don't vomit entries into the void.
-      entryStream.pause()
       if (typeof latest === 'number') {
         // The extra pipeline is to return a stream that will implicitly unpause
         // after having an `.on('data')` listener attached, since using this
         // `data` event broke its initial state.
         resolve({
-          updateStream: entryStream,
+          updateStream: entryStream.pipe(ms.through.obj()),
           updatedLatest: latest
         })
       } else {

test/tap/all-package-metadata-cache-stream-unit.js

@@ -1,18 +1,20 @@
 'use strict'
 
 require('../common-tap.js')
-var test = require('tap').test
-var mkdirp = require('mkdirp')
-var rimraf = require('rimraf')
-var path = require('path')
-var ms = require('mississippi')
-var Tacks = require('tacks')
-var File = Tacks.File
 
-var _createCacheEntryStream = require('../../lib/search/all-package-metadata.js')._createCacheEntryStream
+const getStream = require('get-stream')
+const mkdirp = require('mkdirp')
+const path = require('path')
+const rimraf = require('rimraf')
+const Tacks = require('tacks')
+const {test} = require('tap')
 
-var PKG_DIR = path.resolve(__dirname, 'create-cache-entry-stream')
-var CACHE_DIR = path.resolve(PKG_DIR, 'cache')
+const {File} = Tacks
+
+const _createCacheEntryStream = require('../../lib/search/all-package-metadata.js')._createCacheEntryStream
+
+const PKG_DIR = path.resolve(__dirname, 'create-cache-entry-stream')
+const CACHE_DIR = path.resolve(PKG_DIR, 'cache')
 
 function setup () {
   mkdirp.sync(CACHE_DIR)
@@ -22,10 +24,10 @@ function cleanup () {
   rimraf.sync(PKG_DIR)
 }
 
-test('createCacheEntryStream basic', function (t) {
+test('createCacheEntryStream basic', t => {
   setup()
-  var cachePath = path.join(CACHE_DIR, '.cache.json')
-  var fixture = new Tacks(File({
+  const cachePath = path.join(CACHE_DIR, '.cache.json')
+  const fixture = new Tacks(File({
     '_updated': 1234,
     bar: {
       name: 'bar',
@@ -37,16 +39,13 @@ test('createCacheEntryStream basic', function (t) {
     }
   }))
   fixture.create(cachePath)
-  _createCacheEntryStream(cachePath, function (err, stream, latest) {
-    if (err) throw err
+  return _createCacheEntryStream(cachePath, {}).then(({
+    updateStream: stream,
+    updatedLatest: latest
+  }) => {
     t.equals(latest, 1234, '`latest` correctly extracted')
     t.ok(stream, 'returned a stream')
-    var results = []
-    stream.on('data', function (pkg) {
-      results.push(pkg)
-    })
-    ms.finished(stream, function (err) {
-      if (err) throw err
+    return getStream.array(stream).then(results => {
       t.deepEquals(results, [{
         name: 'bar',
         version: '1.0.0'
@@ -55,82 +54,54 @@ test('createCacheEntryStream basic', function (t) {
         version: '1.0.0'
       }])
       cleanup()
-      t.done()
     })
   })
 })
 
-test('createCacheEntryStream empty cache', function (t) {
+test('createCacheEntryStream empty cache', t => {
   setup()
-  var cachePath = path.join(CACHE_DIR, '.cache.json')
-  var fixture = new Tacks(File({}))
+  const cachePath = path.join(CACHE_DIR, '.cache.json')
+  const fixture = new Tacks(File({}))
   fixture.create(cachePath)
-  _createCacheEntryStream(cachePath, function (err, stream, latest) {
-    t.ok(err, 'returned an error because there was no _updated')
-    t.match(err.message, /Empty or invalid stream/, 'useful error message')
-    t.notOk(stream, 'no stream returned')
-    t.notOk(latest, 'no latest returned')
-    cleanup()
-    t.done()
-  })
+  return _createCacheEntryStream(cachePath, {}).then(
+    () => { throw new Error('should not succeed') },
+    err => {
+      t.ok(err, 'returned an error because there was no _updated')
+      t.match(err.message, /Empty or invalid stream/, 'useful error message')
+      cleanup()
+    }
+  )
 })
 
-test('createCacheEntryStream no entry cache', function (t) {
+test('createCacheEntryStream no entry cache', t => {
   setup()
-  var cachePath = path.join(CACHE_DIR, '.cache.json')
-  var fixture = new Tacks(File({
+  const cachePath = path.join(CACHE_DIR, '.cache.json')
+  const fixture = new Tacks(File({
     '_updated': 1234
   }))
   fixture.create(cachePath)
-  _createCacheEntryStream(cachePath, function (err, stream, latest) {
-    if (err) throw err
+  return _createCacheEntryStream(cachePath, {}).then(({
+    updateStream: stream,
+    updatedLatest: latest
+  }) => {
     t.equals(latest, 1234, '`latest` correctly extracted')
     t.ok(stream, 'returned a stream')
-    var results = []
-    stream.on('data', function (pkg) {
-      results.push(pkg)
-    })
-    ms.finished(stream, function (err) {
-      if (err) throw err
+    return getStream.array(stream).then(results => {
       t.deepEquals(results, [], 'no results')
       cleanup()
-      t.done()
     })
   })
 })
 
-test('createCacheEntryStream missing cache', function (t) {
+test('createCacheEntryStream missing cache', t => {
   setup()
-  var cachePath = path.join(CACHE_DIR, '.cache.json')
-  _createCacheEntryStream(cachePath, function (err, stream, latest) {
-    t.ok(err, 'returned an error because there was no cache')
-    t.equals(err.code, 'ENOENT', 'useful error message')
-    t.notOk(stream, 'no stream returned')
-    t.notOk(latest, 'no latest returned')
-    cleanup()
-    t.done()
-  })
-})
-
-test('createCacheEntryStream bad syntax', function (t) {
-  setup()
-  var cachePath = path.join(CACHE_DIR, '.cache.json')
-  var fixture = new Tacks(File('{"_updated": 1234, uh oh'))
-  fixture.create(cachePath)
-  _createCacheEntryStream(cachePath, function (err, stream, latest) {
-    if (err) throw err
-    t.equals(latest, 1234, '`latest` correctly extracted')
-    t.ok(stream, 'returned a stream')
-    var results = []
-    stream.on('data', function (pkg) {
-      results.push(pkg)
-    })
-    ms.finished(stream, function (err) {
-      t.ok(err, 'stream errored')
-      t.match(err.message, /Invalid JSON/i, 'explains there\'s a syntax error')
-      t.deepEquals(results, [], 'no results')
+  const cachePath = path.join(CACHE_DIR, '.cache.json')
+  return _createCacheEntryStream(cachePath, {}).then(
+    () => { throw new Error('should not succeed') },
+    err => {
+      t.ok(err, 'returned an error because there was no cache')
+      t.equals(err.code, 'ENOENT', 'useful error message')
       cleanup()
-      t.done()
-    })
-  })
+    }
+  )
 })

test/tap/all-package-metadata-entry-stream-unit.js

@@ -1,23 +1,25 @@
 'use strict'
 
-var common = require('../common-tap.js')
-var npm = require('../../')
-var test = require('tap').test
-var mkdirp = require('mkdirp')
-var rimraf = require('rimraf')
-var path = require('path')
-var mr = require('npm-registry-mock')
-var ms = require('mississippi')
-var Tacks = require('tacks')
-var File = Tacks.File
+const common = require('../common-tap.js')
+const getStream = require('get-stream')
+const mkdirp = require('mkdirp')
+const mr = require('npm-registry-mock')
+const ms = require('mississippi')
+const npm = require('../../')
+const path = require('path')
+const rimraf = require('rimraf')
+const Tacks = require('tacks')
+const test = require('tap').test
 
-var _createEntryStream = require('../../lib/search/all-package-metadata.js')._createEntryStream
+const {File} = Tacks
 
-var ALL = common.registry + '/-/all'
-var PKG_DIR = path.resolve(__dirname, 'create-entry-update-stream')
-var CACHE_DIR = path.resolve(PKG_DIR, 'cache')
+const _createEntryStream = require('../../lib/search/all-package-metadata.js')._createEntryStream
 
-var server
+const ALL = common.registry + '/-/all'
+const PKG_DIR = path.resolve(__dirname, 'create-entry-update-stream')
+const CACHE_DIR = path.resolve(PKG_DIR, 'cache')
+
+let server
 
 function setup () {
   mkdirp.sync(CACHE_DIR)
@@ -27,10 +29,11 @@ function cleanup () {
   rimraf.sync(PKG_DIR)
 }
 
-test('setup', function (t) {
-  mr({port: common.port, throwOnUnmatched: true}, function (err, s) {
+test('setup', t => {
+  cleanup()
+  mr({port: common.port, throwOnUnmatched: true}, (err, s) => {
     t.ifError(err, 'registry mocked successfully')
-    npm.load({ cache: CACHE_DIR, registry: common.registry }, function (err) {
+    npm.load({ cache: CACHE_DIR, registry: common.registry }, err => {
       t.ifError(err, 'npm loaded successfully')
       server = s
       t.pass('all set up')
@@ -39,48 +42,45 @@ test('setup', function (t) {
   })
 })
 
-test('createEntryStream full request', function (t) {
+test('createEntryStream full request', t => {
   setup()
-  var cachePath = path.join(CACHE_DIR, '.cache.json')
-  var dataTime = +(new Date())
+  const cachePath = path.join(CACHE_DIR, '.cache.json')
+  const dataTime = +(new Date())
   server.get('/-/all').once().reply(200, {
     '_updated': dataTime,
     'bar': { name: 'bar', version: '1.0.0' },
     'foo': { name: 'foo', version: '1.0.0' }
   }, {
     date: 1234 // should never be used.
   })
-  _createEntryStream(cachePath, ALL, {}, 600, function (err, stream, latest, newEntries) {
-    if (err) throw err
+  return _createEntryStream(cachePath, 600, {}).then(({
+    entryStream: stream,
+    latest,
+    newEntries
+  }) => {
     t.equals(latest, dataTime, '`latest` correctly extracted')
     t.ok(newEntries, 'new entries need to be written to cache')
     t.ok(stream, 'returned a stream')
-    var results = []
-    stream.on('data', function (pkg) {
-      results.push(pkg)
-    })
-    ms.finished(stream, function (err) {
-      if (err) throw err
-      t.deepEquals(results, [{
-        name: 'bar',
-        version: '1.0.0'
-      }, {
-        name: 'foo',
-        version: '1.0.0'
-      }])
-      server.done()
-      cleanup()
-      t.end()
-    })
+    return getStream.array(stream)
+  }).then(results => {
+    t.deepEquals(results, [{
+      name: 'bar',
+      version: '1.0.0'
+    }, {
+      name: 'foo',
+      version: '1.0.0'
+    }])
+    server.done()
+    cleanup()
   })
 })
 
 test('createEntryStream cache only', function (t) {
   setup()
-  var now = Date.now()
-  var cacheTime = now - 100000
-  var cachePath = path.join(CACHE_DIR, '.cache.json')
-  var fixture = new Tacks(File({
+  const now = Date.now()
+  const cacheTime = now - 100000
+  const cachePath = path.join(CACHE_DIR, '.cache.json')
+  const fixture = new Tacks(File({
     '_updated': cacheTime,
     bar: { name: 'bar', version: '1.0.0' },
     cool: { name: 'cool', version: '1.0.0' },
@@ -113,17 +113,17 @@ test('createEntryStream cache only', function (t) {
 
 test('createEntryStream merged stream', function (t) {
   setup()
-  var now = Date.now()
-  var cacheTime = now - 6000000
+  const now = Date.now()
+  const cacheTime = now - 6000000
   server.get('/-/all/since?stale=update_after&startkey=' + cacheTime).once().reply(200, {
     'bar': { name: 'bar', version: '2.0.0' },
     'car': { name: 'car', version: '1.0.0' },
     'foo': { name: 'foo', version: '1.0.0' }
   }, {
     date: (new Date(now)).toISOString()
   })
-  var cachePath = path.join(CACHE_DIR, '.cache.json')
-  var fixture = new Tacks(File({
+  const cachePath = path.join(CACHE_DIR, '.cache.json')
+  const fixture = new Tacks(File({
     '_updated': cacheTime,
     bar: { name: 'bar', version: '1.0.0' },
     cool: { name: 'cool', version: '1.0.0' },
@@ -164,7 +164,7 @@ test('createEntryStream merged stream', function (t) {
 
 test('createEntryStream no sources', function (t) {
   setup()
-  var cachePath = path.join(CACHE_DIR, '.cache.json')
+  const cachePath = path.join(CACHE_DIR, '.cache.json')
   server.get('/-/all').once().reply(404, {})
   _createEntryStream(cachePath, ALL, {}, 600, function (err, stream, latest, newEntries) {
     t.ok(err, 'no sources, got an error')