Improve readability for failing matchers -- toHaveAttr and more

[jesperronn]

After updating to jasmine 2/jasmine-query 2.0.3 I found it hard to read the error messages.

Consider this example:

Given I have result = $(')

expect(result).toHaveAttr('maxchars', '100'); //looks fine, sunshine scenario

expect(result).toHaveAttr('maxchars', '199); //fails X should render a modal window contents Expected { 0 : HTMLNode, length : 1, prevObject : { 0 : HTMLNode, length : 1 }, context : undefined, selector : 'textarea' } to have attr 'maxchars', '199'. (1)

I can debug much faster if I know that the actual field is found. And if I get an error message stating that the attribute is found, but has an incorrect value.

The outcomes of a failing scenario for toHaveAttr should probably cover:

• if the element is found
• if the attribute is found
• if the element has a value, show the diff

Unfortunately, I am not attaching any pull request. Want to hear your thoughts for this.

/Jesper

[RLovelett]

I could not agree more @jesperronn. I think that some of the failing matcher messages are way to cryptic. It would be appreciated to see this improved.

[inv-vinoth]

I was at the same issue until i stumbled upon the OP's thread.

[jesperronn]

@inv-vinoth what is "the OP's thread"? Please provide a link or an explanation

[inv-vinoth]

@jesperronn I was actually referring to your initial post.

expect($(".help-block")).toHaveText("Company Name is required");

Throws the below fail case.

Expected { length : 0, prevObject : { 0 : HTMLNode, context : HTMLNode, length : 1 }, context : HTMLNode, selector : '.help-block' } to have text 'Company Name is required'.

It would be better if the error/fail messages is more friendly.

[topherfangio]

@RLovelett @jesperronn Is there a reason that the PR associated with this issue (#187) has not been merged yet?

I agree with @inv-vinoth that the failures are very hard to read and I made some modifications to the toHaveClass to cope with it. I am definitely willing to clean up the code and add it to a few more matchers (like toHaveText), but I don't want to jump in and do a bunch of work if the code never gets merged.

Here's the re-written toHaveClass matcher

      toHaveClass: function () {
        return {
          compare: function (actual, className) {
            var result = { pass: $(actual).hasClass(className) };
            var wrapper = $(actual.clone()).wrap("div").empty().text("...").parent("div");

            result.message = result.pass ?
              "Expected " + wrapper.html() + " not to have class '" + className + "'." :
              "Expected " + wrapper.html() + " to have class '" + className + "'.";

            return result;
          }
        }
      },

which produces the following kinds of messages:

Expected <li ng-repeat="tab in vm.tabs" ng-click="vm.selectTab(tab)" data-tab-id="2" class="error ng-scope">...</li> not to have class 'error'.

Note that it strips out all of the nested nodes so that you are only left with the HTML for the node that should actually have the class.

If I made similar changes to the other matchers and made a PR, would it actually be merged?

[travisjeffery]

hey peeps, check my comment here: https://github.com/velesin/jasmine-jquery/pull/187#issuecomment-73359125. i'm definitely down for improving the output of the matchers, i'd just like them to be as consistent as possible so if we could update them all at once that'd be great.

[Blackbaud-PaulCrowder]

I'd also like to request this feature. I often find myself using toHaveVal(), toHaveText() and toHaveAttr() only to see a test fail but with no indication of what the actual value was vs. the expected value. I then find myself having to rewrite my test to check the actual value before I can fix it, so I go from this:

expect(el).toHaveAttr('type', 'text');

To this:

expect(el.attr('type')).toBe('text');

Only to change it back once I've tracked down the problem. It would save a lot of time to just see the actual value in the output.