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

runtime error should throw properly #85

Open
leobalter opened this issue Aug 23, 2017 · 13 comments
Open

runtime error should throw properly #85

leobalter opened this issue Aug 23, 2017 · 13 comments

Comments

@leobalter
Copy link
Member

This test is relying on $DONE to throw an error. I wonder if the test should have a flags: [async] frontmatter flag or also just throw a custom error instead. Otherwise, there's nothing telling the runner to wait.

https://github.com/bterlson/test262-harness/blob/4fb2a88f95daa01f8c34d6ec49e26e77079bb722/test/collateral/asyncNegative.js#L11

I found this because I'm getting a failure for this test.

@leobalter
Copy link
Member Author

The same flags: [async] should go to the async.js file in the same folder.

@leobalter
Copy link
Member Author

btw, the error is coming from eshost 3.5.0, it doesn't fail on eshost 3.4.3.

@bterlson
Copy link
Member

Async flag seems appropriate here, but I'm intrigued about the eshost 3.5 regression/change of behavior.

@leobalter
Copy link
Member Author

this might need fetching each commit on eshost since 3.4.3 to find the cause. I'm not familiar with it's code and I suspect my time won't be the best fit to solve it. I'm starting a PTO for ~10 days.

@rwaldron
Copy link
Contributor

I can try to do a git bisect

@leobalter
Copy link
Member Author

I did a git bisect, and also found the exact bug happening by forcing installing [email protected] vs 3.5.0

@rwaldron
Copy link
Contributor

$ git bisect bad
492b29e6a65f749d48e3f3025392f50a7231790b is the first bad commit
commit 492b29e6a65f749d48e3f3025392f50a7231790b
Author: Mike Pennisi <[email protected]>
Date:   Wed Jul 26 15:36:12 2017 -0400

    NodeAgent: Execute scripts in dedicated VM

:040000 040000 626625e4b9b5fbbb29443006f2c8ef6060a38f28 282e2e5266fc3bdd047613172996b1dea1111c76 M    lib
:040000 040000 7a3664b71e755d1e561b0f324234ae5ba3a33e51 9e07ea2fc2743b8365365efdd7063c83d4ae19e8 M    runtimes
:040000 040000 7b66a385bc3ba57ebbbdbaad65e60216587dc80f 570c1a969bcae31633c184c42a04da8e743eb1d3 M    test

@rwaldron
Copy link
Contributor

I wonder if the test should have a flags: [async]

Does not correct the issue

@rwaldron
Copy link
Contributor

lol...

setTimeout is not defined

@bterlson
Copy link
Member

@rwaldron are you investigating? I can take this one if you like.

@rwaldron
Copy link
Contributor

I found it and fixed it.

@bterlson
Copy link
Member

Alright! Excellent.

rwaldron added a commit to rwaldron/eshost that referenced this issue Aug 24, 2017
rwaldron added a commit to rwaldron/eshost that referenced this issue Aug 24, 2017
rwaldron added a commit to tc39/eshost that referenced this issue Aug 24, 2017
@leobalter
Copy link
Member Author

@rwaldron back to the original report: should test262-harness bail out on tests that are supposed to be async?

Like, in this case with a setTimeout, should the harness wait for the async completion without a explicit async flag in the frontmatter?

@rwaldron rwaldron reopened this Aug 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants