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

Add assert.throws #131

Merged
merged 2 commits into from
Dec 5, 2014
Merged

Add assert.throws #131

merged 2 commits into from
Dec 5, 2014

Conversation

domenic
Copy link
Member

@domenic domenic commented Dec 3, 2014

No description provided.

domenic added a commit to domenic/test262-harness that referenced this pull request Dec 3, 2014
@bterlson
Copy link
Member

bterlson commented Dec 4, 2014

Your example isn't validating that a TypeError is thrown, is it? Seems to only pass one parameter...

@domenic
Copy link
Member Author

domenic commented Dec 4, 2014

Ah damn I forgot to amend that commit before pushing up. Will do when I get back to my computer.

@domenic
Copy link
Member Author

domenic commented Dec 4, 2014

Fixed.

Maybe we should add an arity check (perhaps testing that neither param is undefined? or that both are functions?) to prevent this in the future?

It's a potential mistake for people coming from other frameworks where assert.throws(() => {}) works and asserts that anything at all is thrown. For test262 we don't an imprecise version like that of course, but people might do it accidentally like I did.

@bterlson
Copy link
Member

bterlson commented Dec 4, 2014

Yeah I think a check is good. Possibly just a check for func being defined is sufficient?

@domenic
Copy link
Member Author

domenic commented Dec 4, 2014

Added

@bterlson
Copy link
Member

bterlson commented Dec 4, 2014

Do you think it's best to return after the $ERROR call so it bails out? Might prevent showing spurrious errors.

@domenic
Copy link
Member Author

domenic commented Dec 4, 2014

Oh, I thought $ERROR was equivalent to throw Test262Error...

@bterlson
Copy link
Member

bterlson commented Dec 4, 2014

No, that was the intention for $FAIL. $ERROR is "implementation defined" but may not throw to support reporting multiple errors from the same file.

@bterlson
Copy link
Member

bterlson commented Dec 4, 2014

$ERROR(err) is effectively $DONE(err) I think. We probably don't need both even :-P.

@smikes
Copy link
Contributor

smikes commented Dec 4, 2014

We need both because $DONE also indicates async

On Wed, Dec 3, 2014 at 6:57 PM, Brian Terlson notifications@github.com
wrote:

$ERROR(err) is effectively $DONE(err) I think. We probably don't need both even :-P.

Reply to this email directly or view it on GitHub:
#131 (comment)

@domenic
Copy link
Member Author

domenic commented Dec 5, 2014

Fixed to add return statements.

In light of this I'm less confident about removing $FAIL.

@bterlson
Copy link
Member

bterlson commented Dec 5, 2014

LGTM. Still not sure why FAIL is needed but perhaps we can discuss this once I specify the test262 environment more rigorously.

bterlson added a commit that referenced this pull request Dec 5, 2014
@bterlson bterlson merged commit bf9830e into tc39:master Dec 5, 2014
domenic added a commit to domenic/test262-harness that referenced this pull request Dec 5, 2014
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

Successfully merging this pull request may close these issues.

None yet

3 participants