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: skip test if FreeBSD jail will break it #3839

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 15, 2015

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.

`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
@jbergstroem
Copy link
Member

Suggestion: Perhaps add a comment/reference to the issue(s) we've been having.

LGTM.

@Trott
Copy link
Member Author

Trott commented Nov 15, 2015

@jbergstroem I added it in the Fixes: metadata field in the commit message and force pushed. Does that work?

For reference (for anyone who doesn't want to switch to the commit message tab), it's #2472

@jbergstroem
Copy link
Member

All good.

@Trott
Copy link
Member Author

Trott commented Nov 15, 2015

CI: https://ci.nodejs.org/job/node-test-pull-request/740/

Unfortunately, that CI doesn't exercise the code change (other than linting it) because internet tests are skipped in CI. So here's a CI run for FreeBSD only where the Makefile was altered to run the internet tests: https://ci.nodejs.org/job/node-test-commit-freebsd/303/

@jbergstroem The results of that CI run lead me to believe that freebsd102-64 is not in a jail. Is that right? (Because if it is, then common.inFreeBSDJail isn't picking up on that....)

@jbergstroem
Copy link
Member

@Trott correct, freebsd102-64 is a vm at digitalocean.

@Trott
Copy link
Member Author

Trott commented Nov 15, 2015

Unfortunately, the test fails there too. The only difference is that this new code doesn't cause it to be skipped. :-/

@mscdex mscdex added dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests. freebsd Issues and PRs related to the FreeBSD platform. labels Nov 15, 2015
@jbergstroem
Copy link
Member

Thats the FreeBSD machine at joyent (not a jail). Shouldn't matter though. I can have a look later, but broadcast is properly responding at 255.255.255.255.

@jasnell
Copy link
Member

jasnell commented Nov 16, 2015

LGTM

@bnoordhuis
Copy link
Member

Not that you need it but have another LGTM.

Trott added a commit that referenced this pull request Nov 16, 2015
`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]>
@jasnell
Copy link
Member

jasnell commented Nov 16, 2015

Landed in a2144fc

@jasnell jasnell closed this Nov 16, 2015
Trott added a commit that referenced this pull request Nov 17, 2015
`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 pull request Nov 30, 2015
`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 pull request Dec 4, 2015
`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]>
@jasnell jasnell mentioned this pull request Dec 17, 2015
Trott added a commit that referenced this pull request Dec 17, 2015
`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 pull request Dec 23, 2015
`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 Trott deleted the jail branch January 13, 2022 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. freebsd Issues and PRs related to the FreeBSD platform. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants