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

Support API 2020-03-02 #710

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

coorasse
Copy link
Contributor

@coorasse coorasse commented Mar 19, 2020

Purpose of this PR is to improve and fix the Travis build, currently failing, and also run the tests against the latest Stripe version.
This forced me to make also some changes to the code.
Please read my comments within the PR.

I think it might bring some breaking changes.

I thought you were using different Stripe Api Keys to test against different API versions, but I see now that is to be able to run parallel builds. Do you want to keep that? I can re-introduce it.

@@ -6,17 +6,18 @@ rvm:
- 2.4.6
- 2.5.5
- 2.6.3
before_install:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was simply not necessary

- STRIPE_TEST_SECRET_KEY_A=sk_test_BsztzqQjzd7lqkgo1LjEG5DF00KzH7tWKF
matrix:
- STRIPE_API_VERSION=2020-03-02

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now, we can run the tests on different versions by simply adding it to the matrix, currently, I want to support only the latest, I haven't checked with older versions. No need for multiple keys anymore (see comment on spec_helper.rb later)

@@ -3,7 +3,7 @@ module RequestHandlers
module ParamValidators

def already_exists_message(obj_class)
"#{obj_class.to_s.split("::").last} already exists."
/#{obj_class.to_s.split("::").last.downcase} already exists./i
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relax it a bit. is different in some requests

@@ -149,7 +149,7 @@
255.times { data << Stripe::Charge.create(amount: 1, currency: 'usd', source: stripe_helper.generate_card_token) }
list = StripeMock::Data::List.new(data, starting_after: "test_ch_unknown")

expect { list.to_h }.to raise_error
expect { list.to_h }.to raise_error(RuntimeError)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not entirely sure

end

c.before(:each) do
allow(StripeMock).to receive(:start).and_return(nil)
allow(StripeMock).to receive(:stop).and_return(nil)
Stripe.api_key = api_key
Stripe.api_version = ENV['STRIPE_API_VERSION']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we can set and test against different API versions. The library might also use Stripe.api_version to be retro-compatible (not necessary imho).

@courtsimas
Copy link

👀

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

Successfully merging this pull request may close these issues.

None yet

2 participants