-
-
Notifications
You must be signed in to change notification settings - Fork 414
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
Add ECMAScript test suite (test262) #567
Conversation
Can u make this branch vs new_lexer so we can see differences? |
Codecov Report
@@ Coverage Diff @@
## master #567 +/- ##
==========================================
- Coverage 73.17% 72.48% -0.70%
==========================================
Files 193 197 +4
Lines 14029 14181 +152
==========================================
+ Hits 10266 10279 +13
- Misses 3763 3902 +139
Continue to review full report at Codecov.
|
Benchmark for 61cf6e9Click to view benchmark
|
Benchmark for ab00e0aClick to view benchmark
|
Done! :) I did a small hack with the primary expressions, until the parser cursor skips line terminators by default and we can remove (or hide) It seems we get a stack overflow somewhere, but I didn't debug it. Still, we get some tests passing already :) |
That is awesome!!! :D I'm curious how many passed |
Difficult to get stats with the stack overflow, and note that we are still not adding the environment to the tests. We only add the first two files, that are always required, but not the extra files that need inclusion, or the built-in required stuff. But, until the stack overflow, here is what I get: Details
I removed a bunch of errors like this one:
It seems we have a bug xD expressions are divided by commas ( |
could we add some documentation on how to set up and run the Also, this is awesome! |
Benchmark for fa950d4Click to view benchmark
|
Yep, for now, if any of you want to try it, you will need to switch to the branch, run The idea is that the tester should be able to generate a JSON output, with the stats. We can then use that JSON to paint a nice webpage with all of our conformance tests. |
Benchmark for 4afee8eClick to view benchmark
|
Benchmark for f3cbd60Click to view benchmark
|
Benchmark for e585e2cClick to view benchmark
|
I had to add some extra exceptions, but the test suite now runs fully. These are the full results:
Note that they are not very reliable. We might have false positives and false negatives, until we fully implement the test runner. I also added a colorful visualization, that doesn't look super good with all the panics, but it's ok for now xD Red dots are failing tests, green dots are passing tests. |
Awesome! |
Benchmark for 57b90baClick to view benchmark
|
Benchmark for 57b90baClick to view benchmark
|
Still looks awesome! :) Maybe we should have a |
I added some a "ignore" file for tests. I seem to be getting a bunch of stack overflows now, not sure why. |
This is much better than hard coding it :)
That is strange 🤔 |
Benchmark for e15914cClick to view benchmark
|
Benchmark for d126e0fClick to view benchmark
|
Benchmark for bedb1b0Click to view benchmark
|
Benchmark for 2f91ddaClick to view benchmark
|
Benchmark for e799ebcClick to view benchmark
|
Benchmark for e730036Click to view benchmark
|
Benchmark for 79b3b6aClick to view benchmark
|
Benchmark for 6c45f38Click to view benchmark
|
Benchmark for d1d4fe7Click to view benchmark
|
Benchmark for 069949dClick to view benchmark
|
Benchmark for c19c3daClick to view benchmark
|
Benchmark for d8fecc0Click to view benchmark
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Why is the workflow running on this PR though when it should only run on merges to master?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks awesome :)
Everything runs on PRs too, except for the benchmark store. |
Should it be failing ? |
It shouldn't I forgot to change the name of the binary after the change in |
Benchmark for fcfd27bClick to view benchmark
|
Benchmark for 5208d30Click to view benchmark
|
Benchmark for 5e28a1dClick to view benchmark
|
Lets merge this :) |
This Pull Request fixes/closes #12.
It changes the following:
tester
crate, in charge of running the test suite../test262
, which contains the test262 repo.forward_val()
andparse_expr()
in order to properly throw aSyntaxError
, which closes Lexing and Parsing errors should throw aSyntaxError
#539.This is still early work in progress, and it's blocked waiting for #559 to land, and probably other things we didn't notice. I wanted to show you the design, though, to see if you like it. Note that it's currently everything in one file, so this is something we need to put in multiple modules.
This branch is based in the
new_lexer
branch. And the current main blocker is the fact that line terminators are not skipped by default, as it's the case in the master branch.