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

Hashbang tests update #2065

Merged
merged 8 commits into from Feb 14, 2019

Conversation

@leobalter
Copy link
Member

commented Feb 5, 2019

This replaces #1983 with new changes

@leobalter leobalter requested a review from rwaldron Feb 5, 2019

@leobalter

This comment has been minimized.

Copy link
Member Author

commented Feb 5, 2019

cc @bmeck

@bmeck
Copy link
Member

left a comment

lgtm

@caitp

This comment has been minimized.

Copy link
Contributor

commented Feb 5, 2019

I'd still be happier if parseTestRecord.py supported this upstream, so that running the tests in v8 only requires a tiny change to support.

@leobalter

This comment has been minimized.

Copy link
Member Author

commented Feb 5, 2019

I'd still be happier if parseTestRecord.py supported this upstream, so that running the tests in v8 only requires a tiny change to support.

Yes, I'm yet to look at that part. Thanks for the reminder.

@leobalter

This comment has been minimized.

Copy link
Member Author

commented Feb 6, 2019

It wasn't easy to find the bug but I finally figure it out. I'm rewriting this python script to something that reflects better the current reality of Test262 tests but providing the same returns not avoid breaking anything else. I'll let you know when it's ready.

]) {
assert.throws(SyntaxError, () => ctor('#!\n_',''), `${ctor.name} Call argument`);
assert.throws(SyntaxError, () => ctor('#!\n_'), `${ctor.name} Call body`);
assert.throws(SyntaxError, () => new ctor('#!\n_',''), `${ctor.name} Construct argument`);

This comment has been minimized.

Copy link
@mathiasbynens

mathiasbynens Feb 7, 2019

Member

nit: missing space after comma

GeneratorFunction,
AsyncGeneratorFunction,
]) {
assert.throws(SyntaxError, () => ctor('#!\n_',''), `${ctor.name} Call argument`);

This comment has been minimized.

Copy link
@mathiasbynens

mathiasbynens Feb 7, 2019

Member

nit: missing space after comma


assert.sameValue(eval('#!\n'), undefined);
assert.sameValue(eval('#!\n1'), 1)
assert.sameValue(eval('#!2\n'), undefined);

This comment has been minimized.

Copy link
@mathiasbynens

mathiasbynens Feb 7, 2019

Member

Let’s try both direct and indirect eval.

@mathiasbynens

This comment has been minimized.

Copy link
Member

commented Feb 7, 2019

re: file names like encoded-bang-041.js, I’d prefer escaped over encoded to match the phrasing used in the spec for escape sequences

LGTM % comments

Apply review feedback
- indirect eval
- files renaming
- space after comma

@leobalter leobalter dismissed stale reviews from mathiasbynens and bmeck via 0084c7c Feb 8, 2019

@leobalter

This comment has been minimized.

Copy link
Member Author

commented Feb 8, 2019

@mathiasbynens I applied the feedback there.

Still working on the python script.

@rwaldron rwaldron merged commit 6e4b434 into master Feb 14, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@leobalter leobalter deleted the hashbang branch Feb 19, 2019

@leobalter leobalter referenced this pull request Feb 19, 2019

Closed

Fix parseTestRecord.py #2080

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.