-
Notifications
You must be signed in to change notification settings - Fork 44
test262 tests #69
Comments
I'd love to help with this but I'm not really sure how to get started. |
+1 I would like to participate also. |
I think a test plan would be the most valuable thing to get started. If you really want to dive in and start writing tests, you can use e.g. https://github.com/tc39/test262/pull/479/files as a guide. |
Here's a starter test plan informed by my experience prototyping. There's plenty of stuff missing, though.
There's a lot of stuff I've left out of this list:
|
I don't think we want to add this kind of direct test to test262. Instead, we only want to test the aspects that are observable from the language, i.e. for-await-of and yield*. If some implementation were able to implement those without every creating a concrete Async-from-Sync iterator class, then that's great, and should be allowed! We shouldn't write test262 tests that they would start failing. Maybe we should even mention in the spec the aspects of Async-from-Sync iterator that are unobservable. It sounds like there are a lot. |
This means that if the sync iterator returns a rejected promise, that rejection will properly be propagated to the async-from-sync iterator. Noted in #69 (comment).
Ok, I have a few volunteers and we'll meet and discuss this this Friday. |
I think it's worth having tests upstream wrt interactions between the async iterator and sync iterator (and whoever is calling the iterator methods), because those are observable. It's awkward to only test it in for-await-of, especially if eventually there is another user of GetIterator(async) user. |
It may be awkward, but I think it's worth writing awkward tests to allow implementations to optimize fully. |
well, by "VM Hooks" I'm not necessarily saying "expose Async-from-Sync Iterator to normal JS", I'm saying "allow VM test harnesses to provide a way to perform this thing, even if normal JS code can't" I'm absolutely in favour of minimizing the cost of the runtime Async-from-Sync iterator proxy, and if subclassing it or manually creating it, or modifying its prototype chain is out of the question, that works for me |
Yeah, but my point is that some implementations might not even be able to expose it, if they've optimized it away super-extensively, and so we shouldn't write tests that those implementations are unable to pass (since they are unable to provide |
Could someone please change @caitp's test plan, a check-list? We can tick them whenever tests land. |
Here's a link to @thefourtheye 's work which we'll build on on Friday (if he won't finish it by then :P) |
I'm going to start writing these tests today, as I believe my prototype has become feature complete today. |
Great, it would be very helpful if you kept a list (like thefourtheye suggested) of tests you've already written and tests you'd like help with - I have 5 hours blocked in my schedule to work on this on Friday and thefourtheye already did some work. |
Runtime
Grammar
|
There's a bunch of things to check off (I know some of these have been covered already). There's a ton more stuff to add to the list, though. |
I wonder if there's a better way to organize this than GitHub checklists, given that those require write access to check. Trello board? Separate shared repo? I dunno, suggestions welcome. |
Going to start working on this now. Separate repo and trello board sound good. |
WIP - doing (updating as I go):
|
This seems to be incorrect per spec actually. It should reject with a TypeError. |
WIP - doing (updating as I go): AsyncGeneratorDeclaration throw a SyntaxError if AsyncGeneratorBody contains SuperProperty |
@domenic ptal at https://github.com/caitp/test262/pull/2/files and let me know if what we're doing makes sense. |
Would you prefer it if we wrote tests for the current spec and update when it updates or would you prefer it if we test it rejects? Also, I'm on IRC if you want more direct comm |
I don't understand the question? The current spec says it rejects. |
@domenic roger, will fix. Question about: async functions don't do this outside of strict mode - do async generators throw in sloppy mode? |
Yeah, you're right, should be written as rejects promise
The current spec says to reject, not throw, and it's likely to stay that way. In async functions/generators, pretty much any instance of "throw" really means reject, other than for early errors |
These are specific to the Method versions, which always have strict formal parameters whether in strict mode or not |
@caitp can you please take a look at the start of the work (as a PR to your fork of test262) and let us know if we're on the right track or not? |
@domenic are async generators subclasses of AsyncFunction or Function? From the spec it looks like they're a direct subclass of |
Function. That's intentional. |
Hey, how can I test this one: AsyncGeneratorDeclaration throw a SyntaxError if BindingIdentifier is "yield" and function is nested inside of a GeneratorDeclaration, GeneratorExpression, GeneratorMethod, AsyncFunctionExpression or AsyncMethod? Is there any specific folder to put this tests? |
Hi, Added tests for: |
We're done for now - @thefourtheye might add more tests later today. All collaborators (me, @sashakru , @Linkgoron and @thefourtheye signed the CLA) and I'll rebase after today so it's all neatly done in a single PR (to @caitp's own PR for test262 for easy validation) |
Great to see more contributors to test262 here! |
@caitp asked that we submit separate PRs to test262 (we'll do so this
Friday). Let us know if there is anything else you'd like us to take a look
at
…On Wed, Jan 18, 2017 at 2:43 AM, littledan ***@***.***> wrote:
Great to see more contributors to test262 here!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#69 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABQSzYB7TLxlBlUHjlEzmYM3QfretJTNks5rTWA8gaJpZM4Le2va>
.
|
Separate PRs seem like a good idea, for licensing at the very least. Let me know if you need any help getting that set up. Looks like there's still a bunch more checkmarks unchecked, so if you're available for it, then just plugging away sounds great. |
I think the checklist is pretty incomplete, anyways :( there's not really much mention of for-await-of or it's interactions with rejected promises or AsyncIteratorClose. I'm sure there's a ton of stuff left to add. |
I'll do more work on Friday to ensure that our PRs are split into separate items - then I'll look at the tests for iterators and for async functions and see what's tested. Hopefully I'll be able to figure it out. |
I don't think it's necessary to split the PRs up to be small. The more important thing is that the PRs are filed against the upstream test262, rather than Caitlin's repository. It's fine for a commit to have tons of tests in it or just one test. |
started writing tests for yield* in async generator |
@benjamingr @thefourtheye would you mind submitting your tests to the test262 repository directly? @arai-a sorry for missing that ping. I will give feedback on the tests on Monday! |
another small chunk for abrupt cases in yield* in async generator. |
and tc39/test262#871 |
added another chunk in yield* tc39/test262#883 |
Was a list ever created to track the remaining tests? |
One of the requirements for stage 4 is test262 tests. Besides being a process requirement, it's also great for interoperability.
Are there people that are willing to help work on this? I know @caitp has been working on an implementation in V8, which comes with its own tests presumably in mjsunit format; apparently it's now possible to write test262 tests directly in V8 for later upstreaming, which would be one route.
Alternately, the community and/or myself could help port @caitp's tests and write our own. @benjamingr seems to have volunteered :)
One valuable thing we can do besides even writing tests is come up with a test plan. That is, go through the spec and write down all the testable things and edge case behaviors that we want to be sure to hit when writing tests. Just getting that done alone will help the test writers a lot.
The text was updated successfully, but these errors were encountered: