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

testing ECMAScript parsers #196

Closed
jmdyck opened this issue Mar 13, 2015 · 13 comments
Closed

testing ECMAScript parsers #196

jmdyck opened this issue Mar 13, 2015 · 13 comments

Comments

@jmdyck
Copy link
Contributor

jmdyck commented Mar 13, 2015

I'm wondering if it would be possible to increase test262's usefulness for testing ECMAScript parsers. Presumably, every positive test in the suite is a positive test for a parser, but not so for negative tests. I'm guessing that the negative parser tests are (pretty much?) a subset of the tests marked negative: SyntaxError, but I don't think there's a way to auto-detect which ones.

(According to https://github.com/tc39/test262/blob/master/CONTRIBUTING.md :

To assert that an error is thrown during lexing or parsing, before
any lines of JavaScript are executed, use the following pattern:

/*
 * @negative ^((?!NotEarlyError).)*$
 */

throw NotEarlyError;
var var = var;

But note that (a) this uses the old header syntax, and (b) no "negative:" field in the suite currently uses that pattern, so I suspect it's stale advice.)

What I had in mind is a new field for the file-header (say, "parser-negative:") to designate which tests are negative for parsers. The value for the field could be an indication of roughly at what 'level' the error occurs (lexical, syntactic, early error, others?) to accommodate parsers of different "depths". E.g., when testing a parser that checks for early errors, you'd expect tests matching

 /parser-negative: (lexical|syntactic|early error)/

to cause an error, but when testing a parser that doesn't, you'd only expect

 /parser-negative: (lexical|syntactic)/

tests to cause errors. (I realize that not all parsers would conform to such a categorization of errors, but at least it would be a better starting point than now.)

(I suppose, if tests had references back to the pertinent section(s) of the spec, that might supply the necessary info, e.g. a pointer to a Syntax section vs an Early Errors section. But I haven't heard any discussion of adding such references lately, so I'm doubtful it'll happen any time soon.)

@jugglinmike
Copy link
Contributor

I think this is a great idea, and I'd like to help make it happen! I don't
quite understand the categorization of errors: "lexical", "syntactic", and
"early error", though. My intuition would be to allow for two types:
ReferenceError and SyntaxError, so I'd like to know more about the
distinction you're proposing.

@jmdyck
Copy link
Contributor Author

jmdyck commented May 30, 2015

(The 'negative' field has values 'SyntaxError' and 'ReferenceError', so presumably already handles your intuition.)

The thing is, a SyntaxError can arise in various different ways:

  1. during tokenization;
  2. during parsing; or
  3. when checking for early errors.

Moreover, these can happen "statically" (for the top-level Script or Module) or 4. "dynamically" (for a call to eval() or Function()).

Now, when the test suite is only testing ES implementations, these distinctions aren't that important, because the end result is the same. However, if you're testing something less than a full implementation, the distinctions are important, because your software probably doesn't implement all sources of SyntaxErrors. So if the test is tagged as 'SyntaxError', you want to know if it's a SyntaxError that your software should or shouldn't be raising.

In particular, an ES parser would presumably not raise "runtime" SyntaxErrors (4 above). It might or might not enforce early error rules (3). If you've written a code highlighter for JS, you might not even care about parsing errors (2), but still be interested in tokenization errors (1).

That's the kind of distinction I'm proposing.

@jugglinmike
Copy link
Contributor

Thanks for the explanation. I think I understand most of this now, but one
thing that would help a lot is a set of examples. Could you verify these?

1. during tokenization;

I can't think of an example of an error like this. Maybe source text encoded in
UTF-16? Do you know of any existing tests in this project that demonstrate
this class of error?

2. during parsing;

Would this be something like:

var x = {;

3. when checking for early errors.

For instance:

0 = 0;

and also

let x;
let x;

4. "dynamically" (for a call to eval() or Function()).

As in

eval("!!!");

@jugglinmike
Copy link
Contributor

Although the contribution guidelines now recommend using assert.throws for all runtime errors, this is not always possible. Some tests must assert generation of runtime errors from the global scope, precluding the callback-function-wrapping that assert.throws requires:

This means that even with a new tag like parserError, Test262 will still need a mechanism for asserting uncaught runtime errors.

We could stick with the negative tag, but I would like to propose a clean
break from that and re-implementation as globalError. Beyond being a more
descriptive name (one that more clearly differentiates from parserError),
globalError could be limited to the names of built-in Error constructors (as
opposed to an arbitrary regular expression, which has enabled overly-generic
tests
).

@bterlson I know there are test runners beyond the one included in this
repository, so in some ways the frontmatter format is a sort of API. I still
don't have a good sense for how averse we should be to changing the format,
though. Do you think the benefits make the negative->globalError change
worthwhile?

@michaelficarra
Copy link
Member

This should be fixed by #559. I'm going to move the contents of that PR to https://github.com/tc39/test262-parser-tests shortly.

@michaelficarra
Copy link
Member

@jmdyck You can now find the parser tests at https://github.com/tc39/test262-parser-tests.

@jmdyck
Copy link
Contributor Author

jmdyck commented Jun 14, 2016

While test262-parser-tests improves things for testing ES parsers, I don't think it "fixes" this issue per se. Rather, PRs like #382, #542, and #655 are in the process of addressing it.

@michaelficarra
Copy link
Member

@jmdyck As I understand the request, you were looking for the tests to be marked with the expected error "phase". The published parser tests separate out grammar errors from early errors, so I'd at least call that progress.

@jmdyck
Copy link
Contributor Author

jmdyck commented Jun 14, 2016

I'd call it progress too. (I did say that test262-parser-tests improves things.)

@michaelficarra
Copy link
Member

Oh, sorry, I did not mean to imply that you would not call that progress. I realise now it could have been read that way.

@jmdyck
Copy link
Contributor Author

jmdyck commented Jun 15, 2016

No problem.

@leobalter
Copy link
Member

@jmdyck now that #382, #542, and #655 are merged, is there anything else we should land to fix this issue?

@adrianheine
Copy link
Contributor

I think this can be closed, since the original issue is solved.

@rwaldron rwaldron closed this as completed Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants