Will it have impact on startup time?

Will it have impact on startup time?

No not really.

@thefourtheye only when the flag is enabled, otherwise we are looking at a few integer checks for preloaded_module_count

CI: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/201/

Test is failing on smartos-14 (32 and 64bit):

not ok 535 - test-preload.js
# 
# assert.js:89
# throw new assert.AssertionError({
# ^
# AssertionError: '' == 'A\nhello\n'
# at ChildProcess.<anonymous> (/home/iojs/build/workspace/iojs+pr+other/nodes/smartos14-64/test/parallel/test-preload.js:78:10)
# at emitTwo (events.js:87:13)
# at ChildProcess.emit (events.js:172:7)
# at Process.ChildProcess._handle.onexit (internal/child_process.js:200:12)

@Fishrock123 even without this patch it should never be empty... that is... concerning

cc @nodejs/platform-solaris

is there a person I can talk to about this, as I don't run SmartOS normally, and am completely lost as to how it could be empty/worked prior to this.

cc @jbergstroem / @misterdjules again

CI again https://jenkins-iojs.nodesource.com/job/node-test-pull-request/119/

Seems fine to me. @bmeck how do you feel about just ignoring it on smartos?

The commit message is too long and commit log has to be improved.

@thefourtheye improved how

LGTM otherwise, I'll just add some detail to the commit description if you don't. :)

I can reproduce this fail but haven't had time to look into it. Would appreciate if @misterdjules had a moment.

rebased. uses camelcase now. repro'd fail on smartos, trying to reduce it

@nodejs/platform-solaris i am unsure how to test this exactly, but is closing stdin before it is fully drained dropping the content on smartos?

ping @No9, perhaps you can lend an eye to this one since you're an Illumos user?

If anyone else is using illumos/smartos and would like to help out reviewing that'd be great!

also would love to additional people to @nodejs/platform-solaris if there are any

@rvagg I'll look in over the weekend

@No9 would you be interested in being part of the small team of people that (in my case, at least tries to) debugs sunos stuff?

@jbergstroem Thanks for asking
Yes I would :)

added to @nodejs/platform-solaris, thanks @No9!

ping @bmeck

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

I'm also testing github statues here, see #6674 for more info but please ignore it as it may be incorrect.

Landed with some nits in 41a063f

(Changed the Skip check to use common.isSunOS)

CI ensuring the nits still work: https://ci.nodejs.org/job/node-test-commit-smartos/2435/

Hmm, just got this on my OS X

=== release test-preload ===                                                   
Path: parallel/test-preload
assert.js:90
  throw new assert.AssertionError({
  ^
AssertionError: '' == 'A\nhello\n'
    at ChildProcess.<anonymous> (/Users/Jeremiah/Documents/node/test/parallel/test-preload.js:86:10)
    at emitTwo (events.js:106:13)
    at ChildProcess.emit (events.js:191:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:204:12)
Command: out/Release/node /Users/Jeremiah/Documents/node/test/parallel/test-preload.js

Passed here. Reproducible?
On May 11, 2016 2:31 PM, "Jeremiah Senkpiel" notifications@github.com
wrote:

Hmm, just got this on my OS X

=== release test-preload ===
Path: parallel/test-preload
assert.js:90
throw new assert.AssertionError({
^
AssertionError: '' == 'A\nhello\n'
at ChildProcess. (/Users/Jeremiah/Documents/node/test/parallel/test-preload.js:86:10)
at emitTwo (events.js:106:13)
at ChildProcess.emit (events.js:191:7)
at Process.ChildProcess._handle.onexit (internal/child_process.js:204:12)
Command: out/Release/node /Users/Jeremiah/Documents/node/test/parallel/test-preload.js

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#2253 (comment)

Strange, it happened as part of #6697, but I din't touch any of those parts of the test. I can't reproduce while looking the test either.

Failed on AIX in last night's run as well #6716

This is starting to look like some sort of race condition

@bmeck @Fishrock123 on AIX it looks like a consistent failure. If it helps I can give one or both of you access to run on that machine.

We should also skip on AIX for now then, but we do need to investigate more.

@bmeck looks like there are some regressions here. Adding dont-land for now, but please feel free to open a PR against v4.x-staging if this should be packported

@thealphanerd No, those are just the result of other issues.

this should be fine if it lands with #6728

thanks @Fishrock123 will work on that later today

@bmeck would you be willing to do a manual backport of this and #6728 ?

@thealphanerd onto v4?

indeed. It is not landing cleanly

ping @bmeck

@thealphanerd backported against v4.5.0 (no staging exists right now?) on https://github.com/bmeck/node/tree/backport-2253 , still flaky it seems after running tests

the staging is v4.x-staging

If it is still flaky then perhaps we should just mark this don't land?

@thealphanerd these are just adding tests for expected behavior, if we don't want to land it thats fine

@bmeck if that is the case then we should land it. Would you be willing to open a PR?

@thealphanerd #8340