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

td.replace multiple property names at once #146

Closed
albertogasparin opened this issue Oct 13, 2016 · 1 comment
Closed

td.replace multiple property names at once #146

albertogasparin opened this issue Oct 13, 2016 · 1 comment

Comments

@albertogasparin
Copy link

I'm wondering if it would be possible to replace multiple property names on a object.
Right now we have to:

td.replace(myObj, 'method1')
td.replace(myObj, 'method2')
td.replace(myObj, 'method3')

It would be nicer:

td.replace(myObj, ['method1', 'method2', 'method3']) // , [...manualReplacements])

Return an array of stubbed functions instead of a single one.

@searls
Copy link
Member

searls commented Oct 14, 2016

At first blush, you're right that the call would be nice:

td.replace(myObj, ['method1', 'method2', 'method3'])

That looks good.

My issue is what the return value should be. At present, td.replace is already a bit overloaded. To recap, depending on what you pass to it, you have:

  • td.replace('a string') -- attempts to do module replacement (Node.js) or throw an error (browser). What you get back is an object of doubles, or a double function, based on what's exported by the replaced module
  • td.replace(anObj, 'aMethod') -- replaces anObj.aMethod and returns the double for easy configuration, also sets up the hooks for that double to be removed at the next td.reset call

So my issue with this proposal is that getting an Array back feels like it muddies the public contract of this function even more in a surprising way. If it returns anything, I think it ought to return the object (myObj), since the caller will know which methods they want to replace.

But, if we were to actually do that, then the difference between td.replace(myObj, […]) and td.object(myObj) would simply be "replace these multiple things" vs "replace all the functions on this thing", and I think that's a pretty narrow line to draw.

If I ask myself "what behavior do I want to encourage with terse syntax", it's definitely that I want people to pass totally fake objects to their subjects so they have clear control over everything that object is doing and there is no ambiguity to the read as to what's real and what's fake. What this feature would do is encourage people to pick-and-choose methods (perhaps only the ones they think the subject needs) and effectively be encouraged by td's API to create Partial Mocks. Partial mocks are really an antipattern and I wouldn't want to aid & abet their creation.

To be honest, now that I have had a chance to reflect on this, the only reason I included the td.replace(obj, 'foo') in td.js was because a lot of people use object properties as a namespacing scheme where module systems aren't available, and so it's perfectly possible that replacing a single property is a valid replacement of an entire dependency.

So, in summary, I'm okay with the awkwardness of multiple td.replace calls like this, because it might prompt conversations like this one where I instead suggest that the test is probably better off with a complete replacement of myObj with a fake by using td.object(myObj). Call it syntactic vinegar

@searls searls closed this as completed Oct 14, 2016
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