Should methods on super types be stubbed in td.object?

[paultyng]

I added this unit test to demonstrate the issue.

I have a supertype and subtype, stubbing the subtype, but supertype methods are not stubbed (and don't even exist).

[paultyng]

I'm not sure what the correct behavior should be here, but if this is working correctly as is, should probably be documented at least.

[searls]

Interesting edge case & patch!

I honestly hadn't thought about this at all, because I don't personally take advantage of prototype chaining very often, and CoffeeScript has (thus far) been the only environment that's made it really easy for me.

Separately, this PR is making me wish that we had an additional test suite in ES6. Perhaps we should add one as an example project and put it in the build. I think I'd want to be sure this "feature" worked for ES6 classes before blessing it with documentation/support.

[searls]

That said I don't see any issue with merging this if it passes the build. Making any fake thing should presumably make the whole thing fake.

I don't have the project pulled down right now -- what was the behavior before you patched object.coffee?

[paultyng]

Here is the travisci job without the object.coffee fix:

https://travis-ci.org/testdouble/testdouble.js/jobs/158768797

[paultyng]

Now that I think about it, that travis run may have had a bug, but basically you would get undefined in the td.when call for the super's method since the method did not exist at all.

[paultyng]

Added a small babel example, had to modify the code a little for it to work. Not sure if there is a lodash equivalent of the method, I looked briefly through the source but didn't see one.

[jasonkarns]

There's _.functionsIn which will give array of own+inherited (enumerable)
properties that are functions. Only thing worth checking is if we care
about including non-enumerable properties. (getOwnPropertyNames returns
enumerable and non-enumerable)

There's also _.keysIn which gives all enumerable own+inherited properties (function or not).

[paultyng]

_.functionsIn isn't in 3.10, but there is _.functions.

I tested _.functions(type.prototype) and _.keysIn(type.prototype) and neither seem to work. Perhaps I shouldn't call it on the prototype? I think they are meant to work on an instance or something?

My understanding is also that ES2015 class methods are not enumerable, so I imagine that's why they don't work.

[searls]

I don't have time to review this thoroughly right now, but I noticed you're using Ava in the babel example. My understanding of Ava is very limited, but I'm worried that its process-splitting mechanism plus the global state td.js carries about the state of the current doubles will be a vector for race conditions and erratic behavior. If you want to ensure a consistent run, I'd recommend switching to another test runner if you're seeing anything like that

[paultyng]

I can set those tests to serial (test.serial(...)) or invoke ava with --serial, although each file would still be concurrent, so yeah, now that I look through the td state management I agree that it doesn't really make any sense for ava to be used with testdouble, so I could switch it to mocha, whichever you prefer.

I mostly used it because its ES2105 support is a bit better than mocha and I had some long running test suites so I've been favoring ava for those for obvious reasons. Currently we used sinon to stub our external dependencies where necessary, I just heard about testdouble.js on the Javascript Jabber podcast and was testing it out on one of our projects.

[paultyng]

I just went ahead and switched it to mocha, modeled after the node example.

[searls]

Thanks @paultyng -- separately I'm very interested to see whether td.js and ava actually would work together, I honestly have no clue, just the worry it might not

[paultyng]

I opened #131 to track that, I'd like to see it as well :)

[searls]

Finally got this pulled down and ran it. Great job wiring everything together. Digging in now

[searls]

Great patch. I added one extra test in 05e97a5 as a sanity check but this seems a-ok to me. Makes me want to go upgrade lodash while we're at it.

[searls]

Landed in 1.6.2. Thanks @paultyng!