feat(server): Add stop method #3153
Conversation
lib/server.js
Outdated
| @@ -302,6 +302,11 @@ class Server extends KarmaEventEmitter { | |||
| } | |||
| executor.schedule() | |||
| }) | |||
|
|
|||
| this.on('stop', function (done) { | |||
There was a problem hiding this comment.
So 'stop' handler is only defined if config.autoWatch?
There was a problem hiding this comment.
I thought that made sense, since otherwise the server should stop anyway. Do you think it should always be available?
That would allow stopping a server that's taking too long in a single run. I don't have that usecase myself, but maybe others do.
There was a problem hiding this comment.
I think having the API go away when the config changes could be unnecessarily mysterious.
There was a problem hiding this comment.
Yes, you're right. That would be confusing. Moved outside this condition.
lib/server.js
Outdated
| @@ -327,16 +332,17 @@ class Server extends KarmaEventEmitter { | |||
| done(code || 0) | |||
| } | |||
|
|
|||
| this.emitAsync('exit').then(() => { | |||
| return this.emitAsync('exit').then(() => new Promise((resolve, reject) => { | |||
There was a problem hiding this comment.
I think this would be clearer with an ArrowFunction
...() => {
return new Promise...
There was a problem hiding this comment.
I'm sorry, I don't understand. This is already an arrow function.
Do you mean adding the block to the arrow function instead of the return value directly?
There was a problem hiding this comment.
Yes a function rather than an ArrowExpression
There was a problem hiding this comment.
Oh I guess it is called a concise body vs a block body
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions#Function_body
There was a problem hiding this comment.
Got it, changed.
lib/server.js
Outdated
| @@ -302,6 +302,11 @@ class Server extends KarmaEventEmitter { | |||
| } | |||
| executor.schedule() | |||
| }) | |||
|
|
|||
| this.on('stop', function (done) { | |||
| this.log.debug('Exiting.') | |||
There was a problem hiding this comment.
How about 'Received stop event, exiting' or something that makes it clear why we exit.
There was a problem hiding this comment.
Sounds good, will change.
|
Awesome, thank you for taking the time to look at this! |
Fix #3149