Add test on handling broken promises in AsyncGenerator.prototype.return #3472
Conversation
legendecas
force-pushed
the
ecma262-2683
branch
from
eb96caf
to
b421f84
Compare
legendecas
force-pushed
the
ecma262-2683
branch
from
b421f84
to
53eebee
Compare
There was a problem hiding this comment.
Thanks for these tests! I have to say, this is my first time really digging into how AsyncGenerators are specified, so several of my comments might come from misunderstanding something. But hopefully I've also suggested some improvements that make it easier to comprehend by someone new to this part of the specification.
Do we need to have tests for the "executing" and "awaiting-return" states as well? Or is this enough since steps 10 and 11 of https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-asyncgenerator-prototype-return weren't touched by the normative PR?
test/built-ins/AsyncGeneratorPrototype/return/return-suspendedYield-broken-promise-try-catch.js
Outdated
Show resolved
Hide resolved
test/built-ins/AsyncGeneratorPrototype/return/return-suspendedYield-broken-promise-try-catch.js
Outdated
Show resolved
Hide resolved
test/built-ins/AsyncGeneratorPrototype/return/return-blocked-broken-promise.js
Outdated
Show resolved
Hide resolved
test/built-ins/AsyncGeneratorPrototype/return/return-blocked-broken-promise.js
Outdated
Show resolved
Hide resolved
test/built-ins/AsyncGeneratorPrototype/return/return-blocked-broken-promise.js
Outdated
Show resolved
Hide resolved
test/built-ins/AsyncGeneratorPrototype/return/return-suspendedYield-broken-promise-try-catch.js
Outdated
Show resolved
Hide resolved
test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js
Outdated
Show resolved
Hide resolved
I didn't find such tests. I can add those in a follow-up PR. |
test/built-ins/AsyncGeneratorPrototype/return/return-suspendedYield-broken-promise-try-catch.js
Outdated
Show resolved
Hide resolved
| yield; | ||
| return 'this is never returned'; | ||
| } catch (err) { | ||
| assert.sameValue(err.message, 'broken promise'); |
There was a problem hiding this comment.
Since this test is scrutinizing the completion semantics of this function, we should avoid making assertions within the body. Instead, we could store the value in a mutable binding declared in a higher scope, and assert that value in the test's final validation phase.
There was a problem hiding this comment.
Thank you for your suggestion! Updated :)
legendecas
force-pushed
the
ecma262-2683
branch
from
b959bba
to
a8bf843
Compare
There was a problem hiding this comment.
Thanks, @legendecas, this looks good to me!
We'll coordinate with the ECMA262 maintainers to merge this and the corresponding spec patch.
|
Thanks again, @legendecas! |
…rn (tc39#3472) * Add test on handling broken promises in AsyncGenerator.prototype.return * fixup! * fixup!
Tests for tc39/ecma262#2683.