testdouble.js icon indicating copy to clipboard operation
testdouble.js copied to clipboard

Contains matcher with array does not work as expected

Open Schoonology opened this issue 6 years ago • 3 comments

Upon re-reading the documentation, I figured out what I was doing wrong. That said, I found it surprising that td.matchers.contains(['some', 'array', 'elements']) did not work, even though both td.matchers.contains('some', 'array', elements') and td.matchers.contains({ object: 'with', keys: 'and values' }) do work.

That said, there was some other issue, too, and just doing td.when(spawn('npm', ['test'], { cwd: '...' })).thenReturn('fake child process') worked as originally originally expected. 🤷‍♂️

Far from urgent, but would love to get others' thoughts.

Schoonology avatar Jun 07 '18 00:06 Schoonology

This incongruity throws me off too. The API is so liberal in what it accepts that I don't know if it can satisfy all cases as it currently exists.

Be conservative in what you send, be liberal in what you … LOL

Here's what contains does now:

  • Receive a string, match against a string, assume the string should contain the substring (e.g. td.matchers.contains('foo').__matches('barfoo'))
  • Receive a regex, match against a string, assume the string should match the regex (e.g. td.matchers.contains(/\d/).__matches('A1'))
  • Receive multiple string arguments, match each against a string, assume the string should contains all substrings (e.g. td.matchers.contains('foo', 'bar').__matches('barfoo'))
  • Receive an element, match against an array, assume the array should contain the element (e.g. td.matchers.contains(3).__matches([1,2,3]))
  • Receive multiple element args, match against an array, assume the array should contain all elements (e.g. td.matchers.contains(3,2).__matches([1,2,3]))
  • Receive an object, match against an object, assume the second object should contain everything deeply specified in the first (e.g. td.matchers.contains({a: 1}).__matches({a: 1, b: 2})). This caused the addition of other more ridiculous examples:
    • Receive a date, match against a date, assume the times should be equal (e.g. `td.matchers.contains(new Date(2018)).__matches(new Date(2018)))

So why not two arrays?

So, usually equality means containing in the above (as is the case for matching strings, objects, dates, and even regex), but in the case of arrays, it doesn't. If we change this so we can say.

Here's my best rationale for why the following returns false:

td.matchers.contains([3, 2]).__matches([1,2,3]) // false!

My thinking when I wrote this was "well, this is unfortunate and will be hard to document, but with Arrays, order expressly matters, so this should be a variadic function and we should treat each arg as a separate test of the actual". Worse, if we were to make this change, it'd make some cases a bit ambiguous, like:

td.matchers.contains([3, 2]).__matches([1,2,[3,2]]) // true!
td.matchers.contains([3, 2]).__matches([3,2]) // also true! hope this isn't a false positive!

That said, who am I to talk? I mean look at this actual behavior:

td.matchers.contains(45).__matches({a: 45}) // false
td.matchers.contains([3, 2]).__matches({a: [3,2]}) // true. Wat?!

What do you think we should do?

searls avatar Jun 08 '18 17:06 searls

Thinking through the cases you brought up, I think the existing behaviour might be the least surprising overall. If we do anything, maybe we can add a more visible warning to that section of the documentation: https://github.com/testdouble/testdouble.js/blob/master/docs/5-stubbing-results.md#arrays

NOTE: Calling td.matchers.contains([1, 2]) tests that the array contains the [1, 2] within itself, and not that the array contains the elements 1 and 2.

var smoosh = td.function()

td.when(smoosh(td.matchers.contains(['of', 'arrays']))).thenReturn('smooshed')

smoosh(['array', 'of', 'arrays']) // undefined
smoosh([['array'], ['of', 'arrays']]) // smooshed

For more information, please see this issue.

Schoonology avatar Jun 11 '18 18:06 Schoonology

:100: I'd love a PR to that effect.

searls avatar Jun 11 '18 20:06 searls

Stale. Closing. Please reopen if still relevant and I will look into it.

giltayar avatar Sep 16 '23 11:09 giltayar