search

mcollina / docker-loghose

Collect all the logs from all docker containers
MIT License
63 stars 15 forks source link

Test fails on node.js v6.x #14

Closed megastef closed 8 years ago

megastef commented 8 years ago

It seems that something in the buffer API did change, parser tests fail when with npm test e.g.:

9) The parser outputs the chunk wrapped in an object:
     Error: If encoding is specified then the first argument must be a string
      at new Buffer (buffer.js:106:13)
      at Object.module.exports.buildBuffer (test/helper.js:28:16)
      at Context.<anonymous> (test/parser/parser.js:25:44)
      at Context.<anonymous> (test/parser/parser.js:14:5)

@cammellos, could you pls. look into it? I think only the parser test is wrong, I had no issues with node version 6.x and docker-loghose.

Once this works, we could update .travis.yml for Node 6.

cammellos commented 8 years ago

Sure, I should be able to have a look at it this weeked, thanks!

On Thu, Jun 16, 2016 at 11:21 AM, Stefan Thies notifications@github.com wrote:

It seems that something in the buffer API did change, parser tests fail when with npm test e.g.:

9) The parser outputs the chunk wrapped in an object: Error: If encoding is specified then the first argument must be a string at new Buffer (buffer.js:106:13) at Object.module.exports.buildBuffer (test/helper.js:28:16) at Context. (test/parser/parser.js:25:44) at Context. (test/parser/parser.js:14:5)

@cammellos https://github.com/cammellos, could you pls. look into it? I think only the pareser test is wrong, I had no issues with node version 6.x and docker-loghose.

Once this works, we could update .travis.yml for Node 6.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mcollina/docker-loghose/issues/14, or mute the thread https://github.com/notifications/unsubscribe/AA-EsBSvqZnPXoFnfxzgWOpvu4v1DPGRks5qMSORgaJpZM4I3PO- .

megastef commented 8 years ago

Made it easy for you, tests passing on Node v6 with this change. Pls review: https://github.com/mcollina/docker-loghose/pull/13/commits/8be900d45c50313fb32d90fd289769b7f4475d35

megastef commented 8 years ago

Better look at https://github.com/mcollina/docker-loghose/pull/13/commits/6fe138465576d800b480c55c75fc74124125a279 :) the previous did fail on node 0.10. Part of this PR https://github.com/mcollina/docker-loghose/pull/13 - please comment there if anything is wrong with the changes in helper.js

megastef commented 8 years ago

Solved with https://github.com/mcollina/docker-loghose/pull/13