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

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

Open
DanKaplanSES opened this issue Jan 8, 2024 · 5 comments
Open

Comments

@DanKaplanSES
Copy link
Contributor

DanKaplanSES commented Jan 8, 2024

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
Copy link
Contributor

fatso83 commented Jan 9, 2024

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
Copy link
Contributor Author

DanKaplanSES commented Jan 10, 2024

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
Copy link
Contributor

fatso83 commented Jan 10, 2024

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
Copy link
Contributor Author

DanKaplanSES commented Jan 11, 2024

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.

@fatso83
Copy link
Contributor

fatso83 commented Sep 15, 2024

I'm still open to this, btw. We just need to find an API that seems meaningful.

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