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

SpreadElement used in ArrayLiteral is lacking tests #671

Closed
avdg opened this issue Jun 11, 2016 · 11 comments
Closed

SpreadElement used in ArrayLiteral is lacking tests #671

avdg opened this issue Jun 11, 2016 · 11 comments
Labels

Comments

@avdg
Copy link
Contributor

avdg commented Jun 11, 2016

I've noticed only 26 indirect test related to testing spread in ArrayLiterals, they are easy to pass if spread is followed only by an identifier. A assignmentExpression is allowed after the spread punctuator in current grammar.

@jugglinmike
Copy link
Contributor

Thanks for the report! It's great to see parsers making use of Test262 and even better to see contributions coming back. (By the way, you may be interested in gh-196 and the proposed solution at gh-655.)

@leobalter and I recently began a coverage audit for ES2015 features, and "spread in array initializers" made our list, too.

I've submitted gh-672 to address this.

@avdg
Copy link
Contributor Author

avdg commented Jun 13, 2016

Thanks! And yeah, I had some doubts about these negative tests. Thanks for clarifying that error will be checked soon.

@jugglinmike
Copy link
Contributor

Apologies: I'm a little confused about your comment. When you say, "some doubts
about these negative tests," are you referring to technically incorrect test
files (either in the master branch or in this pull request)? Or are you
referring to the difficulty of consuming tests that are expected to produce
runtime errors?

@avdg
Copy link
Contributor Author

avdg commented Jun 13, 2016

Uh, I rather thought about the error checking involved into these negative tests.

@avdg
Copy link
Contributor Author

avdg commented Jun 13, 2016

And sorry, this was a bit off topic

@jugglinmike
Copy link
Contributor

No problem--I just wanted to be sure that we don't have any invalid tests!

@michaelficarra
Copy link
Member

@avdg: You may also be interested in #559. I'd love to see UglifyJS start using those tests. I will get them published to https://github.com/tc39/test262-parser-tests ASAP.

@avdg
Copy link
Contributor Author

avdg commented Jun 13, 2016

I think we have enough error messages for now to work on, but yeah, these extra tests could be used as an extra verification tool.

@avdg
Copy link
Contributor Author

avdg commented Jun 13, 2016

@michaelficarra Are there es5.1 specific tests too?

@michaelficarra
Copy link
Member

@avdg No. In my opinion, all of the tests should be kept up to date with the latest working draft. Anyway, the tests are now published at https://github.com/tc39/test262-parser-tests.

@avdg
Copy link
Contributor Author

avdg commented Jun 15, 2016

Should now be resolved in #672

@avdg avdg closed this as completed Jun 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants