Repository: pinojs/pino
Pull request #445 (2018-07-08 18:42:02)

100% coverage


Patch:
diff --git a/lib/events.js b/lib/events.js
index 250864c..d1f5309 100644
--- a/lib/events.js
+++ b/lib/events.js
@@ -37,15 +37,10 @@ function events (instance) {
     function theWorldIsBurning (err) {
       if (theWorldIsBurning.called === true) return
       theWorldIsBurning.called = true
-      runInternalHandler()
-      handlers.handledOnTerminate = true
+      stream.flushSync()
       if (isExtreme()) instance[onTerminatedSym](this.name, err)
     }
 
-    function runInternalHandler () {
-      if (handlers.handledOnTerminate) return
-      stream.flushSync()
-    }
     function isExtreme () {
       const stream = instance[streamSym]
       return stream instanceof SonicBoom &&
diff --git a/lib/levels.js b/lib/levels.js
index 565ca60..4a4f348 100644
--- a/lib/levels.js
+++ b/lib/levels.js
@@ -67,19 +67,17 @@ function getLevelVal () {
 }
 
 function setLevelVal (num) {
-  if (typeof num === 'string') return this[setLevelSym](num)
+  if (typeof num !== 'number') throw Error('levelVal must be a number')
 
   const { labels, values } = this.levels
 
-  if (this.emit) {
-    this.emit(
-      'level-change',
-      labels[num],
-      num,
-      labels[this[levelValSym]],
-      this[levelValSym]
-    )
-  }
+  this.emit(
+    'level-change',
+    labels[num],
+    num,
+    labels[this[levelValSym]],
+    this[levelValSym]
+  )
 
   this[levelValSym] = num
 
@@ -93,10 +91,10 @@ function setLevelVal (num) {
 }
 
 function setLevel (level) {
-  const { labels, values } = this.levels
+  const { values } = this.levels
   if (typeof level === 'number') {
-    if (isFinite(level) === false) throw Error('unknown level ' + level)
-    level = labels[level]
+    this.levelVal = level
+    return
   }
   if (values[level] === undefined) throw Error('unknown level ' + level)
   this.levelVal = values[level]
diff --git a/lib/tools.js b/lib/tools.js
index e485c67..944e4d0 100644
--- a/lib/tools.js
+++ b/lib/tools.js
@@ -133,21 +133,18 @@ function asChindings (instance, bindings) {
 
 function getPrettyStream (opts, prettifier, dest) {
   if (prettifier && typeof prettifier === 'function') {
-    return asMetaWrapper(prettifier(opts), dest)
+    return prettifierMetaWrapper(prettifier(opts), dest)
   }
   try {
     var prettyFactory = require('pino-pretty')
-    prettyFactory.asMetaWrapper = asMetaWrapper
-    return asMetaWrapper(prettyFactory(opts), dest)
+    prettyFactory.asMetaWrapper = prettifierMetaWrapper
+    return prettifierMetaWrapper(prettyFactory(opts), dest)
   } catch (e) {
     throw Error('Missing `pino-pretty` module: `pino-pretty` must be installed separately')
   }
 }
 
-function asMetaWrapper (pretty, dest) {
-  if (!dest) dest = process.stdout
-  else if (!dest.write) throw new Error('the destination must be writable')
-
+function prettifierMetaWrapper (pretty, dest) {
   return {
     [needsMetadataGsym]: true,
     lastLevel: 0,
diff --git a/package.json b/package.json
index 2cb2851..6578fb3 100644
--- a/package.json
+++ b/package.json
@@ -19,6 +19,7 @@
     "browser-test": "airtap --local 8080 test/browser*test.js",
     "test": "standard | snazzy && NODE_OPTIONS='--no-warnings -r qodaa' tap -j 4 --no-cov test/*test.js",
     "ci": "standard | snazzy && TAP_TIMEOUT=480000 NODE_OPTIONS='--no-warnings -r qodaa' tap -j 4 --cov test/*test.js",
+    "cov-ui": "NODE_OPTIONS='--no-warnings -r qodaa' tap -j 4 --coverage-report=html test/*test.js",
     "bench": "node benchmarks/utils/runbench all",
     "bench-basic": "node benchmarks/utils/runbench basic",
     "bench-object": "node benchmarks/utils/runbench object",


Test patch:
diff --git a/test/add-level.test.js b/test/add-level.test.js
index dc0e789..dd707c6 100644
--- a/test/add-level.test.js
+++ b/test/add-level.test.js
@@ -4,7 +4,7 @@ const { test } = require('tap')
 const { sink, once } = require('./helper')
 const pino = require('../')
 
-test('can add a custom level via constructor', async ({is}) => {
+test('can add a custom level via exported pino function', async ({is}) => {
   const stream = sink()
   const instance = pino({level: 'foo', levelVal: 35}, stream)
   is(typeof instance.foo, 'function')
@@ -23,7 +23,7 @@ test('can add a custom level to a prior instance', async ({is}) => {
   is(msg, 'bar')
 })
 
-test('custom level via constructor does not affect other instances', async ({is}) => {
+test('custom level via exported pino function does not affect other instances', async ({is}) => {
   const instance = pino({level: 'foo3', levelVal: 36})
   const other = pino()
   is(typeof instance.foo3, 'function')
@@ -93,6 +93,9 @@ test('custom levels exists on children', async ({is}) => {
 test('rejects already known labels', async ({is}) => {
   const instance = pino({level: 'info', levelVal: 900})
   is(instance.levelVal, 30)
+  is(instance.addLevel('error', 200), false)
+  is(instance.levels.values.error, 50)
+  is(200 in instance.levels.labels, false)
 })
 
 test('reject already known values', async ({is}) => {
@@ -101,6 +104,10 @@ test('reject already known values', async ({is}) => {
   } catch (e) {
     is(e.message.indexOf('level value') > -1, true)
   }
+  const instance = pino()
+  is(instance.addLevel('foo', 50), false)
+  is(instance.levels.labels[50], 'error')
+  is('foo' in instance.levels.values, false)
 })
 
 test('reject values of Infinity', async ({throws}) => {
diff --git a/test/basic.test.js b/test/basic.test.js
index 2de09a2..dddc815 100644
--- a/test/basic.test.js
+++ b/test/basic.test.js
@@ -1,12 +1,15 @@
 'use strict'
 const os = require('os')
+const { join } = require('path')
+const { readFileSync } = require('fs')
+const proxyquire = require('proxyquire')
 const { test } = require('tap')
 const { sink, check, once } = require('./helper')
-const proxyquire = require('proxyquire')
 const pino = require('../')
 const { version } = require('../package.json')
 const { pid } = process
 const hostname = os.hostname()
+const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms))
 
 test('pino version is exposed on export', async ({is}) => {
   is(pino.version, version)
@@ -414,6 +417,25 @@ test('handles objects with null prototype', async ({same}) => {
   })
 })
 
+test('pino.destination', async ({same}) => {
+  const tmp = join(
+    os.tmpdir(),
+    '_' + Math.random().toString(36).substr(2, 9)
+  )
+  const instance = pino(pino.destination(tmp))
+  instance.info('hello')
+  await sleep(250)
+  const result = JSON.parse(readFileSync(tmp).toString())
+  delete result.time
+  same(result, {
+    pid: pid,
+    hostname: hostname,
+    level: 30,
+    msg: 'hello',
+    v: 1
+  })
+})
+
 // https://github.com/pinojs/pino/issues/222
 test('children with same names render in correct order', async ({is}) => {
   const stream = sink()
diff --git a/test/fixtures/pretty/child.js b/test/fixtures/pretty/child.js
new file mode 100644
index 0000000..cc1f289
--- /dev/null
+++ b/test/fixtures/pretty/child.js
@@ -0,0 +1,7 @@
+global.process = { __proto__: process, pid: 123456 }
+Date.now = function () { return 1459875739796 }
+require('os').hostname = function () { return 'abcdefghijklmnopqr' }
+var pino = require(require.resolve('./../../../'))
+var log = pino({prettyPrint: true}).child({a: 1})
+log.info('h')
+setTimeout(() => log.info('h2'), 200)
diff --git a/test/levels.test.js b/test/levels.test.js
index 4c2b1c9..6db4b1d 100644
--- a/test/levels.test.js
+++ b/test/levels.test.js
@@ -91,7 +91,7 @@ test('child returns level integer', async ({is}) => {
   is(child.levelVal, 50)
 })
 
-test('set the level via constructor', async ({is}) => {
+test('set the level via exported pino function', async ({is}) => {
   const expected = [{
     level: 50,
     msg: 'this is an error'
@@ -178,6 +178,18 @@ test('silent is a noop', async ({fail}) => {
   instance['silent']('hello world')
 })
 
+test('set silent via Infinity', async ({fail}) => {
+  const instance = pino({
+    level: Infinity
+  }, sink((result, enc) => {
+    fail('no data should be logged')
+  }))
+
+  Object.keys(pino.levels.values).forEach((level) => {
+    instance[level]('hello world')
+  })
+})
+
 test('silent stays a noop after level changes', async ({is, isNot, fail}) => {
   const noop = require('../lib/tools').noop
   const instance = pino({
@@ -235,3 +247,35 @@ test('setting level in child', async ({is}) => {
   instance.error('this is an error')
   instance.fatal('this is fatal')
 })
+
+test('setting level by assigning a number to levelVal', async ({is}) => {
+  const instance = pino()
+  is(instance.levelVal, 30)
+  is(instance.level, 'info')
+  instance.levelVal = 50
+  is(instance.levelVal, 50)
+  is(instance.level, 'error')
+})
+
+test('setting level by assigning a number to level', async ({is}) => {
+  const instance = pino()
+  is(instance.levelVal, 30)
+  is(instance.level, 'info')
+  instance.level = 50
+  is(instance.levelVal, 50)
+  is(instance.level, 'error')
+})
+
+test('setting level by assigning a known label to level', async ({is}) => {
+  const instance = pino()
+  is(instance.levelVal, 30)
+  is(instance.level, 'info')
+  instance.level = 'error'
+  is(instance.levelVal, 50)
+  is(instance.level, 'error')
+})
+
+test('assigning a string to levelVal will throw', async ({throws}) => {
+  const instance = pino()
+  throws(() => { instance.levelVal = 'error' })
+})
diff --git a/test/pretty.test.js b/test/pretty.test.js
index 7f2b2cd..b998384 100644
--- a/test/pretty.test.js
+++ b/test/pretty.test.js
@@ -8,7 +8,7 @@ const writer = require('flush-write-stream')
 const { once } = require('./helper')
 const pino = require('../')
 
-test('can be enabled via constructor', async ({isNot}) => {
+test('can be enabled via exported pino function', async ({isNot}) => {
   var actual = ''
   const child = fork(join(__dirname, 'fixtures', 'pretty', 'basic.js'), {silent: true})
 
@@ -20,7 +20,7 @@ test('can be enabled via constructor', async ({isNot}) => {
   isNot(actual.match(/\(123456 on abcdefghijklmnopqr\): h/), null)
 })
 
-test('can be enabled via constructor with pretty configuration', async ({isNot}) => {
+test('can be enabled via exported pino function with pretty configuration', async ({isNot}) => {
   var actual = ''
   const child = fork(join(__dirname, 'fixtures', 'pretty', 'level-first.js'), {silent: true})
 
@@ -32,7 +32,7 @@ test('can be enabled via constructor with pretty configuration', async ({isNot})
   isNot(actual.match(/^INFO.*h/), null)
 })
 
-test('can be enabled via constructor with prettifier', async ({isNot}) => {
+test('can be enabled via exported pino function with prettifier', async ({isNot}) => {
   var actual = ''
   const child = fork(join(__dirname, 'fixtures', 'pretty', 'pretty-factory.js'), {silent: true})
 
@@ -45,8 +45,24 @@ test('can be enabled via constructor with prettifier', async ({isNot}) => {
   isNot(actual.match(/^INFO.*h/), null)
 })
 
-test('does not throw error when enabled with stream specified', async () => {
-  pino({prettyPrint: true}, process.stdout)
+test('does not throw error when enabled with stream specified', async ({doesNotThrow}) => {
+  doesNotThrow(() => pino({prettyPrint: true}, process.stdout))
+})
+
+test('throws when prettyPrint is true but pino-pretty module is not installed', async ({throws, is}) => {
+  // pino pretty *is* installed, and probably also cached, so rather than
+  // messing with the filesystem the simplest way to generate a not found
+  // error is to simulate it:
+  const prettyFactory = require('pino-pretty')
+  require.cache[require.resolve('pino-pretty')].exports = () => {
+    throw Error(`Cannot find module 'pino-pretty'`)
+  }
+  throws(() => pino({prettyPrint: true}))
+  try { pino({prettyPrint: true}) } catch ({message}) {
+    is(message, 'Missing `pino-pretty` module: `pino-pretty` must be installed separately')
+  }
+
+  require.cache[require.resolve('pino-pretty')].exports = prettyFactory
 })
 
 test('can send pretty print to custom stream', async ({is}) => {
@@ -77,3 +93,18 @@ test('ignores `undefined` from prettifier', async ({is}) => {
   await once(child, 'close')
   is(actual, '')
 })
+
+test('parses and outputs chindings', async ({is, isNot}) => {
+  var actual = ''
+  const child = fork(join(__dirname, 'fixtures', 'pretty', 'child.js'), {silent: true})
+
+  child.stdout.pipe(writer((s, enc, cb) => {
+    actual += s
+    cb()
+  }))
+  await once(child, 'close')
+  isNot(actual.match(/\(123456 on abcdefghijklmnopqr\): h/), null)
+  isNot(actual.match(/\(123456 on abcdefghijklmnopqr\): h2/), null)
+  isNot(actual.match(/a: 1/), null)
+  is(actual.match(/a: 1/g).length, 2)
+})
diff --git a/test/timestamp.test.js b/test/timestamp.test.js
index 8e942ae..0d594ea 100644
--- a/test/timestamp.test.js
+++ b/test/timestamp.test.js
@@ -33,7 +33,7 @@ test('inserts timestamp by default', async ({ok, is}) => {
   is(result.msg, 'foobar')
 })
 
-test('omits timestamp with option', async ({is}) => {
+test('omits timestamp when timestamp option is false', async ({is}) => {
   const stream = sink()
   const instance = pino({timestamp: false}, stream)
   instance.info('foobar')
@@ -42,6 +42,16 @@ test('omits timestamp with option', async ({is}) => {
   is(result.msg, 'foobar')
 })
 
+test('inserts timestamp when timestamp option is true', async ({ok, is}) => {
+  const stream = sink()
+  const instance = pino({timestamp: true}, stream)
+  instance.info('foobar')
+  const result = await once(stream, 'data')
+  is(result.hasOwnProperty('time'), true)
+  ok(new Date(result.time) <= new Date(), 'time is greater than timestamp')
+  is(result.msg, 'foobar')
+})
+
 test('child inserts timestamp by default', async ({ok, is}) => {
   const stream = sink()
   const logger = pino(stream)
@@ -62,3 +72,18 @@ test('child omits timestamp with option', async ({is}) => {
   is(result.hasOwnProperty('time'), false)
   is(result.msg, 'foobar')
 })
+
+test('pino.stdTimeFunctions.unixTime returns seconds based timestamps', async ({is}) => {
+  const opts = {
+    timestamp: pino.stdTimeFunctions.unixTime
+  }
+  const stream = sink()
+  const instance = pino(opts, stream)
+  const now = Date.now
+  Date.now = () => 1531069919686
+  instance.info('foobar')
+  const result = await once(stream, 'data')
+  is(result.hasOwnProperty('time'), true)
+  is(result.time, 1531069920)
+  Date.now = now
+})