assertThrows should raise a meaningful error message #21

Open
kgilpin opened this Issue Apr 13, 2012 · 16 comments

3 participants

@kgilpin

assertThrows always raises the same error:

Nodemock rules breaked!

whereas assert writes to the console:

method call for: '" + method + "()' with params: " + getParamString(entry.args) + " was not executed!

Both methods should share the same description. assert should print it, assertThrows should raise it.

@arunoda
Owner

Can you please provide me the sample code when this happens.
So I can debug easily.

@kgilpin

I have forked and reworked the assertions like this:

this.assertionMessages = function() {
    var messages = [];
    for(var method in entries) {
        var entriesForMethod = entries[method];
        entriesForMethod.forEach(function(entry) {
            if(!entry.shouldFail && entry.executed == false) {
                var methodName = self.context ? self.context + "#" + method : method;
                messages.push("Method " + methodName + getParamString(entry.args) + " was not called");
            }
        });
    }
    return messages.length > 0 ? messages : null;
}

this.assert = function() {
    var messages = this.assertionMessages();
    if ( messages ) {
        messages.forEach(function(message) {
            console.error(message);
        });
        return false;
    }
    else
        return true;
};

this.assertThrows = function() {
    var messages = this.assertionMessages();
    if ( messages )
        throw new Error(messages.join("\n"));
};
@kgilpin

In this snippet, context is my other suggestion of a description for the mock.

@realistschuckle

+1 to this issue.

@arunoda, would it help for me to fork the project and implement this as a pull request for you?

@arunoda
Owner
@realistschuckle

That sounds great. I'll happily take over for you since you've moved on to bigger and better things. :)

I happen to like mocking frameworks. I have a proprietary one for python and I contribute to stretcher/testify mocks for golang.

I use realistschuckle as my handle on both Github profile and npmjs profile via autotest. Please add me as a maintainer on npmjs and transfer ownership of the repo to me. I'll give it some attention.

@realistschuckle

You can transfer ownership in the repository's Settings > Options > Danger Zone™ > Transfer button.

@arunoda
Owner
@realistschuckle

Other than leaving the issues behind, it works for me.

@realistschuckle

Oh, before I fork it, would you add a LICENSE to the repo? I want to make sure that everything is clear in terms of usage, since you do not want to transfer ownership...

@arunoda
Owner
@arunoda
Owner
@realistschuckle

No problem. I have patience.

@arunoda
Owner

Okay, I've added it. Send me a pull request to the README. Add anything you feel okay. I think we don't need the docs to be here.

@arunoda
Owner

screen shot 2014-01-29 at 9 08 49 am

@realistschuckle

Thank you for adding me. Happy coding!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment