testdouble / testdouble.js

A minimal test double library for TDD with JavaScript
MIT License
1.42k stars 142 forks source link

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

Open searls opened 9 years ago

searls commented 9 years ago

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 commented 7 years ago

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

Schoonology commented 7 years ago

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?

searls commented 7 years ago

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

samjonester commented 7 years ago

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.

searls commented 7 years ago

@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"