Openfoodnetwork: [Spike] Automate Stripe testing

[lin-d-hop]

Description

As a tester and developer I want to know if Stripe testing can be fully automated.

Currently testers test that new enterprises can be connected to a Stripe account and this account can then be used to take payments in every release. This is the main automation win for testers.

Acceptance Criteria & Tests

To give an overview - the following is a list of all the places users/enterprises interact with Stripe in the platform:

1. Enterprise Owners/Managers can take payments from backoffice Orders -> Payments
2. Shoppers can save cards from Accounts interface
3. Shoppers can give permission for enterprises to charge their cards in Accounts interface
4. Shoppers can buy with cards at Checkout
5. Shoppers can save cards at Checkout
6. Enterprise Owners/Managers can create subscriptions with cards that a shopper has previously saved and given permission for the enterprise to charge from
7. Enterprise Owners/Manager can refund - partially and fully - credit owning to shoppers.

This is a spike so no testing required.

[Matt-Yorkley]

Am I right in thinking the plan is something like this?

• Use a Stripe testing account to create a dedicated API key that we will use exclusively in the CI build
• Add the API key to the test suite
• Write new specs that actually make real calls to Stripe, using the various "test cards" from their documentation

[luisramos0]

Hey Matt, we dont have a plan. Kristina was assigned to this just before she stopped contributing regularly and I have never really had a look at it.

Your plan is one option.
I found this now: https://github.com/stripe/stripe-mock

[luisramos0]

@mkllnk mentioned a few tools they use in CERES? VCR? I think now is the right time for us to decide what's the tech approach to this challenge. First we need a list of options :-) @mkllnk can you comment? Thanks!

[mkllnk]

Well, Ceres Fair Food doesn't use Stripe. The used payment gateway has simple API calls without webhooks. But my idea to got forward would be:

• Use VCR to record and playback API interactions from Ruby side. So if we call the Stripe API, we have to do that once to record the response. The response is stored in a file in the repository and automated tests will use it to replay that response without ever needed to contact Stripe. If you want to test the real interaction with Stripe again, just delete the file and it gets re-recorded.
• Use Puffing Billy to mock API calls within the browser. It allows us to simulate Stripe's behaviour when a user enters a new card.
• Use BrowserStack automation with a real staging server to test live staging payments. I wouldn't do live payments in CI because CI should be fast, independent and reliable. Automated release testing on staging is easier and closer to production.

[luisramos0]

great, thanks Maikel!

I see that we are already using webmock for most of our stripe tests. Webmock is mostly used for our controller tests but some of our feature tests are using webmock already. I wonder if all we need to do is to add some more compreensive checkout feature specs for stripe that use webmock.

I think the difference between webmock and vcr is that VCR will call the real endpoint and store the response while using webmock we have to write down the response ourselves. After having a look now, I dont think that VCR would bring a lot more than what we can do with webmock.
I think our challenge is really to write the more advanced tests that the new SCA workflows involve with redirects, etc.
Thoughts?

[mkllnk]

• Yes, just webmock is a solution.
• VCR works with webmock, there's not problem in having both (as we do at Fair Food).
• VCR makes it easier to re-record interactions with a live API. This makes it much easier to figure out problems after an API update.
• We will need even more tools to properly test all SCA workflows. Stripe may send an email or text which we need to receive and click on a link or verify in some other way. I hope that some best practices will evolve that we can re-use.

[Matt-Yorkley]

I think in most cases the code on our end is just looking at the response code and checking basic fields like error in the response, right? It looks like it would be quite easy to do with Webmock :+1:

[luisramos0]

This is how we are doing it right now here

I wonder if it's just a matter of expanding these tests to cover more scenarios.

We would also have to do this type of stubbing in feature tests to test the redirects.

[Matt-Yorkley]

Yes, I think that's what we need. We have these issues where a card is not accepted, the stripe response seems reasonable, but then something weird happens on our end with persisting orders or payments. Some feature tests with stubbed responses on rejected payments and then checking our order and payment objects would be really great.

[luisramos0]

:ok_hand: we have a plan. This will be top of dev ready again very soon :+1:

[luisramos0]

this is related to #5014

[Matt-Yorkley]

This epic is moving into dev ready, so it would be good to clarify things a bit. I feel like there's potentially a few levels of complexity here and there's definitely multiple solutions. It might be that we ultimately need multiple "phases" of implementation here, with different solutions.

I'm basically imagining this epic on an ease-value matrix where we lump the simpler work into phase one (which can go into dev-ready this week), and the more complicated work into phase two (which we can argue about later :wink:).

As a rough plan for phase one:

• Start by creating some basic feature specs for checkout processing with Stripe, using request-stubbing as Luis has done here, or possibly with the stripe-ruby-mock gem (see below)
• Test some examples like a card being rejected for insufficient funds during checkout, and add coverage for things like the behavior of associated payment objects after a failed payment, etc.
• Expand the examples and see how far we get

Maybe phase two would cover things like subs and webhooks, and might require something beyond Webmock? How does that sound?

[Matt-Yorkley]

I've seen the stripe-ruby-mock gem recommended in multiple places for Stripe testing in Rails, and there's a guide for writing specs with it here. It provides lots of helpers for mocking Stripe behavior, and has pre-defined fixtures for various Stripe responses that can be called easily.

...this library overrides stripe-ruby's request method to skip all http calls and instead directly return test data. This allows you to write and run tests without the need to actually hit stripe's servers.

[luisramos0]

Nice. I think we can do two things as part of this issue:

• extend spec/requests/checkout/stripe_sca_spec.rb to cover more card types and special cases (like a card that requests extra auth only on the first payment)
• create spec/features/consumer/shopping/checkout_stripe_spec.rb and write feature tests with webmock as the request spec above.
  In these feature spec, I think we should include the redirect cases (cards needs extra SCA approval) where the stripe response includes a link that the app must follow and I think we should have a couple of tests for this redirect, at least for the simple cases of the card that needs extra sca auth and is rejected and the same card that is approved.

I'd go with webmock only, a new dependency stripe-ruby-mock may be an over kill (I think we would need to keep it updated according to stripe upgrades).
Subs can be left out of this story :+1:

[Matt-Yorkley]

If we run into issues with requests made from javascript we can try capybara-webmock which allows stubbing requests made from javascript in the browser in feature specs.

[luisramos0]

ah, ok, maybe we will need that, nice!

[Matt-Yorkley]

I've had a quick play with this and the stripe js form doesn't load, which messes with the tests. Maybe we can try this gem from thoughtbot: fake_stripe, which looks like it replaces stripe.js during tests with something the suite can actually use.

[luisramos0]

what about the spec in spec/features/consumer/shopping/checkout_stripe_spec.rb?

[Matt-Yorkley]

what about the spec in spec/features/consumer/shopping/checkout_stripe_spec.rb?

Ah, I hadn't updated master for a while when I made a branch to play with Stripe. This test in that spec only checks the use of saved cards, and I think the form elements for that are OFN and not Stripe.

The issue is that when StripeJS loads, it injects an iframe with the form elements into the page, and that iframe does not load in the test suite, so it can't be filled out. There's also some other interactions between our Angular classes and the StripeJS Element class that fail without it. The fake_stripe gem has a fixture to replace StripeJS, but it has a few issues: the newer versions don't work with our version of Capybara, and the faked Stripe class is missing some methods that our Angular code interacts with. I've made a very rough working version of it here: #6050

[luisramos0]

ok :+1: nice approach.
I think we can still test quite a few things with saved cards. I mean, just expanding the existing spec with saved cards: create different saved cards and checkout with them. I wonder if we could test the sca redirects with saved cards.
Even if we use fake_stripe I think we should do the most without it so we are as much independent from that dependency as possible.

I'd prefer to find a solution without any other dependency because any dependency here will break and we will have to upgrade, debug, fix. It's a bit of an investment.... but I dont think we have an alternative!
Having said this, I think your PR looks great!

So, I'd write as many tests as possible with saved cards and no dependencies. As an example of a test: we can have two saved cards and pay with a failed one first and then try another one that works but only after sca auth.
And I'd add necessary dependencies to test scenarios we cant test with saved cards: your PR.

[luisramos0]

Hello @Matt-Yorkley you have created https://github.com/openfoodfoundation/openfoodnetwork/pull/6050 and talked a bit about this in the last few weeks. Do you want to assign yourself to this issue?

[lin-d-hop]

@Matt-Yorkley agreed to take it on with me in Slack. I'm going to assign you Matt :smile: Let me know if you need to not do this one. It's top of the priorities list so we'd love to get it done.... but obvs looking after yourself if more important :heart:

If you want this Spike to become issues (and a new epic) let me know. Happy to help.

[Matt-Yorkley]

So I've done a spike already and made a PR here that gives us a way to handle Stripe requests in specs: #6050. I'll update it now and move it to code review.

I think it needs some review and discussion to see if we all agree on the proposed implementation. It adds two basic tests and a helper that can be used as a template for writing the other specs in the list. :+1:

[Matt-Yorkley]

There's some behavior we should probably cover that caused issues recently and isn't tested, which is: attempting a payment that fails, then immediately attempting another payment that succeeds. Multiple payment objects are created and their states are different.

[Matt-Yorkley]

I'll close this spike and create a new epic and we can curate a list of what we want there.