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

Assert: Add alias for throws called 'raises' #675

Merged
merged 2 commits into from
Oct 6, 2014

Conversation

jzaefferer
Copy link
Member

Brings back the original method name, for compatibility with environments that consider 'throws' a reserved word.

Fixes #663

Also moves some tests for assert to its own file, in a separate commit. The new raises test is at the end of test/assert.js.

The jshint exception is necessary to make it shut up about assert[ "throws" ] not using dot notation.

@mgol
Copy link
Member

mgol commented Oct 2, 2014

The jshint exception is necessary to make it shut up about assert[ "throws" ] not using dot notation.

It's not necessary; jsHint haven't barked on throws since May: jshint/jshint#1659

// Provide an alternative to assert.throws(), for enviroments that consider throws a reserved word
(function() {
/*jshint sub:true */
Assert.prototype.raises = Assert.prototype[ "throws" ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based on what I got from #663, we should entirely replace throws for raises and invert the statement above to create a backwards compatibility copy of the main raises into throws, making the former a deprecated method until 2.0.0.

@jzaefferer
Copy link
Member Author

It's not necessary; jsHint haven't barked on throws since May: jshint/jshint#1659

I'm not using dot notation to make the code compile in problematic environments, where throws is considered a reserved word. jshint was complaining about that, not about the usage of the reserved word.

Also I updated this to mention the problematic envs we know about (2nd commit).

@jzaefferer
Copy link
Member Author

@leobalter @JamesMGreene anything else?

@JamesMGreene
Copy link
Member

Since you extracted the assertion tests, please be sure to update the Gruntfile to run that test script on Node as well ("test-on-node" task).

Other than that, LGTM.

Brings back the original method name, for compatibility with environments
that consider 'throws' a reserved word.

Fixes qunitjs#663
Closes qunitjs#675
@jzaefferer
Copy link
Member Author

Updated. Edited first commit and squashed the third one into the second. This should be good to land as-is.

@leobalter
Copy link
Member

LGTM. :shipit:

@jzaefferer jzaefferer merged commit cdfb6c2 into qunitjs:master Oct 6, 2014
leobalter pushed a commit that referenced this pull request Oct 27, 2015
The assert.raises method is an alias for assert.throws removed on
QUnit 1.15.0 and partially reintroduced on QUnit 1.16.0. See #675.

Since then, this method was not exposed to the global scope as the
other QUnit assertion methods. This is now fixed and tested on this
commit.

Fixes #883
Closes #884
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Bring back assert.raises
4 participants