Sinon: Unsure how / no way to `replace` methods with `fakes`

[akdor1154]

Describe the bug
If I fake a method (a function with a this reference) with sinon.fake, it looks like its this reference will become broken and unbindable.

This makes fakes entirely useless for stubbing methods of OOP style code.

To Reproduce
Steps to reproduce the behavior:

const sinon = require('sinon')
const method = function() { return this.a; }
const fakeMethod = sinon.fake(method);

const o = {
  a: 'asdf',
  method: method,
  fakeMethod: fakeMethod,
  replacedMethod: method,
  explicitlyBoundMethod: method
}

sinon.replace(o, 'replacedMethod', sinon.fake(o.replacedMethod));
sinon.replace(o, 'explicitlyBoundMethod', sinon.fake(o.explicitlyBoundMethod).bind(o)); // i would hope this workaround would be considered unnecessary, even if it did work...

o.method() // 'asdf'
o.fakeMethod() // undefined
o.replacedMethod() // undefined
o.explicitlyBoundMethod() // undefined

Expected behavior
Setup as above.

o.method() // 'asdf'
o.fakeMethod() // 'asdf'
o.replacedMethod() // 'asdf'
o.explicitlyBoundMethod() // 'asdf'

Context (please complete the following information):

• Library version: 6.0.1
• Environment: nodejs 8 on linux from nodesource

[mroderick]

I ran your example with [email protected] and current master in node 8 and node 10 on macOS.
All the lines returned asdf as desired.

Are you still able to reproduce this issue?

[akdor1154]

That's a very strange result, I'm testing at home now (still Linux) on Node 10 with both the REPL and a js file, and I'm still seeing undefined for my test case above. Did you copy-paste my repro directly? (I'm wondering if you hand-reproduced and wrote an arrow function somewhere there might be slightly different binding behaviour). I have a mac at work I can try but I'd probably be even more confused if there were actual platform-specific differences in behaviour here! Sinon 6.1.3 and master (by npm install sinonjs/sinon)

test.js.txt

[akdor1154]

runkit repro: https://runkit.com/akdor1154/5b415c4a4999cd0012bb094d

[rgroothuijsen]

Confirmed the issue on my own machine (Ubuntu 18.04, Node 8.10.0). The explicitly bound method workaround does seem to produce the expected result if used in a slightly different way:

sinon.fake(o.explicitlyBoundMethod.bind(o))

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[akdor1154]

still no way in 6.3.3. @mroderick I'm really confused as to how you managed to get this binding to work, are you able to demonstrate anything in my runkit that's done incorrectly?

[fatso83]

I verified this too, just replacing console.log with assert statements. Tested using 10.2, 9.6, and 8.0

const assert = require("assert");
const sinon = require("sinon");
const method = function() {
  return this.a;
};
const fakeMethod = sinon.fake(method);

const o = {
  a: "asdf",
  method: method,
  fakeMethod: fakeMethod,
  replacedMethod: method,
  explicitlyBoundMethod: method
};

sinon.replace(o, "replacedMethod", sinon.fake(o.replacedMethod));
sinon.replace(
  o,
  "explicitlyBoundMethod",
  sinon.fake(o.explicitlyBoundMethod).bind(o)
); // i would hope this workaround would be considered unnecessary, even if it did work...

assert.equal(o.method(), "asdf");
assert.equal(o.fakeMethod(), undefined);
assert.equal(o.replacedMethod(), undefined);
assert.equal(o.explicitlyBoundMethod(), undefined);

[fatso83]

EDIT: I posted something wrong. Buggy code. Fixed now
~The real problem is that the fake doesn't return the value from the wrapped function. I trimmed down all the fat from the original:~

It doesn't seem to use the outer context in the wrapped function.

const assert = require("assert");
const sinon = require("sinon");
const method = sinon.stub().callsFake(function() {
  return this.foo;
});

const o = {
  foo: "asdf",
  method
};

//o.fakeMethod = sinon.fake(method.bind(o)); // works
o.fakeMethod = sinon.fake(method); // doesn't use `this` when invoked

const results = o.fakeMethod();
assert.equal(method.callCount, 1);
assert.equal(o.fakeMethod.callCount, 1);
assert.equal(results, "asdf");

[fatso83]

OK, found a fix. It was the most obvious fix, but a bug in the driver code seemed to imply it was wrong. Submitting a PR.

[fatso83]

Published as 6.3.4