sinonjs / sinon

Test spies, stubs and mocks for JavaScript.
https://sinonjs.org/
Other
9.6k stars 769 forks source link

Feature Request: Add API support to stub a parent class constructor. #2578

Open DanKaplanSES opened 6 months ago

DanKaplanSES commented 6 months ago

Is your feature request related to a problem? Please describe. I've seen a lot of questions asking how to stub a parent class's constructor. @fatso83 provides a solution here, but it is written in plain JavaScript. Unless you are searching the sinon github repository, this solution is hard to find.

It's written in vanilla js, meaning the prototype's lifecycle must be managed by the developer: they have to undo the Object.setPrototypeOf change or the child class's prototype will remain modified, causing test pollution.

Describe the solution you'd like I would like sinon to provide a way to stub a parent class constructor with its API. That way, sinon can manage the prototype lifecycle like it manages other test double lifecycles.

Describe alternatives you've considered

  1. @fatso83 provides a solution here in plain JavaScript.
  2. Perhaps the API of sinon.createStubInstance could be expanded to support this use case?

Additional context

fatso83 commented 6 months ago

What about just wrapping the code I provided in a utility called sandbox.stubParentConstructor? I am not really feeling I am contributing all that much with this, but you asked about naming 😄 Not sure what else I can help with.

Just a tip: try writing the docs for the feature (describing the API, the context and how to use it) before writing the code and tests. It helps in solidifying a good API.

DanKaplanSES commented 6 months ago

What about just wrapping the code I provided in a utility called sandbox.stubParentConstructor? I am not really feeling I am contributing all that much with this, but you asked about naming 😄

That does help, actually. Thanks.

Not sure what else I can help with.

You've already helped me a lot, but I'm also looking for feedback on the premise of this proposal. The contribution guide says:

Pick an issue to fix, or pitch new features. To avoid wasting your time, please ask for feedback on feature suggestions with an issue.

I don't have a lot of GitHub knowledge/experience; have you given me the feedback I need to attempt this or should I get signoff from someone else first?

Just a tip: try writing the docs for the feature (describing the API, the context and how to use it) before writing the code and tests. It helps in solidifying a good API.

Good advice. By just thinking about the first step, I'm wondering if it should be stub.stubParentConstructor instead of sandbox.stubParentConstructor. The former may not be possible and I don't like the stub.stub... anyway. :T

fatso83 commented 6 months ago

I'm wondering if it should be stub.stubParentConstructor

I am not totally sure which parameters are supposed be passed into stubParentConstructor, but I don't think this looks right. Usually you capture a reference to the stub when you do something fancy with it:

const stub = sinon.stub();
const result = stub.stubParentConstructor(FooConstructor)

What would result hold? Is it different than stub? Makes for a confusing API, IMHO, so better stuff it at the sandbox level.

DanKaplanSES commented 6 months ago

I'm wondering if it should be stub.stubParentConstructor

Makes for a confusing API, IMHO...

After reading your thoughts, I 100% agree. Please ignore that idea.