Conversation
Cover shared failure/success behavior for Promise.allKeyed and Promise.allSettledKeyed, including non-object rejection, non-constructor context, key ordering, and allSettled result-shape outcomes.
d6a6c31 to
50702d4
Compare
…nt coverage - Change key-order-preserved settle sequence to 2nd, 3rd, 1st to avoid false passes from reverse-order implementations - Use Reflect.ownKeys instead of Object.keys in empty-object assertions for exhaustive own-key verification - Add separate BigInt rejection tests gated behind BigInt feature flag
607531a to
589f4da
Compare
- prototype-keys-ignored: inherited enumerable properties are excluded ([[OwnPropertyKeys]] returns only own keys) - symbol-keys: enumerable symbol-keyed properties are included in the result alongside string keys - arg-is-function: functions with enumerable own properties are accepted as valid object arguments; built-in non-enumerable properties excluded
fd62c35 to
6c64e5d
Compare
ptomato
left a comment
There was a problem hiding this comment.
Thanks! Just a few minor comments.
For future reference, the tests are more readable when using async helpers. (And would avoid the $DONE called twice situation) No need to go back and change these unless you really want to, though.
test/built-ins/Promise/allKeyed/resolve-not-callable-reject-with-typeerror.js
Outdated
Show resolved
Hide resolved
- Adopt asyncHelpers.js across all 23 async test files - Replace manual .then($DONE, $DONE) with asyncTest wrapper - Convert TypeError rejection tests to assert.throwsAsync for more robust error checking - Fixes double-$DONE bug in resolve-not-callable-reject-with-typeerror
6d6ab23 to
89ed7e3
Compare
|
Thanks for the review @ptomato! Converted all async tests to use asyncTest / assert.throwsAsync from asyncHelpers.js -- fixes the double-$DONE issue and should be easier to read going forward. null and undefined should have been already covered in the chain (lines 28-29). |
| @@ -0,0 +1,36 @@ | |||
| // Copyright (C) 2026 Danial Asaria (Bloomberg LP). All rights reserved. | |||
There was a problem hiding this comment.
was this file supposed to be named reject-immediately?
There was a problem hiding this comment.
Either one works for me, we usually don't sweat the filenames too much as long as they are descriptive (and not spec references like "13.1.9.step5.js" or "fail-RequireInternalSlot-step2.js")
There was a problem hiding this comment.
Im happy either way too. Mostly just checking it wasn't a typo.
There was a problem hiding this comment.
Not a typo! Went with the shorter form, but happy to rename if there's a preference.
|
|
||
| var sym = Symbol('s'); | ||
| var input = { str: Promise.resolve(1) }; | ||
| input[sym] = Promise.resolve(2); |
There was a problem hiding this comment.
I think it would be good to also have a non-enumerable symbol. Then we've covered all 4 combinations of string/symbols and enumerable/non-enumerable.
There was a problem hiding this comment.
Good idea--added a non-enumerable symbol to both symbol-keys.js files so we cover all 4 cases.
ptomato
left a comment
There was a problem hiding this comment.
👍, thanks!
We can merge after addressing Ashley's comment, or merge now and leave it for a followup, whatever you prefer.
Covers all 4 combinations of string/symbol and enumerable/non-enumerable.
Addressed! Should be good to merge now |
Summary
This PR adds the first runtime/behavioral coverage for the await-dictionary proposal’s Promise keyed combinators:
Promise.allKeyedPromise.allSettledKeyedIt follows the initial boilerplate-only PR and focuses on high-signal success/failure semantics and ordering behavior.
What this PR covers
Shared coverage for both methods
For both
Promise.allKeyedandPromise.allSettledKeyed, this PR adds tests for:promisesis not an Object (primitive matrix:undefined,null, number, string, boolean, symbol)NewPromiseCapabilitypath)resolveis non-callable (GetPromiseResolve/IfAbruptRejectPromisepath){}to an empty null-prototype result objectPromise.allKeyed-specificPromise.allSettledKeyed-specific{ status: "fulfilled", value }{ status: "rejected", reason }Notes
features: [await-dictionary].Promise/allandPromise/allSettled.What this PR does not cover
This PR intentionally leaves some deeper edge paths for follow-up work, including:
promiseResolvethrowing or returning a non-thenable[[OwnPropertyKeys]]/[[GetOwnProperty]]remainingElementsCountstep-8 edge behaviorReferences