Skip to content

Conversation

hayes
Copy link
Collaborator

@hayes hayes commented Jun 15, 2017

Tests have not been passing on node 8. This should get them passing again, but I I did not properly dig into whether or not changing the tests is the right thing, or if there is something that was broken in the instrumentation.

I suspect that changing the tests is safe here, but further investigation is probably required to ensure nothing was actually broken.

@hayes hayes force-pushed the hayes/fix-http-tests branch 2 times, most recently from 03f12c7 to 61113e7 Compare June 15, 2017 22:13
@watson
Copy link
Collaborator

watson commented Jun 16, 2017

As far as I know there's nothing wrong with the tests in Node 7 (except for the thing that's actually a bug in Node it self). Which things failed in Node 7?

@hayes
Copy link
Collaborator Author

hayes commented Jun 16, 2017

You are correct, I initially had.added an exception for that but in this PR before I really understood the issue, I should update the commit message and PR description

@hayes hayes changed the title fix http tests on node v7 & node 8 fix http tests on node 8 Jun 16, 2017
@watson
Copy link
Collaborator

watson commented Jun 16, 2017

Cool 👍 And thanks for taking the time to fix this 👊 I guess this then takes care of my concern in #115 (comment) if we can merge it soon

@hayes
Copy link
Collaborator Author

hayes commented Jun 16, 2017

Dug in a little more, and I am pretty confident everything is working correctly on node 8, so this should be good to merge.

@hayes hayes force-pushed the hayes/fix-http-tests branch from 61113e7 to 67ce620 Compare June 16, 2017 21:47
@hayes hayes merged commit 1e1aa3a into othiym23:master Jun 16, 2017
@hayes hayes deleted the hayes/fix-http-tests branch June 16, 2017 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants