toEqual returns wrong results for RegExps #259

Closed
slevithan opened this Issue Jul 21, 2012 · 4 comments

Comments

Projects
None yet
4 participants

This should not pass, but does:

expect(/a/gi).toEqual(/a/);

This should not pass, but does:

expect(/a/).toEqual(/b/);

This should pass, but does not:

var regexp = /a/;
regexp.x = true;
expect(regexp).toEqual(/a/);

The reason it should pass is that, IMO, toEqual for regexes should compare only source, global, ignoreCase, multiline, and sticky properties, since those are the only properties that are generally relevant for regex objects. For comparison, that's how QUnit's deepEqual works for regex objects. Also, copying a regex using new RegExp(regexp) does not preserve other properties.

Accepting pull request #234 should fix all of these cases, but I figured my first two examples here, at least, needed their own issue. Even if #234 isn't accepted, expect(/a/).toEqual(/b/) obviously should not pass. This is causing a significant number of my tests to break.

Contributor

infews commented Aug 12, 2012

Thanks for finding this problem. I've made a Tracker story for this issue. You can follow along here: https://www.pivotaltracker.com/story/show/34261329 . Another issue (https://www.pivotaltracker.com/story/show/34261329) has suggested a change of technique for equality, so we'll make sure that this is fixed as well.

Thanks for using Jasmine!

dwt commented Nov 9, 2012

+1 is this merged already? Iave also stumbled about this.

Contributor

ragaskar commented Nov 9, 2012

I'm pretty sure we've merged one of the regex comparison fixes to HEAD, but we haven' t yet cut a new gem. We should be doing that shortly....

Contributor

infews commented May 26, 2013

This is fixed on a branch - matchers_redo. We've moved to a new equals function based on what Underscore does. It's much better overall and handles RegExps (and Dates, etc.) much better than Jasmine's 1.x equals_ function.

Expect this to be on Jasmine master in a couple of days and will definitely be in the next 2.0 pre-release.

infews closed this May 26, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment