-
Notifications
You must be signed in to change notification settings - Fork 573
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
Extract sync/async interfaces from Stripe services #1260
Comments
@SeanFarrow You can write unit tests using the real API instead of stripe-mock without issues. This is not something we have blocked you from doing today and using the normal methods would let you hit the API as expected. I'm likely misunderstanding what is blocking you though. If that's the case, could you provide some examples of what does not work for you? |
@ remi-stripe,
Whilst I could hit the API without using stripe-mock, I don’t really want to do that in unit tests, as this then elevates them to integration tests.
I’m looking to mock out the stripe services to ensure they are called and the return values I expect are handled using something like NSubstitute. Not to mention this, this would then help dependency injection.
Does that clarify things?
From: remi-stripe <notifications@github.com>
Sent: 24 August 2018 19:10
To: stripe/stripe-dotnet <stripe-dotnet@noreply.github.com>
Cc: Sean Farrow <sean.farrow@seanfarrow.co.uk>; Mention <mention@noreply.github.com>
Subject: Re: [stripe/stripe-dotnet] Extract sync/async interfaces from Stripe services (#1260)
@SeanFarrow<https://github.com/SeanFarrow> You can write unit tests using the real API instead of stripe-mock without issues. This is not something we have blocked you from doing today and using the normal methods would let you hit the API as expected.
I'm likely misunderstanding what is blocking you though. If that's the case, could you provide some examples of what does not work for you?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1260 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABY1foZRSEWiKmaZRiYHOaf198K-XiXQks5uUEGFgaJpZM4WLw14>.
|
@SeanFarrow Okay I see what you mean now. I do see how it could be useful but I don't think it's something we'd ship in the near future. It's not something we have in any of the other libraries either today. I'll flag it as Future though in case it's something we could tackle in the future. |
@ remi-stripe,
It’s not that hard, I’m happy to do the work to save you doing it yourself, if a pr would be accepted.
Would probably take me a couple of days max.
From: remi-stripe <notifications@github.com>
Sent: 25 August 2018 01:29
To: stripe/stripe-dotnet <stripe-dotnet@noreply.github.com>
Cc: Sean Farrow <sean.farrow@seanfarrow.co.uk>; Mention <mention@noreply.github.com>
Subject: Re: [stripe/stripe-dotnet] Extract sync/async interfaces from Stripe services (#1260)
@SeanFarrow<https://github.com/SeanFarrow> Okay I see what you mean now. I do see how it could be useful but I don't think it's something we'd ship in the near future. It's not something we have in any of the other libraries either today. I'll flag it as Future though in case it's something we could tackle in the future.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1260 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABY1frWFmpci4pgrujHtfW6Umug_fbgLks5uUJpPgaJpZM4WLw14>.
|
@SeanFarrow We're in the middle of a pretty big refactor of the entire library so it might be worth waiting a few weeks. You can follow along in #1253 and #1252 We would definitely be interested in a PR though! What I'd recommend is to make a simple proof of concept on one service (or even one method) to showcase what you're aiming for and make sure it's the right direction. That way you don't do all the work for nothing Let me know what you think! |
That timescale works for me as I don't need this straight away. I'll create interfaces from a single service as a proof of concept, do you have any suggestions as to what the best one would be? |
@sean-stripe I'd use one of the most common ones like Customer or Charge! |
@ remi-stripe,
Customers looks a goodbet for a proof of concept.
I notice that there are several properties, specifically:
public bool ExpandDefaultSource { get; set; }
public bool ExpandDefaultCustomerBankAccount { get; set; }
that are not used, should these be removed?
From: remi-stripe <notifications@github.com>
Sent: 26 August 2018 01:38
To: stripe/stripe-dotnet <stripe-dotnet@noreply.github.com>
Cc: Sean Farrow <sean.farrow@seanfarrow.co.uk>; Mention <mention@noreply.github.com>
Subject: Re: [stripe/stripe-dotnet] Extract sync/async interfaces from Stripe services (#1260)
@sean-stripe<https://github.com/sean-stripe> I'd use one of the most common ones like Customer or Charge!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1260 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABY1frCMrueaZJEutexIuKGKzH8Rb4JSks5uUe3ugaJpZM4WLw14>.
|
Those properties are definitely still in use! You set them to true to enable expansion of a connected resource. We have some custom code in the service logic that looks for properties that start with |
Why do you need interfaces? All of the services' methods are virtual, and I'm mocking them in my unit tests with Moq successfully. Also, for integration tests, I override |
Whilst doing this is an option, if a method is ever not virtual, or you forget to mock a method and that method performs side-effects, that is then not a unit test.
So onterfaces are still needed in my view, it makes the library confirm to the solid principles.
From: Víťa Tauer <notifications@github.com>
Sent: 03 September 2018 21:28
To: stripe/stripe-dotnet <stripe-dotnet@noreply.github.com>
Cc: Sean Farrow <sean.farrow@seanfarrow.co.uk>; Mention <mention@noreply.github.com>
Subject: Re: [stripe/stripe-dotnet] Extract sync/async interfaces from Stripe services (#1260)
Why do you need interfaces? All of the services' methods are virtual, and I'm mocking them in my unit tests with Moq successfully.
Also, for integration tests, I override StripeConfiguration.HttpMessageHandler with my read-response-from-assembly-resources custom handler.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1260 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABY1fg_TP_T_MN8LLU7NzVx_RjmKdPWkks5uXZDKgaJpZM4WLw14>.
|
Implemented in 20.0.0. |
It would be nice if we could use stripe services in unit tests. I'm happy using stripe-mock in integration tests, but the way things are currently mean I'll have to abstract the stripe services if I want to unit test them.
In the project I'm working on it is mandatory that we test as much as is possible.
The text was updated successfully, but these errors were encountered: