initial work by benjamingr · Pull Request #2 · caitp/test262

benjamingr · 2017-01-13T14:50:11Z

No description provided.

caitp · 2017-01-13T15:02:48Z

test/language/expressions/async-generators/early-errors-expression-parameter-await.js

+  type: SyntaxError
+---*/
+
+(async function* (await) {});


I believe these two early errors tests are already present in the PR

Roger, removed.

caitp · 2017-01-13T15:04:38Z

test/language/expressions/async-generators/early-errors-next-receiver-not-async-iterator.js

+---*/
+
+(async function*() {})().next.call((function*() {})()).then(assert.throw.bind(assert), function(err) {
+    ass9ert(err instanceof TypeError);


Nit: typo.

I would also declare the functions as variables here and put logic on more lines

Otherwise looks good

caitp · 2017-01-13T15:07:38Z

test/language/expressions/async-generators/early-errors-next-receiver-not-async-iterator.js

+esid: pending
+description: >
+  next() rejects with a TypeError when receiver is not an AsyncGenerator instance:
+  phase: early


The phase and type frontmatter bits should be removed, since this is not really a negative test

caitp · 2017-01-13T15:13:29Z

test/language/expressions/async-generators/early-errors-next-receiver-not-async-iterator.js

+  type: TypeError
+---*/
+
+(async function*() {})().next.call((function*() {})()).then(assert.throw.bind(assert), function(err) {


There's probably a better way to assert that the resolve handler is never called. Maybe just throw a Teat262Assertion in the handler indicating that it shouldn't be called, it's easier to understand

I can't find Test262Assertion (or Teat262Assertion) in the source. I'll throw an Error - let me know if that's fine.

Mobile Swype makes code review hard. It's Test262Error

caitp · 2017-01-13T15:14:42Z

test/built-ins/Symbol/asyncIterator/prop-desc.js

+/*---
+esid: early
+description: >
+    `Symbol.iterator` property descriptor


AsyncIterator

caitp · 2017-01-13T15:15:50Z

test/built-ins/Symbol/asyncIterator/cross-realm.js

+
+var OSymbol = $.createRealm().global.Symbol;
+
+assert.sameValue(Symbol.asyncIterator, Symbol.asyncIterator);


This assertion looks wrong, missing the other realm symbol

Right, fixed.

caitp · 2017-01-13T15:16:20Z

test/built-ins/Symbol/asyncIterator/cross-realm.js

+features: [Symbol.asyncIterator]
+---*/
+
+var OSymbol = $.createRealm().global.Symbol;


OSymbol could probably be more descriptive of what it is

I copied it from the regular async function tests, what would be a good name?

I'd probably call it otherRealmSymbol, but it's just personal preference

benjamingr · 2017-01-13T15:25:00Z

Ok, going to do that, thanks.

…

On Fri, Jan 13, 2017 at 5:21 PM, ⭐caitp⭐ ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In test/built-ins/Symbol/asyncIterator/cross-realm.js <#2>: > @@ -0,0 +1,15 @@ +// Copyright (C) 2016 the V8 project authors. All rights reserved. +// This code is governed by the BSD license found in the LICENSE file. +/*--- +author: Benjamin Gruenbaum ***@***.***> +esid: early +description: Value shared by all realms +info: > + Unless otherwise specified, well-known symbols values are shared by all + realms. +features: [Symbol.asyncIterator] +---*/ + +var OSymbol = $.createRealm().global.Symbol; I'd probably call it otherRealmSymbol, but it's just personal preference — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQSzZpWWd6h_jZSb5GB9C-iWbapDsLWks5rR5Z1gaJpZM4Li9F1> .

symbol tests more symbol tests feedback by caitp simplify symbol test fix built in tests initial work ORealm -> otherRealmSymbol Error->Test262Error figure out what these tests mean subclass tests 6.3.2.1

…ingr/test262 into AsyncGeneratorExpressions

# The first commit's message is: initial work fix test to reject instead of throw symbol tests more symbol tests feedback by caitp simplify symbol test fix built in tests initial work ORealm -> otherRealmSymbol Error->Test262Error figure out what these tests mean AsyncGeneratorMethod throw a SyntaxError if UniqueFormalParameters contains AwaitExpression AsyncGeneratorMethod throw a SyntaxError if UniqueFormalParameters contains YieldExpression. AsyncGeneratorMethod throw a SyntaxError if HasDirectSuper of AsyncGeneratorMethod is true. async generators test fix test to reject instead of throw symbol tests more symbol tests feedback by caitp simplify symbol test fix built in tests initial work ORealm -> otherRealmSymbol Error->Test262Error figure out what these tests mean subclass tests 6.3.2.1 # This is the commit message caitp#2: test name # This is the commit message tc39#3: port thefourtheyes tests # This is the commit message tc39#4: fixed async/yield test naming (added expression), also added two tests for yield/await named params # This is the commit message tc39#5: oops, untracked accesors # This is the commit message tc39#6: renamed tests to conform with other early tests # This is the commit message tc39#7: removed unnecessary copyright # This is the commit message tc39#8: added wrongly created file # This is the commit message tc39#9: Update AsyncGenerator-is-extensible.js

Async generator expressions

…ingr/test262 into AsyncGeneratorExpressions

benjamingr · 2017-01-13T17:50:24Z

OK, we're done for the day, please take a look and let me know.

benjamingr · 2017-01-13T19:03:30Z

@caitp I'm done for the day - please take a look, I'll fix any issues you run into.

caitp

the one thing is, I think it's better if the tests by each author are grouped into their own commits, and possibly their own pull requests on the test262 repository, to make CLA verification easier, and so that the author of specific tests is attributed as the author of the commit when merged.

I think I'd prefer to review this stuff on the upstream repository. But here's a few minor comments

caitp · 2017-01-13T16:08:27Z

test/built-ins/AsyncGenerator/AsyncGenerator-construct.js

+author: Benjamin Gruenbaum <benjamingr@gmail.com>
+esid: pending
+description: >
+  %AsyncGeneratorProrotype% creates functions with or without new and handles arguments


This is mostly about arguments parsing in CreateDynamicFunction

caitp · 2017-01-13T16:09:32Z

test/built-ins/AsyncGenerator/AsyncGenerator-is-subclass.js

+description: >
+  %AsyncGeneratorFunction% is a subclass of Function
+  (The AsyncGeneratorFunction constructor is a standard built-in
+   function object that inherits from the Function constructor. )


The parenthesized bit might not add much, dunno

caitp · 2017-01-13T18:51:19Z

test/built-ins/AsyncGenerator/AsyncGenerator-next-callable.js

+ ---*/
+
+async function* foo() { };
+assert("next" in Object.getPrototypeOf(foo()));


Let's do hasOwnProperty instead here and similar places, rather than "in" --- or even just getOwnProperty and check the property descriptor too

caitp · 2017-01-13T18:52:06Z

test/built-ins/AsyncGenerator/AsyncGeneratorPrototye-constructor.js

+/*---
+author: Sakthipriyan Vairamani (thefourtheye) <thechargingvolcano@gmail.com>
+description: Check if %AsyncGeneratorFunction% function's `prototype`
+             object's `constructor` property is %AsyncGeneratorFunction%


Nit: remove "Check if"

caitp · 2017-01-13T18:54:12Z

test/built-ins/AsyncGenerator/exposed-via-constructor-property.js

+             `.constructor` property.
+---*/
+
+const AsyncGeneratorFunction = async function* () {}.constructor;


Many test262 tests avoid using lexical variables unless specifically testing things related to lexical variables

caitp · 2017-01-13T18:57:23Z

test/built-ins/Symbol/asyncIterator/prop-desc.js

@@ -0,0 +1,17 @@
+// Copyright (C) 2015-2017 the V8 project authors. All rights reserved.


I'd probably rename this file to just "asyncIterator.js" unless this matches the conventions of the other well known symbol tests

This matches the conventions of the async function tests. I don't feel strongly about the name though.

fair enough

…ingr/test262 into AsyncGeneratorExpressions

arai-a · 2017-02-16T20:14:05Z

test/built-ins/Symbol/asyncIterator/name-prop.js

+---*/
+
+
+assert.sameValue(Symbol.asyncIterator.name, '[Symbol.asyncIterator]');


"[Symbol.asyncIterator]" is not a name of the symbol, but the method of %AsyncIteratorPrototype%
https://tc39.github.io/proposal-async-iteration/#sec-asynciteratorprototype-asynciterator

caitp and others added 2 commits January 12, 2017 21:14

Add tests for async generator function expressions

c43a36a

initial work

d7ab9f4

caitp reviewed Jan 13, 2017

View reviewed changes

async generators test

0c017e8

Benjamin Gruenbaum added 2 commits January 13, 2017 17:45

fix test to reject instead of throw

3561cc3

symbol tests more symbol tests feedback by caitp simplify symbol test fix built in tests initial work ORealm -> otherRealmSymbol Error->Test262Error figure out what these tests mean subclass tests 6.3.2.1

test name

93e6855

benjamingr force-pushed the AsyncGeneratorExpressions branch from c149e24 to 93e6855 Compare January 13, 2017 15:46

sasha and others added 10 commits January 13, 2017 17:47

Merge branch 'AsyncGeneratorExpressions' of https://github.com/benjam…

7207e80

…ingr/test262 into AsyncGeneratorExpressions

port thefourtheyes tests

55ecee2

test

fc0a26d

move test

0e1bea0

Merge branch 'AsyncGeneratorExpressions' of https://github.com/benjam…

e1225d8

…ingr/test262 into AsyncGeneratorExpressions

Merge pull request #1 from Linkgoron/AsyncGeneratorExpressions

383a85e

Async generator expressions

%AsyncGeneratorPrototype%.next.[[Call]] exists (is callable).

e7dde08

Merge branch 'AsyncGeneratorExpressions' of https://github.com/benjam…

fced437

…ingr/test262 into AsyncGeneratorExpressions

AsyncGenerator Tests

17c6ab2

benjamingr force-pushed the AsyncGeneratorExpressions branch from fced437 to 17c6ab2 Compare January 13, 2017 17:47

caitp reviewed Jan 13, 2017

View reviewed changes

sasha added 2 commits January 14, 2017 12:53

change %AsyncGeneratorPrototype%.next.[[Call]] exists (is callable) test

1d2a911

Merge branch 'AsyncGeneratorExpressions' of https://github.com/benjam…

ac56fdd

…ingr/test262 into AsyncGeneratorExpressions

caitp force-pushed the AsyncGeneratorExpressions branch from c43a36a to 995b9af Compare January 19, 2017 19:27

arai-a reviewed Feb 16, 2017

View reviewed changes


		var OSymbol = $.createRealm().global.Symbol;

		assert.sameValue(Symbol.asyncIterator, Symbol.asyncIterator);

		@@ -0,0 +1,17 @@
		// Copyright (C) 2015-2017 the V8 project authors. All rights reserved.

		---*/


		assert.sameValue(Symbol.asyncIterator.name, '[Symbol.asyncIterator]');

Conversation

benjamingr commented Jan 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benjamingr commented Jan 13, 2017 via email

Uh oh!

benjamingr commented Jan 13, 2017

Uh oh!

benjamingr commented Jan 13, 2017

Uh oh!

caitp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caitp Jan 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caitp Jan 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

caitp Jan 13, 2017 •

edited

Loading

caitp Jan 13, 2017 •

edited

Loading