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

Where we recieve no exception in raises() use a relevant message. #266

Closed
wants to merge 4 commits into from
Closed

Where we recieve no exception in raises() use a relevant message. #266

wants to merge 4 commits into from

Conversation

alexjeffburke
Copy link
Contributor

Currently in qunit if you write an assertion as follows:

raises(function() {
...
// raises no exception
});

the message you receive in qunit says your return value does not match null. I stumbled on this while writing unit tests for some code where I work and it caused a short head scratching moment until I looked in the qunit code.

Please let me know if you'd prefer me to open an issue and debate a solution there - since this is my first attempt at a contribution to qunit I'm happy to refactor / rework the solution as necessary.

Add an additional optional argument to pushFailure() which can be used to
write a message corresponding to what occurred.

In the raises assertion if actual is undefined it means no exception
occurred; since we were expecting one in this case use the modified
pushFailure() to notify the user.
@@ -790,6 +792,10 @@ extend( QUnit, {
output += "<table><tr class='test-source'><th>Source: </th><td><pre>" + escapeInnerText( source ) + "</pre></td></tr></table>";
}

if ( actual ) {
output += "<table><tr class='test-actual'><th>Result: </th><td><pre>" + escapeInnerText( actual ) + "</pre></td></tr></table>";
Copy link
Member

Choose a reason for hiding this comment

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

For consistency with other output, maybe put Result above Source.

@alexjeffburke
Copy link
Contributor Author

Thanks for the suggestion - I reordered actual above source and also took a cue from QUnit.push() to avoid repeating the opening/closing table tags in the output.

Regarding the message displayed, I'd originally chosen "No exception was raised." but am now in two minds myself about whether it should say raised or thrown. Do you have a preference?

@Krinkle
Copy link
Member

Krinkle commented Jun 26, 2012

I prefer using "thrown" or "caught". But the QUnit method is named "raises" instead of "throws" ([1]) because of reserved word restrictions.

Although looking back, I don't think that is correct (see also #267).

@jzaefferer
Copy link
Member

Its now QUnit.throws, so let's get this updated to match that.

@alexjeffburke
Copy link
Contributor Author

I was very inclined to say "thrown" also - and with the method name change it all seems consistent. Just pushed a small revision which changes the output message to read "No exception was thrown."

@jzaefferer
Copy link
Member

Thanks! Rebased into f4f91c8

@jzaefferer jzaefferer closed this Jun 29, 2012
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.

3 participants