Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test that the sync iterator that returns a rejected promise closes. #3990

Conversation

ioannad
Copy link
Contributor

@ioannad ioannad commented Jan 15, 2024

This adds tests for the normative changes in tc39/ecma262#2600 for the last block of new steps in AsyncFromSyncIteratorContinuation.

These tests have been tested with this PR of engine262, which implements the normative changes.

3rd of 3 PRs that should finish addressing #3387 .

@ioannad ioannad requested a review from a team as a code owner January 15, 2024 12:12
Copy link
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct to me. I was thinking of suggesting using assert.throwsAsync() but ultimately I think this is fine because it's clearer with the for-await loop.

Would be nice to get some eyes on it from some of the people involved in the normative PR, e.g. @bakkot, @mhofman

@ioannad ioannad changed the title Test that the sync iterator that returns a rejected promise closes when called via for-await-of. Test that the sync iterator that returns a rejected promise closes. Jan 17, 2024
when a rejected promise is returned normally by the iterator.

These tests correspond to this normative changes example:
https://docs.google.com/presentation/d/1W9EJoWOvi6jU1A2bwyixzOceoP_ASAeaOYER3Zy9P00/edit#slide=id.g10c9207737d_0_31
This includes the test case mentioned in ecma262 issue comment:
tc39/ecma262#1849 (comment)

Also using yield* we can test the changes to AsyncFromSyncIteratorContinuation
when called via .throw().
@ptomato ptomato force-pushed the async-from-sync-iterator-close-tests-next-rejected-promise branch from 2768020 to 09ca86b Compare January 23, 2024 21:26
@ptomato ptomato merged commit fed13c1 into tc39:main Jan 23, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants