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

Replace constructors with constructors #193

Merged
merged 11 commits into from
Mar 6, 2017
Merged

Replace constructors with constructors #193

merged 11 commits into from
Mar 6, 2017

Conversation

searls
Copy link
Member

@searls searls commented Mar 3, 2017

iu

Since starting the library, when you've done td.object(SomeConstructorFunction) or used td.replace to replace a constructor, td.js has returned a function bag to the test of the instance methods on the constructor. This PR reverses course, and instead returns a fake constructor instead. It's also the first breaking change to the API since we went 1.0, so releasing this will require a major version bump.

Benefits to you, dear reader:

Stuff to-be-done:

  • calling td.object(SomeConstructorFunction) will return a fake constructor which inherits from it, and for which all its functions are replaced by test double functions
    • ✅ any "static" functions defined on the function itself (e.g. SomeConstructorFunction.create())
    • ✅ any "instance" functions defined on prototype
    • ✅ any inherited static/instance functions from types the constructor extends
    • constructor which should be a reference to the fake constructor itself
    • toString which will tell you it's a "test double constructor"
    • ❌ built-in functions inherited from Function.prototype and Object.prototype (like valueOf, hasOwnProperty)
  • calling td.replace('../some/constructor/exporting/module') will now replace both the actual module with these (new and improved!) fake constructors and return the same fake constructor to the test
  • fix unit tests under node
  • fix unit tests in browser
  • fix tons of example tests
  • update docs
  • plan what we want to bundle in this major bump with other team members (e.g. ES6 port? dropping Node<4? Yarn?)

Required style changes

This means you'll have to sprinkle a prototype onto any existing stubbings in your test code base of constructor-replaced functions. For example:

// production code
var Dog = function Dog()
Dog.prototype.woof = function () {}

// the *OLD* 1.x behavior
var dog = td.object(Dog)
td.when(dog.woof()).thenReturn('bark')
dog.woof() // => returns 'bark'

// the *NEW* 2.x behavior
var FakeDog = td.object(Dog)
td.when(FakeDog.prototype.woof()).thenReturn('bark')
FakeDog.prototype.woof() // => returns 'bark'
new FakeDog().woof() // => returns 'bark'

Conclusion

To be honest, for my own practice 1.x worked better, but it was less obvious and it required me to contort the codebase into knots to facilitate other people's styles, like support for static methods. Whichever style you prefer, I apologize for not having the foresight to have implemented this API in the first place.

Because I understand some people have a lot of existing tests and (as tests are untested), they'll be wary of transitioning them, I plan on maintaining a 1.x-specific release branch with the objective of backporting critical bugfixes.

Fixes #166

@searls searls added this to the 2.0.0 milestone Mar 3, 2017
@searls searls added this to WIP in td.js 2.0 Mar 3, 2017
* Whack newly-unnecessary wrappers "yay!"
* Fixes unit tests under browsers & node
* webpack is still failing
My conservative "wait a year because debugging inside the webpack 
codebase sounds terrible" strategy pays off yet again!
This backgrounding webpack which is definitely NOT what we want
This only showed up in 1 of our 4 or 5 example projects
(webpack) and it wasn't obvious that the root cause was 
the source listing referencing td.

How the heck do I test for this global not to be set when
all the tests set it?? Ughhhhhhh
@searls searls merged commit 47c2db0 into master Mar 6, 2017
@searls searls deleted the 166-constructor branch March 6, 2017 15:34
@searls searls moved this from WIP to Done in td.js 2.0 Mar 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[BREAKING] Return constructor for td.replace()
1 participant