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: remove disabled test-dgram-send-error #12330

Closed
wants to merge 1 commit into from

Conversation

@Trott
Copy link
Member

commented Apr 11, 2017

This test was disabled in 2013 because it spams random IPs with UDP
messages. We've been doing fine for four years without so let's delete
it.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test dgram

test: remove disabled test-dgram-send-error
This test was disabled in 2013 because it spams random IPs with UDP
messages. We've been doing fine for four years without so let's delete
it.

@Trott Trott added dgram test labels Apr 11, 2017

@Trott

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2017

/cc @bnoordhuis

I suppose this test can be rewritten to be a better net citizen. But I'm not sure it's worth it. The code base has no doubt evolved considerably in the last four years.

I did spend a little time trying to rewrite it using IPv6 black-holing but couldn't get it to work.

If there's concern about deleting it, a compromise could be to delete it and open an issue in the issue tracker describing the desired test and labeling it help wanted. It's probably more likely to attract attention there anyway. We can even copy/paste the entire current code of the test.

@Trott

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2017

Trott added a commit to Trott/io.js that referenced this pull request Apr 13, 2017
test: remove disabled test-dgram-send-error
This test was disabled in 2013 because it spams random IPs with UDP
messages. We've been doing fine for four years without so let's delete
it.

PR-URL: nodejs#12330
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2017

Landed in 3900cf6

@Trott Trott closed this Apr 13, 2017

@jasnell jasnell referenced this pull request May 11, 2017
@gibfahn gibfahn referenced this pull request Jun 15, 2017
2 of 3 tasks complete
gibfahn added a commit that referenced this pull request Jun 18, 2017
test: remove disabled test-dgram-send-error
This test was disabled in 2013 because it spams random IPs with UDP
messages. We've been doing fine for four years without so let's delete
it.

PR-URL: #12330
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gibfahn added a commit that referenced this pull request Jun 20, 2017
test: remove disabled test-dgram-send-error
This test was disabled in 2013 because it spams random IPs with UDP
messages. We've been doing fine for four years without so let's delete
it.

PR-URL: #12330
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins added a commit that referenced this pull request Jul 11, 2017
test: remove disabled test-dgram-send-error
This test was disabled in 2013 because it spams random IPs with UDP
messages. We've been doing fine for four years without so let's delete
it.

PR-URL: #12330
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins referenced this pull request Jul 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.