-
-
Notifications
You must be signed in to change notification settings - Fork 729
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
[Split Checkout] Adds tests on payments #8772
[Split Checkout] Adds tests on payments #8772
Conversation
bddb642
to
c550408
Compare
e8e5cca
to
82809a5
Compare
82809a5
to
e267ae4
Compare
e267ae4
to
d945e69
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are becoming a spec pro!
Can you address those style issues?
let!(:payment_method3) { create(:payment_method, distributors: [distributor], name: "Cash") } | ||
let!(:payment_method4) { create(:payment_method, distributors: [distributor], name: "BoGuS") } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would name these cash
and bogus
instead of payment_method3
and payment_method4
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mkllnk 🙏
On the payment method re-namings:
- I've renamed
payment_method3
tocash
payment_method4
was actually a detour, it got removed on the following commit - sorry if this was a bit misleading.
ac4516c
to
82b6d39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds perfect! Nice one! 👌
Should we merge it directly?
What? Why?
Contributes to #8667 by adding coverage on payment method selection.
These can perhaps be extended in other dedicated spec files, for example on:
-> added in commit 1f6157dspec/system/consumer/shopping/checkout_paypal_spec.rb
spec/system/consumer/shopping/checkout_stripe_spec.rb
What should we test?
Green build.
Release notes
Changelog Category: Technical changes
Adds tests on payment methods for split checkout.