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

matchers.contains(number) support #102

Closed
focusaurus opened this issue Apr 27, 2016 · 6 comments
Closed

matchers.contains(number) support #102

focusaurus opened this issue Apr 27, 2016 · 6 comments

Comments

@focusaurus
Copy link

Given this repro snippet

const td = require('testdouble')

const repro = td.function()
repro([1])
td.verify(repro(td.matchers.contains(1)))

I'd like this to succeed, but currently it throws an exception "Error: Error: testdouble.js - td.matchers.contains - this matcher only supports strings, arrays, and plain objects"

I think to fix this is just a matter of or _.isNumber(containing) on

if _.isString(containing)

@searls
Copy link
Member

searls commented Apr 27, 2016

Ah, the issue is that you're saying that [1] should contain 1, and that doesn't make a lot of sense. Have you tried td.verify(repro(td.matchers.contains([1])))?

@searls searls closed this as completed Apr 27, 2016
@focusaurus
Copy link
Author

you're saying that [1] should contain 1, and that doesn't make a lot of sense.

It makes perfect sense to me.

Here's two examples. The ONLY difference is replacing the string 'one' with the number 1. I don't see any reason why both of these should not behave identically. The first one works fine, the second one throws an exception.

const td = require('testdouble')

const withString = td.function()
withString(['one'])
td.verify(withString(td.matchers.contains('one')))

const withNumber = td.function()
withNumber([1])
td.verify(withNumber(td.matchers.contains(1)))

@searls
Copy link
Member

searls commented Apr 29, 2016

Ugh, I really apologize. I shot from the hip and speculated when I replied above because I was on a machine without a dev environment, but now that I can test it out locally you're completely write. The truth is this feature doesn't work how I thought it did.

I'm sort of annoyed with myself for not finding this API quirk before going 1.0, because it's not how I want it, but it's also not worth breaking backwards-compatibility.

Naturally, this mistake means I need to accept any kind of input, including null/undefined/true, etc. I'll take a stab at this now.

@searls searls reopened this Apr 29, 2016
@focusaurus
Copy link
Author

Yeah I can give it some more thought in terms of what the ideal API/behavior would be, but I think at the moment it's not self-consistent. Could go more explicit and tighter matching or more loose and flexible matching I guess.

searls added a commit that referenced this issue Apr 29, 2016
Thanks a lot to @focusaurus for his patience!
@searls
Copy link
Member

searls commented Apr 29, 2016

Check out the update in 1.4.2 and let me know if that works for you

@focusaurus
Copy link
Author

Yes, no more exception! Thanks for the quick fix. Been getting a lot of mileage out of testdouble recently.

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

No branches or pull requests

2 participants