Reorganized test and force 100% code coverage (#1226)

.github/workflows/nodejs.yml

@@ -32,9 +32,9 @@ jobs:
       run: |
         npm run test:unit
 
-    - name: Behavior test
+    - name: Acceptance test
       run: |
-        npm run test:behavior
+        npm run test:acceptance
 
     - name: Type Definitions
       run: |
@@ -121,9 +121,9 @@ jobs:
       run: |
         npm install
 
-    - name: Code coverage
+    - name: Code coverage report
       run: |
-        npm run test:coverage
+        npm run test:coverage-report
 
     - name: Upload coverage to Codecov
       uses: codecov/codecov-action@v1
@@ -131,6 +131,10 @@ jobs:
         file: ./coverage.lcov
         fail_ci_if_error: true
 
+    - name: Code coverage 100%
+      run: |
+        npm run test:coverage-100
+
   license:
     name: License check
     runs-on: ubuntu-latest

index.js

@@ -13,7 +13,7 @@ const Transport = require('./lib/Transport')
 const Connection = require('./lib/Connection')
 const { ConnectionPool, CloudConnectionPool } = require('./lib/pool')
 // Helpers works only in Node.js >= 10
-const Helpers = nodeMajor < 10 ? null : require('./lib/Helpers')
+const Helpers = nodeMajor < 10 ? /* istanbul ignore next */ null : require('./lib/Helpers')
 const Serializer = require('./lib/Serializer')
 const errors = require('./lib/errors')
 const { ConfigurationError } = errors
@@ -130,6 +130,7 @@ class Client extends EventEmitter {
       opaqueIdPrefix: options.opaqueIdPrefix
     })
 
+    /* istanbul ignore else */
     if (Helpers !== null) {
       this.helpers = new Helpers({ client: this, maxRetries: options.maxRetries })
     }
@@ -237,6 +238,7 @@ function getAuth (node) {
   return null
 
   function getUsernameAndPassword (node) {
+    /* istanbul ignore else */
     if (typeof node === 'string') {
       const { username, password } = new URL(node)
       return {

lib/Connection.js

@@ -20,7 +20,7 @@ const {
 } = require('./errors')
 
 class Connection {
-  constructor (opts = {}) {
+  constructor (opts) {
     this.url = opts.url
     this.ssl = opts.ssl || null
     this.id = opts.id || stripAuth(opts.url.href)
@@ -64,6 +64,7 @@ class Connection {
     // https://github.com/nodejs/node/commit/b961d9fd83
     if (INVALID_PATH_REGEX.test(requestParams.path) === true) {
       callback(new TypeError(`ERR_UNESCAPED_CHARACTERS: ${requestParams.path}`), null)
+      /* istanbul ignore next */
       return { abort: () => {} }
     }
 
@@ -73,6 +74,7 @@ class Connection {
     // listen for the response event
     // TODO: handle redirects?
     request.on('response', response => {
+      /* istanbul ignore else */
       if (ended === false) {
         ended = true
         this._openRequests--
@@ -87,6 +89,7 @@ class Connection {
 
     // handles request timeout
     request.on('timeout', () => {
+      /* istanbul ignore else */
       if (ended === false) {
         ended = true
         this._openRequests--
@@ -97,6 +100,7 @@ class Connection {
 
     // handles request error
     request.on('error', err => {
+      /* istanbul ignore else */
       if (ended === false) {
         ended = true
         this._openRequests--
@@ -107,6 +111,7 @@ class Connection {
     // updates the ended state
     request.on('abort', () => {
       debug('Request aborted', params)
+      /* istanbul ignore else */
       if (ended === false) {
         ended = true
         this._openRequests--
@@ -121,7 +126,7 @@ class Connection {
     if (isStream(params.body) === true) {
       pump(params.body, request, err => {
         /* istanbul ignore if  */
-        if (err != null && ended === false) {
+        if (err != null && /* istanbul ignore next */ ended === false) {
           ended = true
           this._openRequests--
           callback(err, null)
@@ -300,6 +305,7 @@ function resolve (host, path) {
 
 function prepareHeaders (headers = {}, auth) {
   if (auth != null && headers.authorization == null) {
+    /* istanbul ignore else */
     if (auth.apiKey) {
       if (typeof auth.apiKey === 'object') {
         headers.authorization = 'ApiKey ' + Buffer.from(`${auth.apiKey.id}:${auth.apiKey.api_key}`).toString('base64')

lib/Helpers.js

@@ -13,6 +13,7 @@ const { ResponseError, ConfigurationError } = require('./errors')
 const pImmediate = promisify(setImmediate)
 const sleep = promisify(setTimeout)
 const kClient = Symbol('elasticsearch-client')
+/* istanbul ignore next */
 const noop = () => {}
 
 class Helpers {
@@ -477,7 +478,7 @@ class Helpers {
         } else if (operation === 'update') {
           actionBody = serialize(action[0])
           payloadBody = typeof chunk === 'string'
-            ? `{doc:${chunk}}`
+            ? `{"doc":${chunk}}`
             : serialize({ doc: chunk, ...action[1] })
           chunkBytes += Buffer.byteLength(actionBody) + Buffer.byteLength(payloadBody)
           bulkBody.push(actionBody, payloadBody)
@@ -641,6 +642,7 @@ class Helpers {
               operation: deserialize(bulkBody[i]),
               document: operation !== 'delete'
                 ? deserialize(bulkBody[i + 1])
+                /* istanbul ignore next */
                 : null,
               retried: isRetrying
             })
@@ -672,6 +674,7 @@ class Helpers {
               // but the ES node were handling too many operations.
               if (status === 429) {
                 retry.push(bulkBody[indexSlice])
+                /* istanbul ignore next */
                 if (operation !== 'delete') {
                   retry.push(bulkBody[indexSlice + 1])
                 }

lib/Transport.js

@@ -22,7 +22,7 @@ const clientVersion = require('../package.json').version
 const userAgent = `elasticsearch-js/${clientVersion} (${os.platform()} ${os.release()}-${os.arch()}; Node.js ${process.version})`
 
 class Transport {
-  constructor (opts = {}) {
+  constructor (opts) {
     if (typeof opts.compression === 'string' && opts.compression !== 'gzip') {
       throw new ConfigurationError(`Invalid compression: '${opts.compression}'`)
     }
@@ -51,7 +51,6 @@ class Transport {
     } else if (opts.nodeSelector === 'round-robin') {
       this.nodeSelector = roundRobinSelector()
     } else if (opts.nodeSelector === 'random') {
-      /* istanbul ignore next */
       this.nodeSelector = randomSelector
     } else {
       this.nodeSelector = roundRobinSelector()
@@ -385,7 +384,7 @@ class Transport {
       }
 
       debug('Sniffing ended successfully', result.body)
-      const protocol = result.meta.connection.url.protocol || 'http:'
+      const protocol = result.meta.connection.url.protocol || /* istanbul ignore next */ 'http:'
       const hosts = this.connectionPool.nodesToHost(result.body.nodes, protocol)
       this.connectionPool.update(hosts)
 

lib/pool/BaseConnectionPool.js

@@ -52,6 +52,7 @@ class BaseConnectionPool {
     }
 
     if (opts.ssl == null) opts.ssl = this._ssl
+    /* istanbul ignore else */
     if (opts.agent == null) opts.agent = this._agent
 
     const connection = new this.Connection(opts)
@@ -201,6 +202,7 @@ class BaseConnectionPool {
       }
 
       address = address.slice(0, 4) === 'http'
+        /* istanbul ignore next */
         ? address
         : `${protocol}//${address}`
       const roles = node.roles.reduce((acc, role) => {

lib/pool/CloudConnectionPool.js

@@ -7,7 +7,7 @@
 const BaseConnectionPool = require('./BaseConnectionPool')
 
 class CloudConnectionPool extends BaseConnectionPool {
-  constructor (opts = {}) {
+  constructor (opts) {
     super(opts)
     this.cloudConnection = null
   }

lib/pool/ConnectionPool.js

@@ -11,7 +11,7 @@ const Connection = require('../Connection')
 const noop = () => {}
 
 class ConnectionPool extends BaseConnectionPool {
-  constructor (opts = {}) {
+  constructor (opts) {
     super(opts)
 
     this.dead = []

package.json

@@ -16,21 +16,19 @@
     "index"
   ],
   "scripts": {
-    "test": "npm run lint && npm run test:unit && npm run test:behavior && npm run test:types",
+    "test": "npm run lint && tap test/{unit,acceptance}/{*,**/*}.test.js && npm run test:types",
-    "test:node8": "npm run lint && tap test/unit/*.test.js -t 300 --no-coverage && npm run test:behavior && npm run test:types",
+    "test:node8": "npm run lint && tap test/{unit,acceptance}/*.test.js && npm run test:types",
-    "test:unit": "tap test/unit/*.test.js test/unit/**/*.test.js -t 300 --no-coverage",
+    "test:unit": "tap test/unit/{*,**/*}.test.js",
-    "test:behavior": "tap test/behavior/*.test.js -t 300 --no-coverage",
+    "test:acceptance": "tap test/acceptance/*.test.js",
     "test:integration": "node test/integration/index.js",
-    "test:integration:helpers": "tap test/integration/helpers/*.test.js --no-coverage -J",
+    "test:integration:helpers": "tap test/integration/helpers/*.test.js",
     "test:types": "tsd",
-    "test:coverage": "tap test/unit/*.test.js test/unit/**/*.test.js test/behavior/*.test.js -t 300 && nyc report --reporter=text-lcov > coverage.lcov",
+    "test:coverage-100": "tap test/{unit,acceptance}/{*,**/*}.test.js --coverage --100 --nyc-arg=\"--exclude=api\"",
-    "test:coverage-ui": "tap test/unit/*.test.js test/unit/**/*.test.js test/behavior/*.test.js -t 300 --coverage-report=html",
+    "test:coverage-report": "tap test/{unit,acceptance}/{*,**/*}.test.js --coverage --nyc-arg=\"--exclude=api\" && nyc report --reporter=text-lcov > coverage.lcov",
+    "test:coverage-ui": "tap test/{unit,acceptance}/{*,**/*}.test.js --coverage --coverage-report=html --nyc-arg=\"--exclude=api\"",
     "lint": "standard",
     "lint:fix": "standard --fix",
-    "ci": "npm run license-checker && npm test && npm run test:integration:helpers && npm run test:integration && npm run test:coverage",
+    "license-checker": "license-checker --production --onlyAllow='MIT;Apache-2.0;Apache1.1;ISC;BSD-3-Clause;BSD-2-Clause'"
-    "license-checker": "license-checker --production --onlyAllow='MIT;Apache-2.0;Apache1.1;ISC;BSD-3-Clause;BSD-2-Clause'",
-    "elasticsearch": "./scripts/es-docker.sh",
-    "elasticsearch:xpack": "./scripts/es-docker-platinum.sh"
   },
   "author": {
     "name": "Tomas Della Vedova",
@@ -86,5 +84,13 @@
   },
   "tsd": {
     "directory": "test/types"
+  },
+  "tap": {
+    "esm": false,
+    "ts": false,
+    "jsx": false,
+    "flow": false,
+    "coverage": false,
+    "jobs-auto": true
   }
 }

test/unit/client.test.js

@@ -1067,3 +1067,28 @@ test('Correctly handles the same header cased differently', t => {
     })
   })
 })
+
+test('Random selector', t => {
+  t.plan(2)
+
+  function handler (req, res) {
+    res.setHeader('Content-Type', 'application/json;utf=8')
+    res.end(JSON.stringify({ hello: 'world' }))
+  }
+
+  buildServer(handler, ({ port }, server) => {
+    const client = new Client({
+      node: `http://localhost:${port}`,
+      nodeSelector: 'random'
+    })
+
+    client.search({
+      index: 'test',
+      q: 'foo:bar'
+    }, (err, { body }) => {
+      t.error(err)
+      t.deepEqual(body, { hello: 'world' })
+      server.stop()
+    })
+  })
+})

test/unit/connection.test.js

@@ -617,6 +617,14 @@ test('Connection id should not contain credentials', t => {
   t.end()
 })
 
+test('Ipv6 support', t => {
+  const connection = new Connection({
+    url: new URL('http://[::1]:9200')
+  })
+  t.strictEqual(connection.buildRequestObject({}).hostname, '::1')
+  t.end()
+})
+
 test('Should throw if the protocol is not http or https', t => {
   try {
     new Connection({ // eslint-disable-line

test/unit/helpers/bulk.test.js

@@ -189,6 +189,51 @@ test('bulk index', t => {
       })
     })
 
+    t.test('refreshOnCompletion custom index', async t => {
+      let count = 0
+      const MockConnection = connection.buildMockConnection({
+        onRequest (params) {
+          if (params.method === 'GET') {
+            t.strictEqual(params.path, '/test/_refresh')
+            return { body: { acknowledged: true } }
+          } else {
+            t.strictEqual(params.path, '/_bulk')
+            t.match(params.headers, { 'content-type': 'application/x-ndjson' })
+            const [action, payload] = params.body.split('\n')
+            t.deepEqual(JSON.parse(action), { index: { _index: 'test' } })
+            t.deepEqual(JSON.parse(payload), dataset[count++])
+            return { body: { errors: false, items: [{}] } }
+          }
+        }
+      })
+
+      const client = new Client({
+        node: 'http://localhost:9200',
+        Connection: MockConnection
+      })
+      const result = await client.helpers.bulk({
+        datasource: dataset.slice(),
+        flushBytes: 1,
+        concurrency: 1,
+        refreshOnCompletion: 'test',
+        onDocument (doc) {
+          return {
+            index: { _index: 'test' }
+          }
+        }
+      })
+
+      t.type(result.time, 'number')
+      t.type(result.bytes, 'number')
+      t.match(result, {
+        total: 3,
+        successful: 3,
+        retry: 0,
+        failed: 0,
+        aborted: false
+      })
+    })
+
     t.test('Should perform a bulk request (custom action)', async t => {
       let count = 0
       const MockConnection = connection.buildMockConnection({
@@ -807,6 +852,53 @@ test('bulk update', t => {
       aborted: false
     })
   })
+
+  t.test('Should perform a bulk request dataset as string)', async t => {
+    let count = 0
+    const MockConnection = connection.buildMockConnection({
+      onRequest (params) {
+        t.strictEqual(params.path, '/_bulk')
+        t.match(params.headers, { 'content-type': 'application/x-ndjson' })
+        const [action, payload] = params.body.split('\n')
+        t.deepEqual(JSON.parse(action), { update: { _index: 'test', _id: count } })
+        t.deepEqual(JSON.parse(payload), { doc: dataset[count++] })
+        return { body: { errors: false, items: [{}] } }
+      }
+    })
+
+    const client = new Client({
+      node: 'http://localhost:9200',
+      Connection: MockConnection
+    })
+    let id = 0
+    const result = await client.helpers.bulk({
+      datasource: dataset.map(d => JSON.stringify(d)),
+      flushBytes: 1,
+      concurrency: 1,
+      onDocument (doc) {
+        return [{
+          update: {
+            _index: 'test',
+            _id: id++
+          }
+        }]
+      },
+      onDrop (doc) {
+        t.fail('This should never be called')
+      }
+    })
+
+    t.type(result.time, 'number')
+    t.type(result.bytes, 'number')
+    t.match(result, {
+      total: 3,
+      successful: 3,
+      retry: 0,
+      failed: 0,
+      aborted: false
+    })
+  })
+
   t.end()
 })
 
@@ -856,10 +948,6 @@ test('bulk delete', t => {
   })
 
   t.test('Should perform a bulk request (failure)', async t => {
-    if (semver.lt(process.versions.node, '10.0.0')) {
-      t.skip('This test will not pass on Node v8')
-      return
-    }
     async function handler (req, res) {
       t.strictEqual(req.url, '/_bulk')
       t.match(req.headers, { 'content-type': 'application/x-ndjson' })

test/unit/helpers/msearch.test.js

@@ -275,7 +275,7 @@ test('Stop a msearch processor (callbacks)', t => {
 })
 
 test('Bad header', t => {
-  t.plan(1)
+  t.plan(2)
 
   const MockConnection = connection.buildMockConnection({
     onRequest (params) {
@@ -294,11 +294,16 @@ test('Bad header', t => {
     t.strictEqual(err.message, 'The header should be an object')
   })
 
+  m.search(null, { query: { match: { foo: 'bar' } } })
+    .catch(err => {
+      t.strictEqual(err.message, 'The header should be an object')
+    })
+
   t.teardown(() => m.stop())
 })
 
 test('Bad body', t => {
-  t.plan(1)
+  t.plan(2)
 
   const MockConnection = connection.buildMockConnection({
     onRequest (params) {
@@ -317,6 +322,11 @@ test('Bad body', t => {
     t.strictEqual(err.message, 'The body should be an object')
   })
 
+  m.search({ index: 'test' }, null)
+    .catch(err => {
+      t.strictEqual(err.message, 'The body should be an object')
+    })
+
   t.teardown(() => m.stop())
 })