Unblock test promise chain when done() is called early. #226
Conversation
| var promise = test.step(func, test, test); | ||
| test.step(function() { | ||
| assert_not_equals(promise, undefined); | ||
| }); | ||
| return Promise.resolve(promise) | ||
| return Promise.race([promise, earlyDonePromise]) |
There was a problem hiding this comment.
I think it's a bit weird to call done() again after earlyDonePromise resolves. Possibly cleaner (and just as correct) would be to get rid of the "return" on this line, and add a "return earlyDonePromise" (maybe renamed to just "donePromise") after it? That way it's clear that the next test always is started when done is called, and the block here is just to make sure done is called when the passed in promise resolves.
This makes it possible to end a promise_test by calling Test.done(). Previously, that would work for a single test, but would get any follow-up test stuck waiting for the global promise chain to resolve.
pwnall
force-pushed
the
fix_early_done
branch
from
c713b57 to
f586b24
Compare
|
@mkruisselbrink thoughts? |
I think this looks good.
I don't think it should. Test.step() also doesn't complain if it is called after a test has finished (it just silently doesn't do anything), so I think it's reasonable to similarly quietly ignore if the promise passed to promise_test resolves or rejects after Test.done() is called. |
This makes it possible to end a promise_test by calling Test.done(). Previously, that would work for a single test, but would get any follow-up test stuck waiting for the global promise chain to resolve.
Test case below. Without the patch, the first test times out and the second test passes. With the patch, both tests pass.
Credit: The fix does not belong to me. I implemented the solution proposed in #225 (comment)
Question: should the race promise reject and complain that Test.done() was called in a promise_test before the test's promise resolved?
This change is