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

Add VCR to and improve all integration tests #132

Merged
merged 1 commit into from
Oct 9, 2018

Conversation

joshcanhelp
Copy link
Contributor

@joshcanhelp joshcanhelp commented Oct 3, 2018

Add VCR HTTP recording to integration test suite. Specifically:

  • Add VCR gem and config
  • Add ~7K lines of recorded HTTP requests/responses as YML files
  • Refactor all integration tests to add VCR, add expect text, and fix several tests
  • Disallow outbound HTTP calls
  • Remove cleanup routine for after(:suite)
  • Fix a handful of bugs in the SDK found while testing

@@ -2,22 +2,28 @@ module Credentials
module_function
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whitespace only in this file

@@ -32,7 +32,7 @@ def resource_server(resource_server_id)
# @return [json] Returns the resource server.
def create_resource_server(identifier, options = {})
raise Auth0::InvalidParameter, 'Must supply a valid resource server id' if identifier.to_s.empty?
if ['<', '>'].include?(options.fetch(:name, ''))
if options.fetch(:name, '').index(/[<>]/)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was not actually checking for what it was trying to check for

@@ -101,7 +101,7 @@ def delete_user(user_id)
# @return [json] Returns the updated user.
def patch_user(user_id, body)
raise Auth0::MissingUserId, 'Must supply a valid user_id' if user_id.to_s.empty?
raise Auth0::InvalidParameter, 'Must supply a valid body' if body.to_s.empty?
raise Auth0::InvalidParameter, 'Must supply a valid body' if body.to_s.empty? || body.empty?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did not catch an empty hash

@@ -13,26 +13,6 @@
config.filter_run focus: true
config.run_all_when_everything_filtered = true
config.include Credentials
config.after(:suite) do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed unnecessary cleanup

@joshcanhelp joshcanhelp added this to the v4-Next milestone Oct 5, 2018
@joshcanhelp joshcanhelp changed the title Add VCR to integration tests [WIP] Add VCR to and improve all integration tests Oct 5, 2018
@joshcanhelp joshcanhelp requested review from machuga and removed request for machuga October 5, 2018 22:13
@machuga machuga self-requested a review October 9, 2018 09:49
Copy link

@machuga machuga left a comment

Choose a reason for hiding this comment

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

Yay for the speed improvements!

@joshcanhelp joshcanhelp force-pushed the add-vcr-to-integration-tests branch from 000e2cc to a05c6bc Compare October 9, 2018 14:41
@joshcanhelp joshcanhelp merged commit 87ae927 into master Oct 9, 2018
@joshcanhelp joshcanhelp deleted the add-vcr-to-integration-tests branch October 13, 2018 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants