Do not call callbacks twice in async boot scripts#260
Do not call callbacks twice in async boot scripts#260bajtos merged 1 commit intostrongloop:2.xfrom lehni:feature/async-boot-scripts-2.x
Conversation
|
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
bajtos
left a comment
There was a problem hiding this comment.
Thank you for the pull request!
test/executor.test.js
Outdated
| count++; | ||
| expect(count).to.eql(1); | ||
| process.nextTick(function() { | ||
| done(); |
There was a problem hiding this comment.
I think this is rather brittle as it relies on boot.execute to make the second call of the callback before process.nextTick is run.
Fortunately, mocha detects the situation when the done callback was called twice and fails the test with a helpful error: Error: done() called multiple times.
I am proposing to simplify this test to simply pass done as the callback of boot.execute.
There was a problem hiding this comment.
Ok, let me test this.
There was a problem hiding this comment.
You're right! I adjusted the PR accordingly now.
|
@slnode ok to test |
| }); | ||
|
|
||
| it('should only call the callback once', function(done) { | ||
| boot.execute(app, simpleAppInstructions(), done); |
There was a problem hiding this comment.
Perhaps add a comment explaining that mocha fails the test when the callback was called multiple times?
See #252 for details
|
Landed, thank you for the contribution! 🎉 |
Description
As discussed in #252 (comment), we need to make sure that a boot script that both returns a promise and calls the callback method does not trigger two calls of the callback method passed to
boot.execute().Related issues
Checklist
guide