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.

Sign up for GitHub

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

Jump to bottom

Investigate flaky test test-dgram-broadcast-multi-process #2472

Closed
joaocgreis opened this issue Aug 20, 2015 · 15 comments
Closed

Investigate flaky test test-dgram-broadcast-multi-process #2472

joaocgreis opened this issue Aug 20, 2015 · 15 comments
Labels
build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.

Comments

@joaocgreis
Copy link
Member

Examples of failures:
@joaocgreis joaocgreis added build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests. labels Aug 20, 2015
@joaocgreis
Copy link
Member Author

It seems that this test always fails on the platforms where it fails, and always passes where it passes.

@jbergstroem
Copy link
Member

The problem (at least on freebsd) is that it doesn't exit properly, making multiple other tests binding to 127.0.0.1:12346 fail.

Edit: s/problem/result of the problem/ to be clear.

@Trott
Copy link
Member

Trott commented Sep 14, 2015

FWIW, I believe this was fixed on the Raspberry Pi by #2808.

As for the FreeBSD issue:

  • It seems that the FreeBSD machines on CI don't treat 255.255.255.255 as a broadcast address or don't allow the node process access to that IP address via UDP. Is that even possible?
  • I believe this is the last issue before the tests in test/internet are all working and not flaky, which means that if we wanted to integrate them into CI, we could. (I believe the last time this came up, there were concerns about TLS tests hitting actual hosts on the Internet frequently enough to be bad net citizens. So there are other perhaps issues to address.)

@jbergstroem
Copy link
Member

@Trott: broadcast is set to something different because the runners inside a jail (we treat localhost slightly different as a result as well) -- 255.255.255.255 isn't reachable.

Do we perhaps want to pass broadcast address to the suite in a similar fashion as localhost?

@Trott
Copy link
Member

Trott commented Sep 14, 2015

@jbergstroem: Sounds like an idea worth trying to me. Alternative possibility if adding another environment variable to the FreeBSD config is frowned upon: If in a FreeBSD jail, get broadcast address by running ifconfig and grabbing the IP address that appears right after a case-sensitive match on broadcast.

@jbergstroem
Copy link
Member

@Trott jails are a bit special (some default values are routed 'magically'), so the granularity of passing through ENV is preferrable imo. I can PR.

@Trott
Copy link
Member

Trott commented Sep 14, 2015

@jbergstroem 👍

@jbergstroem
Copy link
Member

@Trott I'll update the freebsd buildbots to pass proper broadcast -- want me to file a new PR with above commit or is it perhaps part of something else I'm missing?

@Trott
Copy link
Member

Trott commented Sep 14, 2015

@jbergstroem New PR works for me. Thanks!

@jbergstroem jbergstroem mentioned this issue Sep 14, 2015
Closed
jbergstroem added a commit to jbergstroem/node that referenced this issue Sep 14, 2015
@jbergstroem
test: introduce a broadcast helper
48f0a61 
Introduce a helper - very similar to `common.localhostIPv4` that allows test
runner to override a broadcast address. This is required in certain
scenarios - like FreeBSD jails - since networking acts a bit different.

Refs: nodejs#2472
Trott pushed a commit to Trott/io.js that referenced this issue Sep 14, 2015
@jbergstroem @Trott
test: introduce a broadcast helper
beec671 
Introduce a helper - very similar to `common.localhostIPv4` that allows test
runner to override a broadcast address. This is required in certain
scenarios - like FreeBSD jails - since networking acts a bit different.

Refs: nodejs#2472
@jbergstroem
Copy link
Member

Can you rebase this and give it another go? The new freebsd102-64 bots should pass.

@Trott
Copy link
Member

Trott commented Nov 15, 2015

FYI, all FreeBSD buildbots continue to reliably fail this test. The freebsd102-64 passes the test-dgram-multicast-multi-process which the others fail, but all three fail test-dgram-broadcast-multi-process.js.

https://ci.nodejs.org/job/node-test-commit-freebsd/300/

@jbergstroem
Copy link
Member

We will retire the failing ones shortly. Not that it solves the problem with broadcasting and jails, but the issue here doesn't really lie with node or the test suite, rather how freebsd jails work.

@Trott
Copy link
Member

Trott commented Nov 15, 2015

Maybe we can insert code at the top of the test to skip it if common.isFreeBSD is true and insert some other thing here that would tell if you're running in a FreeBSD jail is also true?

@jbergstroem
Copy link
Member

@Trott lets do that then. We already have a helper for freebsd jail checks. Want to PR?

Trott added a commit to Trott/io.js that referenced this issue Nov 15, 2015
@Trott
test: skip test if FreeBSD jail will break it
546f947 
`test/internet/test-dgram-broadcast-multi-process.js` fails if it is in
a FreeBSD jail. This issue is with the way FreeBSD jails work and not
with Node. Skip the test if in a FreeBSD jail.

PR-URL: nodejs#3839
Fixes: nodejs#2472
@Trott
Copy link
Member

Trott commented Nov 15, 2015

PR: #3839

@Trott Trott mentioned this issue Nov 15, 2015
Closed
@Trott Trott closed this as completed in a2144fc Nov 16, 2015
Trott added a commit that referenced this issue Nov 17, 2015
@Trott @Fishrock123
test: skip test if FreeBSD jail will break it
4bb27ba 
`test/internet/test-dgram-broadcast-multi-process.js` fails if it is in
a FreeBSD jail. This issue is with the way FreeBSD jails work and not
with Node. Skip the test if in a FreeBSD jail.

PR-URL: #3839
Fixes: #2472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Trott added a commit that referenced this issue Nov 30, 2015
@Trott
test: skip test if FreeBSD jail will break it
d93921a 
`test/internet/test-dgram-broadcast-multi-process.js` fails if it is in
a FreeBSD jail. This issue is with the way FreeBSD jails work and not
with Node. Skip the test if in a FreeBSD jail.

PR-URL: #3839
Fixes: #2472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Trott added a commit that referenced this issue Dec 4, 2015
@Trott @rvagg
test: skip test if FreeBSD jail will break it
7b294bb 
`test/internet/test-dgram-broadcast-multi-process.js` fails if it is in
a FreeBSD jail. This issue is with the way FreeBSD jails work and not
with Node. Skip the test if in a FreeBSD jail.

PR-URL: #3839
Fixes: #2472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Trott added a commit that referenced this issue Dec 17, 2015
@Trott @jasnell
test: skip test if FreeBSD jail will break it
9ea6bc1 
`test/internet/test-dgram-broadcast-multi-process.js` fails if it is in
a FreeBSD jail. This issue is with the way FreeBSD jails work and not
with Node. Skip the test if in a FreeBSD jail.

PR-URL: #3839
Fixes: #2472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Trott added a commit that referenced this issue Dec 23, 2015
@Trott @jasnell
test: skip test if FreeBSD jail will break it
59016af 
`test/internet/test-dgram-broadcast-multi-process.js` fails if it is in
a FreeBSD jail. This issue is with the way FreeBSD jails work and not
with Node. Skip the test if in a FreeBSD jail.

PR-URL: #3839
Fixes: #2472
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@joaocgreis @jbergstroem @Trott