Nock in version 13 removed socketDelay in favour of delayConnection. So to upgrade to v13 we have to change some unit tests.
More details here:
https://github.com/open-telemetry/opentelemetry-js/pull/1298
Please, assign this issue to me. I will submit the pull request before the end of the next week.
Hi, is this still up for grabs? And if so, could I pick this up?
cc @alolita
@eunice98k yes it is still valid, I'm assigning this to you, thank you
I am sorry. I have been quite busy these months.
Let me update the issue today before the end of the day (UTC) to clarify why it is blocked.
I am sorry. I have been quite busy these months.
Let me update the issue today before the end of the day (UTC) to clarify why it is blocked.
Sounds good! Thank you.
My progress is available in this (very outdated) pull request: #1504. As its description says, the major release changed the expected behavior and some tests can no longer be reproduced.
To simulate several specific scenarios, I had to replace some setup blocks that were using nock with custom http servers. I was close to fixing them all when I ran into a bug in the code itself (not the tests).
I do not remember the cases properly, but they were related to the aborted event on request object and some specific versions of Node.js. In those days, the project was still using Circle CI and I had some problems that did not allow me to take a look at the errors.
I remember it as a tricky good first issue despite it seems very trivial at first sight. Fortunately, the project has evolved a lot since then. Anyway, I will have free time this month, so feel free to ping me in case you need something else :hugs:
This is super helpful. Thank you!
Will get working on it and ping you if I have any questions 馃榿
Can I pick this up?
@cole-easton Do you know if @eunice98k is still working on this ? If not i don't see any issue