add tests for array.prototype.groupToMap #3353

polsevev · 2021-12-15T10:49:22Z

Added tests for proposal Array Grouping

rwaldron · 2021-12-15T17:29:24Z

Wow! This is a great head start here :) Let's wait until our video call on Friday to do a review together.

ljharb · 2021-12-15T17:35:43Z

Similarly to #3354, i think this could use some tests for throwing on a non-function callback, but otherwise it matches my polyfill's tests!

test/built-ins/Array/prototype/groupByToMap/emptyList.js

ptomato

Awesome, thanks - I just have two nitpicks about things that probably slipped in when copying the new stuff from the group PR. I will fix these up and merge it right away.

test/built-ins/Array/prototype/groupToMap/map-instance.js

test/built-ins/Array/prototype/groupToMap/sparse-array.js

ptomato

Re-approving

ljharb · 2022-09-14T18:20:34Z

test/built-ins/Array/prototype/groupToMap/array-like.js

+
+  ...
+includes: [compareArray.js]
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, Symbol.iterator, arrow-function, Map]

ljharb · 2022-09-14T18:21:02Z

test/built-ins/Array/prototype/groupToMap/callback-arg.js

+    c. Let key be ? Call(callbackfn, thisArg, « kValue, 𝔽(k), O »).
+    e. Perform ! AddValueToKeyedGroup(groups, key, kValue).
+  ...
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, arrow-function, Map]

ljharb · 2022-09-14T18:21:34Z

test/built-ins/Array/prototype/groupToMap/callback-throws.js

@@ -0,0 +1,24 @@
+// Copyright (c) 2021 Ecma International.  All rights reserved.


should all these change to 2022?

ljharb · 2022-09-14T18:21:43Z

test/built-ins/Array/prototype/groupToMap/callback-throws.js

+    c. Let propertyKey be ? ToPropertyKey(? Call(callbackfn, thisArg, « kValue, 𝔽(k), O »)).
+
+  ...
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, Map]

ljharb · 2022-09-14T18:21:52Z

test/built-ins/Array/prototype/groupToMap/emptyList.js

+    b. Perform ! CreateDataPropertyOrThrow(map, g.[[Key]], elements).
+
+  ...
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, Map, arrow-function]

ljharb · 2022-09-14T18:23:35Z

test/built-ins/Array/prototype/groupToMap/negativeZero.js

+
+  ...
+includes: [compareArray.js]
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, Map, arrow-function]

ljharb · 2022-09-14T18:23:46Z

test/built-ins/Array/prototype/groupToMap/sparse-array.js

+
+  ...
+includes: [compareArray.js]
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, Map, arrow-function]

ljharb · 2022-09-14T18:23:59Z

test/built-ins/Array/prototype/groupToMap/this-arg-strict.js

+
+  ...
+flags: [onlyStrict]
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, Map]

ljharb · 2022-09-14T18:24:09Z

test/built-ins/Array/prototype/groupToMap/this-arg.js

+
+  ...
+flags: [noStrict]
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, Map]

ljharb · 2022-09-14T18:24:21Z

test/built-ins/Array/prototype/groupToMap/toPropertyKey.js

+
+  ...
+includes: [compareArray.js]
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, Map, Symbol.iterator, arrow-function]

ljharb · 2022-09-14T18:25:08Z

test/built-ins/Array/prototype/groupToMap/array-like.js

+
+  ...
+includes: [compareArray.js]
+features: [array-grouping]


Suggested change

features: [array-grouping]

features: [array-grouping, Map, Symbol.iterator, arrow-function]

ptomato · 2022-09-14T18:32:06Z

re. exhaustive features list, shrug; the documentation in features.txt says about stage 4 features:

Language features that have been included in a published version of the ECMA-262 specification. These flags are largely maintained for historical reasons, though their use for relatively new features (i.e. prior to availability across major implementations) is appreciated.

I don't think these are "relatively new" and it seems burdensome to contributors who just want to get their tests landed, for us to require them. Many of them can be added automatically anyway should anyone complain.

ljharb · 2022-09-14T18:34:25Z

It's totally fine to land this without them, and i can add them to #3664.

However, if we want test262 to ever be usable on packages - which tend to have long lists of supported platforms - exhaustive feature lists are a critical thing to have to ensure that's achievable.

review is nonblocking

ptomato · 2022-09-14T18:36:59Z

Fair enough, I'd say we should avoid putting that burden on test contributors though 😄

Let's open an issue to review that documentation in features.txt and do something like add a pre-commit script that will automatically add as many features as possible.

ptomato · 2022-09-14T18:37:56Z

Agreed, let's land this now as it's been waiting for a long time, and update #3664

ljharb · 2022-09-14T18:40:07Z

Filed #3664.

polsevev force-pushed the groupByToMap branch 4 times, most recently from beb02e0 to eab650a Compare December 15, 2021 11:49

rwaldron added the valid and in progress Issue is valid, action in progress label Dec 15, 2021

legendecas reviewed Jan 6, 2022

View reviewed changes

test/built-ins/Array/prototype/groupByToMap/emptyList.js Outdated Show resolved Hide resolved

ljharb requested a review from a team May 19, 2022 18:23

ljharb added the awaiting author label May 19, 2022

jridgewell mentioned this pull request Sep 1, 2022

Add group/groupToMap tests #3652

Closed

jridgewell changed the title ~~add tests for array.prototype.groupByToMap~~ add tests for array.prototype.groupToMap Sep 2, 2022

ptomato added needs review and removed awaiting author labels Sep 6, 2022

polsevev and others added 2 commits September 14, 2022 11:03

add tests for array.prototype.groupByToMap

5afcba1

Update groupToMap tests

7a4ec2b

ptomato previously approved these changes Sep 14, 2022

View reviewed changes

test/built-ins/Array/prototype/groupToMap/map-instance.js Outdated Show resolved Hide resolved

test/built-ins/Array/prototype/groupToMap/sparse-array.js Outdated Show resolved Hide resolved

ptomato dismissed their stale review via 7a4ec2b September 14, 2022 18:19

ptomato force-pushed the groupByToMap branch from 66ee99d to 7a4ec2b Compare September 14, 2022 18:19

ptomato approved these changes Sep 14, 2022

View reviewed changes

ljharb previously requested changes Sep 14, 2022

View reviewed changes

ljharb reviewed Sep 14, 2022

View reviewed changes

ptomato merged commit 9592077 into tc39:main Sep 14, 2022

ljharb mentioned this pull request Sep 14, 2022

Let's have exhaustive feature lists! #3665

Open

ljharb added a commit that referenced this pull request Sep 14, 2022

Fix features in tests from #3354 and #3353

8f19bb4

catamorphism pushed a commit to catamorphism/test262 that referenced this pull request Oct 4, 2022

Fix features in tests from tc39#3354 and tc39#3353

e2eba2b

catamorphism pushed a commit to catamorphism/test262 that referenced this pull request Oct 5, 2022

Fix features in tests from tc39#3354 and tc39#3353

febd30a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add tests for array.prototype.groupToMap #3353

add tests for array.prototype.groupToMap #3353

polsevev commented Dec 15, 2021

rwaldron commented Dec 15, 2021

ljharb commented Dec 15, 2021

ptomato left a comment

ptomato left a comment

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ljharb Sep 14, 2022

ptomato commented Sep 14, 2022

ljharb commented Sep 14, 2022

ptomato commented Sep 14, 2022

ptomato commented Sep 14, 2022

ljharb commented Sep 14, 2022 •

edited by Ms2ger

	features: [array-grouping]
	features: [array-grouping, Symbol.iterator, arrow-function, Map]

	features: [array-grouping]
	features: [array-grouping, arrow-function, Map]

		@@ -0,0 +1,24 @@
		// Copyright (c) 2021 Ecma International. All rights reserved.

	features: [array-grouping]
	features: [array-grouping, Map, arrow-function]

	features: [array-grouping]
	features: [array-grouping, Map, Symbol.iterator, arrow-function]

add tests for array.prototype.groupToMap #3353

add tests for array.prototype.groupToMap #3353

Conversation

polsevev commented Dec 15, 2021

rwaldron commented Dec 15, 2021

ljharb commented Dec 15, 2021

ptomato left a comment

Choose a reason for hiding this comment

ptomato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ptomato commented Sep 14, 2022

ljharb commented Sep 14, 2022

ptomato commented Sep 14, 2022

ptomato commented Sep 14, 2022

ljharb commented Sep 14, 2022 • edited by Ms2ger

ljharb commented Sep 14, 2022 •

edited by Ms2ger