Automattic / expect.js

Minimalistic BDD-style assertions for Node.JS and the browser.
2.11k stars 209 forks source link

[Feature] Custom message #18

Open thejohnfreeman opened 12 years ago

thejohnfreeman commented 12 years ago

Would it be possible get a facility for adding a custom message to errors? Maybe an extra parameter somewhere, or an extra chained flag:

expect(this).to.be.ok("oh noes");
expect(that).to.be.ok().msg("oh well");
JamesMGreene commented 12 years ago

+1 for the .msg("custom message") chain on the end, if it's possible — I'm thinking it may have to be put into the assertion function call instead but I haven't verified in the code. That said, I think I would probably prefer that the custom message gets added to the existing message that expect.js already formulates for us.

This is actually somewhat of a pain point for me in terms of not being able to convey enough useful information when tests fail while in a loop — usually I note the index or item being iterated over in my custom failure messages. It makes reproducing it take a bit longer as I have to either add console logging or step through it with a debugger to find the point of failure.

rauchg commented 12 years ago

How about or? Would that make sense?

expect(result).to.be(3).or('incorrect result')

JamesMGreene commented 12 years ago

Seems confusing to me. It makes me think of logical OR, such that it would be doing the following:

result === 3 || result === 'incorrect result'
rauchg commented 12 years ago

expect(result).to.be(3, 'incorrect result') or expect(result, 'incorrect result').to.be(3) ?

thejohnfreeman commented 12 years ago

My preference is for the message at the end, as a parameter to the final chained flag.

JamesMGreene commented 12 years ago

Agreed, at the end IF it can't be chained as a separate .msg(...) function at the very end. Understandable if it cannot be chained.

rauchg commented 12 years ago

Ok in that case we just simply need to add it as a parameter to all the methods of the Assertion object.

aseemk commented 12 years ago

Would love this too, and as an extra parameter to the assertion method makes sense to me too.

Not sure how you would be able to chain after the assertion -- the assertion would fail, but it wouldn't be able to throw right away since it doesn't know the message yet. When would it throw then? On msg()? But what if msg() is never called? Etc.

jamesshore commented 11 years ago

+1

eddyystop commented 11 years ago

+1

sophietk commented 11 years ago

+1 expect(that).to.be.ok().msg("oh well"); would be helpful:

aseemk commented 11 years ago

@guille, I might be able to whip up a PR to add an extra message param to all assertion methods in the next couple of weeks. Would you like me to?

rauchg commented 11 years ago

I'm down. And docs

rauchg commented 11 years ago

Thanks!

onedayitwillmake commented 11 years ago

+1 - Right now if you check for example a jquery object for being empty, you get this giant bomb in your console window. This would so be the right solve for that.

I mean we're throwing errors so having an error msg makes sense

wachunga commented 11 years ago

+1

KyleGobel commented 11 years ago

this was brought up a year go, any progress on anything? I wouldn't even care if i had to do

expect.errorMessage("x object didn't exist");
expect(x).to.be.ok();
expect.errorMessage().clear();

or something, I don't care if it even makes sense, i just want some sort of functionality to add information to failing tests.

djuretic commented 11 years ago

+1

aseemk commented 11 years ago

Sorry, I never got around to this. FYI though that the version of expect in Chai.js supports custom messages — and in a useful way (i.e. with the assertion message too):

http://chaijs.com/guide/styles/#expect

Expect also allows you to include arbitrary messages to prepend to any failed assertions that might occur.

var answer = 43;

// AssertionError: expected 43 to equal 42.
expect(answer).to.equal(42); 

// AssertionError: topic [answer]: expected 43 to equal 42.
expect(answer, 'topic [answer]').to.equal(42);

The only thing I don't like about Chai's expect is that its simple assertions (e.g. .ok, .truthy, etc.) are properties rather than methods. That's a major bummer in my mind, but alas.

Edit: that has been fixed in recent versions of Chai — those things can be called as methods now!

jamesshore commented 11 years ago

Chai's a great tool, but it doesn't work in IE 8—that's why I use expect.js. :-)

skeggse commented 10 years ago

The problem I see with the approach of adding a msg function to the toolkit is that it would have to come before the final clause to work intuitively.

// when equal() is called, if the assertion fails it will throw an error
// immediately, meaning that msg() will never evaluate
expect(43).to.equal(42).msg('43 is not the answer');

That said, the syntax is rather nice. It seems like adding a message parameter to all of the assertion methods would work well, and falls in line with other assertion toolkits.

machineghost commented 10 years ago

:+1:

machineghost commented 10 years ago

BTW, if anyone out there is tired of waiting for an official implementation (it has been 2+ years ...), this feature is pretty easy to add yourself. Just ...

!) Run the following code anywhere after you load expect.js (but before running your tests):

    expect.Assertion.prototype.withMessage = function (message) {
        this.message = this.to.message = message;
        return this;
    }
    expect.Assertion.prototype.assert = function (truth, msg, error) {
        var msg = this.message  || this.flags.not ? error.call(this) : msg.call(this),
            ok = this.flags.not ? !truth : truth;
        if (!ok) {
            throw new Error(this.message);
        }
        this.and = new expect.Assertion(this.obj);
    };

2) Add a call to "withMessage" (passing in your message) before the assertion part of your expect statement. For instance:

 expect(false).withMessage('foo').be(true);

Hope that helps someone.

P.S. expect.js developers, I'd be happy to submit a pull request if you'd like to add this code officially.

holic commented 9 years ago

:+1:

jifeon commented 9 years ago

@rauchg Any news here? It would be great feature!

guileen commented 9 years ago

@aseemk :+1: Any news?

aseemk commented 9 years ago

@guileen: I moved to Chai, which already supports this. See https://github.com/Automattic/expect.js/issues/18#issuecomment-27189362. =)

guy-mograbi-at-gigaspaces commented 9 years ago

+1 - we really need it. we have multiple expects in a single it, and we're using browserify - so the line number changes.. it is nearly impossible to understand what failed.. we really need this feature. currently using the work around suggested above. If this is so simple to implement I can't understand what's the hold up.

guy-mograbi-at-gigaspaces commented 9 years ago

FYI - we found we need to tweak the code above. this is our version.

// add custom message to expect.js
// https://github.com/Automattic/expect.js/issues/18
expect.Assertion.prototype.withMessage = function (message) {
    this.message = message;
    return this;
};
expect.Assertion.prototype.origAssert = expect.Assertion.prototype.assert;
expect.Assertion.prototype.assert = function (truth, msg, error, expected) {
    try {
        this.origAssert( truth, msg, error, expected);
    }catch(e){
        if ( this.message ){
            throw new Error(this.message);
        }
        throw e;
    }
};

However this is not a pull request level code, but it works.

for example expect(1+1).to.withMessage('wrong result').be(3); - with message must come before the last once in the chain. not as pretty as we'd like.

rikurb8 commented 9 years ago

Would be real nice :+1:

rafis commented 8 years ago

@guy-mograbi-at-gigaspaces, doesn't work with .withMessage('message').to.be.ok() and .withMessage('message').to.be.an(Object) and similiar.

Meanwhile you can use:

try {
  expect(obj).to.be.an(Object);
} catch(err) {
  expect().fail('My custom error message ' + expect.stringify(obj));
}
guy-mograbi-at-gigaspaces commented 8 years ago

@rafis try: to.withMessage('message').be.ok() - not as clean, but works for us so far.

StreetStrider commented 8 years ago

:+1: for .msg(). The workaround to achieve this functionality is quite ugly.

SCLeoX commented 8 years ago

+10

Since +1 not work well

tortila commented 7 years ago

:+1:

tsheaff commented 5 years ago

Is this still not resolved at all? There's no way to be more specific in your error message about what specifically failed?