test: use common platform helpers everywhere #7845
Conversation
|
|
||
| require('../common'); | ||
| // FIXME add sunos support | ||
| if ('linux freebsd darwin'.indexOf(process.platform) === -1) { | ||
| if (!common.isFreeBSD && !common.isOSX && !common.isLinux) { |
There was a problem hiding this comment.
This might be more readable as !(common.isFreeBSD || common.isOSX || common.isLinux)
|
LGTM with a couple comments. |
|
PR updated. Thanks |
|
CI came back all green. I wonder if the |
|
LGTM
The way I read it, the test was simply always skipped on all platforms. |
|
LGTM |
@cjihrig, after Ben's explanation, does the PR LGTY? |
|
@santigimeno yes. I think we were saying the same thing... I just said it wrong. |
Use the common.isWindows, common.isFreeBSD and common.isSunOS where possible. Add common.isOSX and common.isLinux. Fix `test-fs-read-file-sync-hostname` as in its current form was not being run anywhere. PR-URL: nodejs#7845 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
|
All green except a known |
|
Landed in dee0e3a |
Use the common.isWindows, common.isFreeBSD and common.isSunOS where possible. Add common.isOSX and common.isLinux. Fix `test-fs-read-file-sync-hostname` as in its current form was not being run anywhere. PR-URL: #7845 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test
Description of change
Use the
common.isWindows,common.isFreeBSDandcommon.isSunOSwhere possible. Addcommon.isOSXandcommon.isLinux.Fix
test-fs-read-file-sync-hostnameas in its current form was not being run anywhere./cc @nodejs/testing