search

othiym23 / async-listener

polyfill version of the 0.11 version of the asyncListener API
https://www.npmjs.org/package/async-listener
BSD 2-Clause "Simplified" License
174 stars 52 forks source link

Add Node.js 8 to Travis CI test matrix #115

Closed watson closed 7 years ago

watson commented 7 years ago

The tests are currently failing on Node.js 8 and I'm not that familiar with the test suite yet, so I need some help reading the output and hopefully making the tests pass.

There's already a PR open that fixes some of the tests, but not all of them (#110).

If someone could explain how to interpret the error output from the tests, it would be awesome 😃

hayes commented 7 years ago

Looks good to me

watson commented 7 years ago

@hayes I probably didn't explain my self very well, but I wanted to fix the Node.js 8 bugs before merging this. I can see that you opened #118 to revert this, but it was closed instead of being merged?

I think it's better to revert so that other PR's don't have to spend time debugging a false positive.

hayes commented 7 years ago

@watson this was my bad, I misinterpreted the failures as being related to the same node bug that was affecting v7. I am hoping to just get the test fix for. Node 8 merged today, but if you think it would be better to revert I would be happy to do that. I still have a little digging to do too understand why the results are a little different

watson commented 7 years ago

Nah, I didn't expect a fix for 8 so soon, so if we are close to merging then let's just keep 8 in master