improve to.have.text assertion message for nested elements

[rafmagana]

No description provided.

[jfirebaugh]

Can you please spell out the issue you're trying to fix? Is it that failure messages print the jQuery selection as { Object (0, length, ...) } rather than something more useful? If so, that should be fixed at a higher level, not in a text assertion specific way.

[rafmagana]

Yeah, exactly that, look at this: http://jsfiddle.net/rafmagana/V34cb/2/

I think assertion errors should be more descriptive, at least the errors for to.have.text, because when a test fails it doesn't tell me where the text is expected, it just says { Object (0, length, ...) }, so that makes me write the tests like expect($(element).text()).to.be.equal('text') so that I know what's the expected text, and I think this is a better error:

Expected <div><h1>text</h1></div> to have text "Title"

actually, I'd planned to add:

Expected { Object (0, length, ...) } to have text "Title" but got "text"

but found the former more explicit.

I see the same problem with other assertion errors (like attr), but this is the one that I want fixed first, once we agree on something and you merge this, I'd do the same with the rest of the methods, if you agree.

I didn't find a better way to do it, where would you recommend to fix this? I'll do it. Jus let me know.

[rafmagana]

Bump

[jfirebaugh]

Expected <div><h1>text</h1></div> to have text "Title" is the way it's supposed to work. chai-jquery defines $.fn.inspect to output that <div><h1>text</h1></div> part, and at one point I'm pretty sure that was working -- $.fn.inspect got called, and the html content of the jQuery selection was what you saw in the error message. Not just for to.have.text but for any failing chai-jquery assertion. So when I say "this should be fixed at a higher level", what I mean is, there was probably a change in chai that caused this to regress, and that's what needs to be investigated.

[theghostbel]

I'd like to weigh in as well - that's quite annoying that a lot of chai-jquery matchers in case of failure return message that can't help in understanding why assertion failed. This .text() is one of such examples. That's why in our team we try to use $el.text().shoudl.equal(textToTest)

[jfirebaugh]

Hmm, I actually can't reproduce the problem. I applied this diff, and the test still passes:

diff --git a/test/chai-jquery-spec.js b/test/chai-jquery-spec.js
index 06ed076..cf3f6b7 100644
--- a/test/chai-jquery-spec.js
+++ b/test/chai-jquery-spec.js
@@ -387,7 +387,7 @@ describe("jQuery assertions", function(){
   });

   describe("text", function(){
-    var subject = $('<div>foo</div>');
+    var subject = $('<div><p>foo</p></div>');

     it("passes when the text matches", function(){
       subject.should.have.text("foo")
@@ -400,7 +400,7 @@ describe("jQuery assertions", function(){
     it("fails when the text doesn't match", function(){
       (function(){
         subject.should.have.text("bar");
-      }).should.fail("expected " + inspect(subject) + " to have text 'bar'");
+      }).should.fail("expected <div><p>foo</p></div> to have text 'bar'");
     });

     it("fails negated when the text matches", function(){

This is with jQuery 2.1.0 and chai 1.9.0.

[jfirebaugh]

Okay, I see what's going on. Chai's .objDisplay method has a 40 character limit, beyond which it falls back to some predefined object display logic. I think this is where we need to target a fix -- making .objDisplay more flexible somehow. See e.g. chaijs/chai#166. Also, I would suggest that in the case where objDisplay is called on an object that defines its own inspect method, it should assume that the inspect method knows what it is doing and never truncate or substitute something else for the result. /cc @logicalparadox

[rafmagana]

@theghostbel yes, but that defeats the purpose of chai-jquery, I was doing the same as you, but I think we should fix it.

@jfirebaugh Ok, gotcha, but I don't think this bug has to do with that, because the error I'm having is not shown as [object Object] but as { Object (0, length, ...) }, but I will look into it, thanks.

[rafmagana]

@jfirebaugh actually, I take that back, looks like 40-chars limitation is the problem, so, you're right, this is not a chai-jquery problem but chais itself. Closing this issue.

[logicalparadox]

Chai's .objDisplay has been a pain point for some time. While it is necessary (try logging a node.js request object when asserting property existence), it has not yet been made perfect. Your suggestion for using custom .inspect is the closest thing to consistency that has been mentioned.

If anyone is willing, a PR for Chai in this regard will see fast inclusion.

[rafmagana]

@logicalparadox @jfirebaugh So the logic would be something along these lines?

if(objDisplay['inspect'] === undefined){
   //current logic
 }else{
   str = objDisplay.inspect();
   // and we avoid the "if str >= 40" condition 
 }

is that so?

[jfirebaugh]

I'm thinking you extract this conditional, export it as inspect.aware, and change the existing condition to if (str.length >= 40 && !inspect.aware(obj)).