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

Use Webmock instead of Mocha for HTTP stubbing #496

Merged
merged 7 commits into from
Feb 14, 2017

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Jan 20, 2017

The test suite currently depends on one special method in the Stripe module to stub out HTTP methods in the test suite using the Mocha mocking framework. This is problematic for a couple reasons:

  • The vast majority of tests aren't really testing much of note. We're just testing nothing that we're sending an expected set of parameters to one of our internal methods, and nothing beyond that. The entire HTTP stack is skipped completely.
  • It makes switching out RestClient for something better very difficult. I'm currently trying to tackle Configurable HTTP client #313, and the test suite is turning out to be a major blocker.

This PR is going to be very difficult to review because of so much churn, but it only aims to replace HTTP stubbing calls for the most part. There are also a few other miscellaneous improvements introduced to help with consistency:

  • URLs in test suites now use full URLs in all cases instead of just stubbing out one particular HTTP verb for the duration of the method (e.g. GET). This makes it a lot more explicit as to what each test depends on.
  • URLs in test suites now reply on Stripe.api_base to get a host instead of just hard-coding api.stripe.com.
  • I took out a lot of "refresh" tests from individual models. These are redundant because we have them in the base API resource tests and are available inconsistently between models.
  • Took out a lot of useless assertions that are just checking for values that they just mocked. These were testing Mocha rather than the stripe-ruby library which is totally pointless.
  • Tried to move HTTP stubs closer to the invocations that they're actually used instead of grouping them all together at the beginning of a test.
  • I removed "clear metadata" tests where I found them. These are redundant with the update tests that already existed, and are really just testing our parameter encoder which is better tested elsewhere.
  • Change a lot of "saveable" tests to use .retrieve instead of .new + .refresh. It does the same thing and is more concise, and this makes the whole test suite more consistent with itself (many models were already using .retrieve).

Currently targets #495 (dropping 1.9 support) because Webmock depends on gems that have long since stopped supporting Ruby 1.9.

@brandur-stripe brandur-stripe changed the base branch from brandur-kill-19 to master February 14, 2017 20:12
@brandur-stripe brandur-stripe merged commit 15f77a6 into master Feb 14, 2017
@ob-stripe ob-stripe deleted the brandur-webmock branch June 27, 2017 13:05
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.

2 participants