mirror of
https://github.com/atom/atom.git
synced 2026-01-24 14:28:14 -05:00
Introduce timeout option
The idea with this commit is to implement a timeout functionality that does not rely on a system utility (such as unix `timeout`). Tests could hang because of a CPU-bound task and, as a result, we need to handle timeouts in a separate process. An ideal implementation would first spawn the timeout, which in turn would spawn the tests, acting as a supervisor and making sure they do not exceed the supplied time. However, setting up such an environment would have been trickier, thus the test process spawns the timeout cop before running any test. This, in turn, invokes a `setTimeout` function and kills the parent process as soon as the timeout gets triggered, logging to console the reason why the parent process was killed (and exiting the parent process with code 130). I haven't used `Task` because, in order to log stuff to the console, we need to fork the ChildProcess from `remote`, as otherwise nothing gets written out.
This commit is contained in:
@@ -65,7 +65,7 @@ class AtomApplication
|
||||
exit: (status) -> app.exit(status)
|
||||
|
||||
constructor: (options) ->
|
||||
{@resourcePath, @devResourcePath, @version, @devMode, @safeMode, @socketPath} = options
|
||||
{@resourcePath, @devResourcePath, @version, @devMode, @safeMode, @socketPath, @timeout} = options
|
||||
|
||||
global.atomApplication = this
|
||||
|
||||
@@ -87,9 +87,9 @@ class AtomApplication
|
||||
else
|
||||
@loadState() or @openPath(options)
|
||||
|
||||
openWithOptions: ({pathsToOpen, executedFrom, urlsToOpen, test, pidToKillWhenClosed, devMode, safeMode, newWindow, specDirectory, logFile, profileStartup}) ->
|
||||
openWithOptions: ({pathsToOpen, executedFrom, urlsToOpen, test, pidToKillWhenClosed, devMode, safeMode, newWindow, specDirectory, logFile, profileStartup, timeout}) ->
|
||||
if test
|
||||
@runTests({headless: true, @resourcePath, executedFrom, specDirectory, pathsToOpen, logFile})
|
||||
@runTests({headless: true, @resourcePath, executedFrom, specDirectory, pathsToOpen, logFile, timeout})
|
||||
else if pathsToOpen.length > 0
|
||||
@openPaths({pathsToOpen, executedFrom, pidToKillWhenClosed, newWindow, devMode, safeMode, profileStartup})
|
||||
else if urlsToOpen.length > 0
|
||||
@@ -493,7 +493,7 @@ class AtomApplication
|
||||
# :specPath - The directory to load specs from.
|
||||
# :safeMode - A Boolean that, if true, won't run specs from ~/.atom/packages
|
||||
# and ~/.atom/dev/packages, defaults to false.
|
||||
runTests: ({headless, resourcePath, executedFrom, specDirectory, pathsToOpen, logFile, safeMode}) ->
|
||||
runTests: ({headless, resourcePath, executedFrom, specDirectory, pathsToOpen, logFile, safeMode, timeout}) ->
|
||||
if resourcePath isnt @resourcePath and not fs.existsSync(resourcePath)
|
||||
resourcePath = @resourcePath
|
||||
|
||||
@@ -517,7 +517,7 @@ class AtomApplication
|
||||
isSpec = true
|
||||
devMode = true
|
||||
safeMode ?= false
|
||||
new AtomWindow({windowInitializationScript, resourcePath, headless, isSpec, devMode, testRunnerPath, legacyTestRunnerPath, testPaths, logFile, safeMode})
|
||||
new AtomWindow({windowInitializationScript, resourcePath, headless, isSpec, devMode, testRunnerPath, legacyTestRunnerPath, testPaths, logFile, safeMode, timeout})
|
||||
|
||||
resolveTestRunnerPath: (testPath) ->
|
||||
FindParentDir ?= require 'find-parent-dir'
|
||||
|
||||
@@ -19,7 +19,7 @@ class AtomWindow
|
||||
isSpec: null
|
||||
|
||||
constructor: (settings={}) ->
|
||||
{@resourcePath, pathToOpen, locationsToOpen, @isSpec, @headless, @safeMode, @devMode} = settings
|
||||
{@resourcePath, pathToOpen, locationsToOpen, @isSpec, @headless, @safeMode, @devMode, @timeout} = settings
|
||||
locationsToOpen ?= [{pathToOpen}] if pathToOpen
|
||||
locationsToOpen ?= []
|
||||
|
||||
|
||||
@@ -102,6 +102,7 @@ parseCommandLine = ->
|
||||
options.alias('s', 'spec-directory').string('s').describe('s', 'Set the directory from which to run package specs (default: Atom\'s spec directory).')
|
||||
options.boolean('safe').describe('safe', 'Do not load packages from ~/.atom/packages or ~/.atom/dev/packages.')
|
||||
options.alias('t', 'test').boolean('t').describe('t', 'Run the specified specs and exit with error code on failures.')
|
||||
options.string('timeout').describe('timeout', 'When in test mode, waits until the specified time (in minutes) and kills the process (exit code: 130).')
|
||||
options.alias('v', 'version').boolean('v').describe('v', 'Print the version.')
|
||||
options.alias('w', 'wait').boolean('w').describe('w', 'Wait for window to be closed before returning.')
|
||||
options.string('socket-path')
|
||||
@@ -121,6 +122,7 @@ parseCommandLine = ->
|
||||
safeMode = args['safe']
|
||||
pathsToOpen = args._
|
||||
test = args['test']
|
||||
timeout = args['timeout']
|
||||
specDirectory = args['spec-directory']
|
||||
newWindow = args['new-window']
|
||||
pidToKillWhenClosed = args['pid'] if args['wait']
|
||||
@@ -158,6 +160,6 @@ parseCommandLine = ->
|
||||
|
||||
{resourcePath, devResourcePath, pathsToOpen, urlsToOpen, executedFrom, test,
|
||||
version, pidToKillWhenClosed, devMode, safeMode, newWindow, specDirectory,
|
||||
logFile, socketPath, profileStartup}
|
||||
logFile, socketPath, profileStartup, timeout}
|
||||
|
||||
start()
|
||||
|
||||
@@ -5,6 +5,7 @@ try
|
||||
path = require 'path'
|
||||
ipc = require 'ipc'
|
||||
remote = require 'remote'
|
||||
app = remote.require('app')
|
||||
{getWindowLoadSettings} = require './window-load-settings-helpers'
|
||||
AtomEnvironment = require '../src/atom-environment'
|
||||
|
||||
@@ -32,6 +33,12 @@ try
|
||||
|
||||
document.title = "Spec Suite"
|
||||
|
||||
timeoutCop = null
|
||||
if timeout = getWindowLoadSettings().timeout
|
||||
ChildProcess = remote.require("child_process")
|
||||
timeoutCop = ChildProcess.fork(require.resolve('./timeout-cop'), [remote.process.pid, timeout])
|
||||
app.on "will-exit", -> timeoutCop.kill()
|
||||
|
||||
legacyTestRunner = require(getWindowLoadSettings().legacyTestRunnerPath)
|
||||
testRunner = require(getWindowLoadSettings().testRunnerPath)
|
||||
testRunner({
|
||||
@@ -41,11 +48,9 @@ try
|
||||
buildAtomEnvironment: -> new AtomEnvironment
|
||||
legacyTestRunner: legacyTestRunner
|
||||
})
|
||||
|
||||
catch error
|
||||
if getWindowLoadSettings().headless
|
||||
console.error(error.stack ? error)
|
||||
app = remote.require('app')
|
||||
app.emit('will-exit')
|
||||
remote.process.exit(status)
|
||||
else
|
||||
|
||||
12
src/timeout-cop.js
Normal file
12
src/timeout-cop.js
Normal file
@@ -0,0 +1,12 @@
|
||||
var parentProcessId = process.argv[2];
|
||||
var timeoutInMinutes = process.argv[3];
|
||||
var timeoutInMilliseconds = timeoutInMinutes * 1000 * 60
|
||||
|
||||
function exitTestRunner() {
|
||||
process.kill(parentProcessId, "SIGINT");
|
||||
var errorMessage = "The test suite has timed out because it has been running";
|
||||
errorMessage += " for more than " + timeoutInMinutes + " minutes.";
|
||||
console.log(errorMessage);
|
||||
}
|
||||
|
||||
setTimeout(exitTestRunner, timeoutInMilliseconds);
|
||||
Reference in New Issue
Block a user