Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update nock to v13 and use "delayConnection" instead of socketDelay #1303

Closed
obecny opened this issue Jul 10, 2020 · 11 comments
Closed

Update nock to v13 and use "delayConnection" instead of socketDelay #1303

obecny opened this issue Jul 10, 2020 · 11 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@obecny
Copy link
Member

obecny commented Jul 10, 2020

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:
#1298

@obecny obecny added good first issue Good for newcomers up-for-grabs Good for taking. Extra help will be provided by maintainers labels Jul 10, 2020
@sergioregueira
Copy link
Contributor

Please, assign this issue to me. I will submit the pull request before the end of the next week.

@obecny obecny removed the up-for-grabs Good for taking. Extra help will be provided by maintainers label Aug 14, 2020
@euniceek
Copy link

euniceek commented May 12, 2021

Hi, is this still up for grabs? And if so, could I pick this up?
cc @alolita

@obecny
Copy link
Member Author

obecny commented May 12, 2021

@eunice98k yes it is still valid, I'm assigning this to you, thank you

@obecny obecny assigned euniceek and unassigned sergioregueira May 12, 2021
@sergioregueira
Copy link
Contributor

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.

@euniceek
Copy link

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.

@sergioregueira
Copy link
Contributor

sergioregueira commented May 12, 2021

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 🤗

@euniceek
Copy link

This is super helpful. Thank you!
Will get working on it and ping you if I have any questions 😁

@cole-easton
Copy link
Contributor

Can I pick this up?

@vmarchaud
Copy link
Member

@cole-easton Do you know if @eunice98k is still working on this ? If not i don't see any issue

@spencerwilson
Copy link
Contributor

@dyladan should this be closed due to being fixed by #2652?

@legendecas
Copy link
Member

Fixed by #2652

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants