Repository: pinojs/pino (js)

Issue:
a tap subtest is breaking all following subtests in file
Side-sprout (to keep in line with pino's natural theme) of issue #208: there is a dormant issue in one of pino's test suites, which can reproduced as follow:
* edit `test/levels.test.js` to move the test 'level-change even' anywhere else than at the bottom of the file
* run full test suite with `npm run test` or just the relevant test suite with: `./node_modules/.bin/tap test/levels.test.js`
* all subsequent tests to fail with:
```
    not ok XX - test after end() was called # time=7.746ms
      ---
      stack: |
        EventEmitter.handle (test/levels.test.js:12:7)
        EventEmitter.setLevelVal (pino.js:125:12)
        EventEmitter._setLevel (pino.js:163:17)
        EventEmitter.applyOptions (lib/tools.js:43:8)
        EventEmitter.Pino (pino.js:104:22)
        pino (pino.js:91:16)
        Test.<anonymous> (test/levels.test.js:52:18)
        loop (node_modules/function-loop/index.js:35:15)
      at:
```

Looks like the 'level-change even' test is able to damage pino's or tap's state permanently, meaning there is a bug somewhere.

Pull request:
Use Object.create to define Pino prototype (fix #209)
I'm really not sure how the test was passing previously. Occasionally `removeListener()` would work  but other times it wouldn't. Thus, tests that were meant to run once would run multiple times. I know that the previous prototype definition _should_ be equivalent, but it was clearly misbehaving.

Patch:
diff --git a/pino.js b/pino.js
index 98c4361..d6a29bb 100644
--- a/pino.js
+++ b/pino.js
@@ -102,7 +102,7 @@ function Pino (opts, stream) {
   tools.applyOptions.call(this, opts)
 }
 
-Pino.prototype = new EventEmitter()
+Pino.prototype = Object.create(EventEmitter.prototype)
 
 Pino.prototype.fatal = tools.genLog(levels.levels.fatal)
 Pino.prototype.error = tools.genLog(levels.levels.error)


Test patch:
diff --git a/test/levels.test.js b/test/levels.test.js
index 99904f8..6f57b2f 100644
--- a/test/levels.test.js
+++ b/test/levels.test.js
@@ -119,6 +119,40 @@ test('set the level via constructor', function (t) {
   instance.fatal('this is fatal')
 })
 
+test('level-change event', function (t) {
+  var instance = pino()
+  var handle = function (lvl, val, prevLvl, prevVal) {
+    t.is(lvl, 'trace')
+    t.is(val, 10)
+    t.is(prevLvl, 'info')
+    t.is(prevVal, 30)
+  }
+  instance.on('level-change', handle)
+  instance.level = 'trace'
+  instance.removeListener('level-change', handle)
+  instance.level = 'info'
+
+  var count = 0
+
+  var l1 = function () { count += 1 }
+  var l2 = function () { count += 1 }
+  var l3 = function () { count += 1 }
+  instance.on('level-change', l1)
+  instance.on('level-change', l2)
+  instance.on('level-change', l3)
+
+  instance.level = 'trace'
+  instance.removeListener('level-change', l3)
+  instance.level = 'fatal'
+  instance.removeListener('level-change', l1)
+  instance.level = 'debug'
+  instance.removeListener('level-change', l2)
+  instance.level = 'info'
+
+  t.is(count, 6)
+  t.end()
+})
+
 test('enable', function (t) {
   var instance = pino({
     level: 'trace',
@@ -219,37 +253,3 @@ test('setting level in child', function (t) {
   instance.error('this is an error')
   instance.fatal('this is fatal')
 })
-
-test('level-change event', function (t) {
-  var instance = pino()
-  var handle = function (lvl, val, prevLvl, prevVal) {
-    t.is(lvl, 'trace')
-    t.is(val, 10)
-    t.is(prevLvl, 'info')
-    t.is(prevVal, 30)
-  }
-  instance.on('level-change', handle)
-  instance.level = 'trace'
-  instance.removeListener('level-change', handle)
-  instance.level = 'info'
-
-  var count = 0
-
-  var l1 = function () { count += 1 }
-  var l2 = function () { count += 1 }
-  var l3 = function () { count += 1 }
-  instance.on('level-change', l1)
-  instance.on('level-change', l2)
-  instance.on('level-change', l3)
-
-  instance.level = 'trace'
-  instance.removeListener('level-change', l3)
-  instance.level = 'fatal'
-  instance.removeListener('level-change', l1)
-  instance.level = 'debug'
-  instance.removeListener('level-change', l2)
-  instance.level = 'info'
-
-  t.is(count, 6)
-  t.end()
-})