Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Improve readability for failing matchers -- toHaveAttr and more #184

Open
jesperronn opened this Issue · 7 comments

6 participants

@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.

@jesperronn jesperronn referenced this issue from a commit in jesperronn/jasmine-jquery
@jesperronn jesperronn Improved error messages for toHaveAttr and make it clear to find root…
… cause of failure.

Different error messages if

 * element not found
 * attribute not found
 * incorrect attribute value

See issue #184
456e24b
@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
Collaborator

hey peeps, check my comment here: #187 (comment). 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.