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.

Sign up for GitHub

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

Jump to bottom

Bring back assert.raises #663

Closed
jzaefferer opened this issue Sep 13, 2014 · 11 comments · Fixed by #675
Closed

Bring back assert.raises #663

jzaefferer opened this issue Sep 13, 2014 · 11 comments · Fixed by #675
Labels
Category: API Type: Question Ask questions or seek assistance.
Milestone
pre-2.0

Comments

@jzaefferer
Copy link
Member

This is something that came up in #540. throws is problematic in some environments, which the raises alias avoids. We could just add assert.raises = assert['throws']; in the source and add a note about the alias in the documentation for throws. Very little effort to provide a simple workaround when throws can't be used.

For some history, #323 got rid of parsing errors due to QUnit using throws in the first place. The same workaround is still need to use the method in tests.

It would be useful to have a list of environments affected by this. From the PR linked above, I know of Closure Compiler and Narwhal. @jdalton do you know of others?
@jzaefferer jzaefferer mentioned this issue Sep 13, 2014
Closed
@gibson042
Copy link
Member

👍 to APIs that don't use (formerly) reserved words.

@JamesMGreene
Copy link
Member

shrugs Sure. It would be ideal for reducing confusion to only have 1 method name but I guess we can keep both.

Why did we move to throws from raises originally? Just a more globally recognized meaning?

@gibson042
Copy link
Member

It was all #267, right?

@JamesMGreene
Copy link
Member

Gotcha. Spec compliance strikes again!

@leobalter
Copy link
Member

+1 to bring back the raises method on 2.0.0+ if we state we won't care about CommonJS on throws anymore.

@JamesMGreene JamesMGreene added this to the pre-2.0 milestone Sep 22, 2014
@JamesMGreene JamesMGreene added the Type: Question Ask questions or seek assistance. label Sep 22, 2014
@JamesMGreene
Copy link
Member

Seems like we should just do this in v1.x, shouldn't we? Or is there some reason we shouldn't?

My general preference is exposing as much of the v2.x API in the last v1.x release as possible (especially replacements for any deprecated functionality, which this is not) so that upgrading is even easier to do early on, even if the underlying implementation changes dramatically in v2.x.

@jzaefferer
Copy link
Member Author

Yeah, let's do this in 1.16. CommonJS doesn't matter anyway.

jzaefferer added a commit to jzaefferer/qunit that referenced this issue Oct 2, 2014
@jzaefferer
Assert: Add alias for throws called 'raises'
7212d75 
Brings back the original method name, for compatibility with environments
that consider 'throws' a reserved word.

Fixes qunitjs#663
@jzaefferer jzaefferer mentioned this issue Oct 2, 2014
Merged
@leobalter
Copy link
Member

I probably misread this issue, I believed we would rename throws to raises and make throws deprecated. Well, I prefer having only one method, making both existing as a temporary backwards compat.

@jzaefferer
Copy link
Member Author

I wrote on the original ticket:

We could just add assert.raises = assert['throws']; in the source and add a note about the alias in the documentation for throws. Very little effort to provide a simple workaround when throws can't be used.

Was that so ambiguous? I still think this is a good approach.

@leobalter
Copy link
Member

It wasn't ambiguous, that was my fault on misunderstanding what you've said and to not explain better what I wanted.

@leobalter leobalter reopened this Oct 2, 2014
@leobalter
Copy link
Member

sorry for close/re-opening, clicked on the wrong button.

jzaefferer added a commit to jzaefferer/qunit that referenced this issue Oct 3, 2014
@jzaefferer
Assert: Add alias for throws called 'raises'
255e5df 
Brings back the original method name, for compatibility with environments
that consider 'throws' a reserved word.

Fixes qunitjs#663
jzaefferer added a commit to jzaefferer/qunit that referenced this issue Oct 3, 2014
@jzaefferer
Assert: Add alias for throws called 'raises'
cdfb6c2 
Brings back the original method name, for compatibility with environments
that consider 'throws' a reserved word.

Fixes qunitjs#663
Closes qunitjs#675
jzaefferer added a commit to qunitjs/api.qunitjs.com that referenced this issue Oct 6, 2014
@jzaefferer
Throws: Add note about raises-alias
add1675 
Ref qunitjs/qunit#663
@jzaefferer jzaefferer mentioned this issue Oct 6, 2014
Closed
jzaefferer added a commit to qunitjs/api.qunitjs.com that referenced this issue Dec 3, 2014
@jzaefferer
Throws: Add note about raises-alias
85a9535 
Ref qunitjs/qunit#663
Closes #88
@captbaritone captbaritone mentioned this issue Dec 14, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Category: API Type: Question Ask questions or seek assistance.
Milestone
pre-2.0
Development

Successfully merging a pull request may close this issue.

Assert: Add alias for throws called 'raises' jzaefferer/qunit
4 participants
@jzaefferer @leobalter @JamesMGreene @gibson042