Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error message when matching null/undefined with toContain. #28

Open
wants to merge 2 commits into
base: jasmine1
Choose a base branch
from
Open

Conversation

rehmsen
Copy link

@rehmsen rehmsen commented Dec 30, 2014

I was recently thrown off by the error message

minijasminenode/lib/jasmine-1.3.1.js:1142
  return haystack.indexOf(needle) >= 0;
                  ^
TypeError: Cannot call method 'indexOf' of null

Since this happened when running a pretty large test suite, I was not immediately sure what had happened, nor which test case had actually failed. After some debugging it turned out that I had called expect(foo).toContain(bar), where foo was null. This pull request provides a better error message in this case:

Failures:

  1) toContain should print helpful error message when matching null
   Message:
     Expected null to contain 'needle'.
   Stacktrace:
     Error: Expected null to contain 'needle'.
    at null.<anonymous> (.../minijasminenode/spec/tocontain_spec.js:3:18)

I am not sure this is the correct repo to fix this in.
I also did not find any unit tests for the matchers, so I was not sure if I should add one for this change.
Please advice.

rehmsen and others added 2 commits December 29, 2014 14:09
Currently, calling
  expect(null).toContain(42);
will throw an error

minijasminenode/lib/jasmine-1.3.1.js:1142
  return haystack.indexOf(needle) >= 0;
                  ^
TypeError: Cannot call method 'indexOf' of null

Since often the actual is the return value of some function, it can be non-obvious to find the defect. This change will show a more clear error message, pointing the user to the line in the test causing the failure instead of the line in the jasmine code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant