cleishm / jsmockito

Javascript mocking framework inspired by the awesome mockito
http://jsmockito.org
Other
106 stars 19 forks source link

Feature Request: wrap verification error messages #9

Closed timezra closed 11 years ago

timezra commented 12 years ago

Fantastic library! Thank you for contributing it to the community.

I am currently using JsMockito with JsTestRunner and have run into the following bug (in JsTestRunner): http://code.google.com/p/js-test-driver/issues/detail?id=338 I am hoping that you would be willing to make a minor change to JsMockito to help in this situation. Namely, if you could change lines 132 and 163 from throw description.get(); to throw { message: description.get() };

I realize that JsTestRunner should have no problem handling thrown Strings, but there is an argument to be made for throwing Errors in JS as Objects with a "name" and "message" attribute.

Currently, I have made this change locally, and JsMockito and JsTestRunner work well together. Without this change, the two will not work for me. Hopefully, the JsTestRunner bug will be fixed soon, but in the meanwhile, it would be terrific if you could enhance JsMockito for this usage scenario.

Thanks again for a great tool!

cleishm commented 12 years ago

When I get some time, I'm going to do a little work repackaging this to better support some of the newer test frameworks. That'll likely be one of the changes I make.

gregjacobs commented 12 years ago

It would be very useful if JsMockito threw actual Error objects, rather than String objects as errors. The Error object has many properties that are set by the browser which provide useful information, such as the stack trace and line number.

The actual throw statement should be:

throw new Error( description.get() );

However the most important reason for the change should possibly be because some unit testing frameworks expect an actual Error object when an error is thrown; not a String object. For example, YUI 3 Test looks for the message property on the thrown object, and consequently simply outputs 'undefined' when a test fails from a JsMockito assertion error.

Consequently, in tests that I write with JsMockito, I'm forced to do this:

try {
    JsMockito.verify( someMock ).someMethod();

} catch( errorMsg ) {
    Y.Assert.fail( errorMsg );
}
is24-jdannert commented 12 years ago

We are using qunit-1.10.0.js for our js tests and we have the same problem.

} catch( e ) {
QUnit.pushFailure( "Died on test #" + (this.assertions.length + 1) + " " + this.stack + ": " + e.message, extractStacktrace( e, 0 ) );
...
}

So please throw an error and not a string.

zkhalapyan commented 11 years ago

Hi @chrisleishman, is there any update on this issue? The error that I get within QUnit report is very abstract. For example, see below:

Died on test #2     at http://localhost/og/test/models/reminders/ReminderModelTest.js:74:1: undefined

Here is the message I get with a try-catch statement within my QUnit test case:

2. Wanted but not invoked: obj.cancel(<equal to 0>)
   Source:  
    at Object.<anonymous> (http://localhost/og/test/models/reminders/ReminderModelTest.js:95:9)