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

Warn when arity of stubbing or verification doesn't match function.length

Open searls opened this issue 10 years ago • 5 comments
trafficstars

If I do a test double func:

var dog = {
  bite: function (thing) {}
}
var woof = td.replace(dog, 'bite')

Then setting up a stubbing with a different arity should warn:

td.when(dog.woof('mailman', 'spritely')).thenReturn("UH OH") // arity mismatch warning 

This warning should be able to be muted on a case-by-case basis since function .length is unreliable.

td.when(dog.woof('mailman', 'spritely'), {ignoreArity: true}).thenReturn("UH OH") // no warning

searls avatar Sep 07 '15 18:09 searls

Hey @schoonology what do you think of this given your hatred of function.length?

searls avatar Dec 14 '16 19:12 searls

I don't know what value this provides over our other messages (like the new "report" that tells you of missed stubbings). Do you run into this case often?

Schoonology avatar Dec 15 '16 17:12 Schoonology

The value of this is it's one of the (in JS, very very) few places where we can give people a heads up that the contract between a subject and a dependency are out of sync with one another.

Example: a dependency foo(a) is defined through a unit test of a subject bar that invokes foo(a), and everything is happy. Months later, somebody updates foo to take two arguments foo(a,b), but the test of bar will keep testing as a so-called "fantasy test", because it's ignorant of the change in foo's signature. A warning like this would help

searls avatar Dec 15 '16 18:12 searls

I'm inclined to agree with @Schoonology. Many JS functions have a "dynamic arity", or no explicitly defined arity at all. It seems like that feature would implicitly make the argument that "dynamic arity = bad", given the opinionated (for good reason) nature of other limitations in the library.

samjonester avatar Dec 15 '16 19:12 samjonester

@samjonester two reactions:

  1. That's why I'd only suggest a warning plus a per-stubbing option to ignore it
  2. The purpose of td.js is to help design good dependencies, and I'd wager that 90%+ of the time, a "good" function I write for an internal API has a fixed, known arity.

Of course, how I intend for people to use test doubles (designing clean, isolated units that are decoupled from third party APIs) and how people actually use test doubles (mocking every third-party thing they can get their hands on), this warning would be triggered all the time and most often by people who aren't using the library as I intended, which I'm sure would lead to dozens more "this warning is bad, make it go away" issues, like we've seen in the case of "don't verify stubs"

searls avatar Dec 15 '16 20:12 searls