-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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: increase timeouts on arm #1366
Changes from 2 commits
8855832
0c2210b
30a1882
77f952c
e4a5192
d577d19
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
'use strict'; | ||
|
||
var path = require('path'); | ||
var fs = require('fs'); | ||
var assert = require('assert'); | ||
|
@@ -179,6 +181,34 @@ exports.spawnPwd = function(options) { | |
} | ||
}; | ||
|
||
const PLATFORM_TIMOUT_FACTORS = { | ||
armv6: 6.0, | ||
armv7: 2.0 | ||
}; | ||
|
||
exports.platformTimeout = function (ms) { | ||
if (process.arch !== 'arm') | ||
return ms; | ||
|
||
let cpuinfo; | ||
|
||
// arm version detection based on v8 | ||
try { | ||
cpuinfo = fs.readFileSync('/proc/cpuinfo').toString(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} catch (e) { | ||
return ms; | ||
} | ||
|
||
if (/\nCPU architecture:\s7\n/.test(cpuinfo)) { | ||
return ms * PLATFORM_TIMOUT_FACTORS[/\(v6l\)/.test(cpuinfo) ? | ||
'armv6' : 'armv7']; | ||
} else if (/\nCPU architecture:\s6\n/.test(cpuinfo)) { | ||
return ms * PLATFORM_TIMOUT_FACTORS['armv6']; | ||
} else { | ||
return ms; | ||
} | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there's an easier way: exports.platformTimeout = function(ms) {
if (process.platform !== 'arm')
return ms;
// arm_version is a stringified number or 'default'
if (process.config.variables.arm_version === '6')
return 6 * ms; // ARMv6
return 2 * ms; // ARMv7 and up.
}; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, that's pretty convenient. Didn't know about these variables. |
||
|
||
var knownGlobals = [setTimeout, | ||
setInterval, | ||
setImmediate, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,7 +51,7 @@ function onNoMoreLines() { | |
|
||
setTimeout(function testTimedOut() { | ||
assert(false, 'test timed out.'); | ||
}, 6000).unref(); | ||
}, common.platformTimeout(3000)).unref(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This timeout seems to have been chosen too generous, so I reduced it. |
||
|
||
process.on('exit', function onExit() { | ||
// Kill processes in reverse order to avoid timing problems on Windows where | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,9 @@ var server = tls.createServer(options, function(c) { | |
|
||
server.listen(common.PORT, function() { | ||
var socket = net.connect(common.PORT, function() { | ||
socket.setTimeout(240, assert.fail); | ||
socket.setTimeout(common.platformTimeout(240), function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you undo the style change here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test did show an error There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I mean is the |
||
throw new Error('timeout'); | ||
}); | ||
|
||
var tsocket = tls.connect({ | ||
socket: socket, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,7 +72,7 @@ function parent() { | |
setTimeout(function() { | ||
throw new Error('hang'); | ||
}); | ||
}, 4000).unref(); | ||
}, common.platformTimeout(2000)).unref(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
|
||
var s = spawn(node, [__filename, 'server'], opt); | ||
var c; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor style issue:
function(ms)