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

test: add tests for OutgoingMessage setTimeout #24148

Conversation

robin-drexler
Copy link
Contributor

These tests ensure that OutgoingMessage setTimeout method
will call setTimeout on its socket

Co-authored-by: ZauberNerd zaubernerd@zaubernerd.de

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

These tests ensure that OutgoingMessage setTimeout method
will call setTimeout on its socket

Co-authored-by: ZauberNerd <zaubernerd@zaubernerd.de>
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Nov 6, 2018
@addaleax
Copy link
Member

addaleax commented Nov 6, 2018

@addaleax addaleax added code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Nov 6, 2018
@robin-drexler
Copy link
Contributor Author

I think the CI failed due to reasons unrelated to our changes. I'd rebase against current master and see if CI gets green, however I think this would invalidate all existing reviews.

Any hints what to do? :)

@danbev
Copy link
Contributor

danbev commented Nov 7, 2018

Any hints what to do? :)

We can re-run the failing node-test-commit-smartos/ saving us from having to re-run the ones that succeeded.

@robin-drexler
Copy link
Contributor Author

@danbev I'm afraid I'm not allowed to do it. Can you?

@danbev
Copy link
Contributor

danbev commented Nov 7, 2018

I'm afraid I'm not allowed to do it. Can you?

Yep, sorry about not being clear on this. I restarted the build and you should be able to see the result here:
https://ci.nodejs.org/job/node-test-commit-smartos/21484/

@Trott
Copy link
Member

Trott commented Nov 8, 2018

Landed in 730ec83.

Thanks for the contribution! 🎉

(If you're interested in other possible contributions to Node.js but don't have a good idea of where to start looking, some ideas are posted at https://www.nodetodo.org/next-steps/.)

@Trott Trott closed this Nov 8, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Nov 8, 2018
These tests ensure that OutgoingMessage setTimeout method
will call setTimeout on its socket

Co-authored-by: ZauberNerd <zaubernerd@zaubernerd.de>

PR-URL: nodejs#24148
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@robin-drexler
Copy link
Contributor Author

Thanks for merging! :)

The altered commit seems to have removed @ZauberNerd as co-author (although it's still in the commit message).

Do you know if there's any way to get him his github attribution back?

@Trott
Copy link
Member

Trott commented Nov 9, 2018

Do you know if there's any way to get him his github attribution back?

I'm guessing that merging the Co-Author: trailer with our metadata will mess up our tooling, so I think it may not be simple to get the attribution back, I'm sorry to say... But I welcome correction from anyone more knowledgable than me on this stuff.

BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
These tests ensure that OutgoingMessage setTimeout method
will call setTimeout on its socket

Co-authored-by: ZauberNerd <zaubernerd@zaubernerd.de>

PR-URL: #24148
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
These tests ensure that OutgoingMessage setTimeout method
will call setTimeout on its socket

Co-authored-by: ZauberNerd <zaubernerd@zaubernerd.de>

PR-URL: nodejs#24148
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
codebytere pushed a commit that referenced this pull request Dec 13, 2018
These tests ensure that OutgoingMessage setTimeout method
will call setTimeout on its socket

Co-authored-by: ZauberNerd <zaubernerd@zaubernerd.de>

PR-URL: #24148
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 26, 2018
These tests ensure that OutgoingMessage setTimeout method
will call setTimeout on its socket

Co-authored-by: ZauberNerd <zaubernerd@zaubernerd.de>

PR-URL: #24148
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@codebytere codebytere mentioned this pull request Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants