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

Provide full url to process_page when there is more than 50 entries. #40

Merged
merged 3 commits into from
Jun 6, 2016

Conversation

rlb3
Copy link
Contributor

@rlb3 rlb3 commented Jun 3, 2016

The previous variable next_page_uri was returning the end point /2010-04-01/Accounts/xxxxxxx/Messages.json and causing HTTPotion to raise a HTTPotion.HTTPError because it wasn't a full url.

There are four tests failing but this is because the doc test for the url_generator don't have an account_sid and are production an empty path: ie. https://api.twilio.com/2010-04-01/Accounts//Resources.json, but expect "https://api.twilio.com/2010-04-01/Accounts/Resources.json"

I'm not sure how to test this because it only shows up contacting Twilio and you have more that 50 resources to stream.

Fixes #38

rlb3 added 3 commits June 3, 2016 10:16
The previous variable next_page_uri was returning the end point

`/2010-04-01/Accounts/xxxxxxx/Messages.json`

and causing HTTPotion to raise a HTTPotion.HTTPError because it wasn't a
full url.
@danielberkompas
Copy link
Owner

Thanks! Reviewing this now.

@danielberkompas
Copy link
Owner

Looks like all the tests are passing on Travis. Are they not passing for you?

@rlb3
Copy link
Contributor Author

rlb3 commented Jun 6, 2016

This is the output from running the tests.

Ξ Work/ex_twilio git:(process_page) ▶ mix test  
......

  1) test doc at ExTwilio.UrlGenerator.build_url/3 (6) (ExTwilio.UrlGeneratorTest)
     test/ex_twilio/url_generator_test.exs:22
     Doctest failed
     code: ExTwilio.UrlGenerator.build_url(Resource, nil, iso_country_code: "US", type: "Mobile", page: 20) === "https://api.twilio.com/2010-04-01/Accounts//Resources/US/Mobile.json?Page=20"
     lhs:  "https://api.twilio.com/2010-04-01/Resources/US/Mobile.json?Page=20"
     stacktrace:
       lib/ex_twilio/url_generator.ex:34: ExTwilio.UrlGenerator (module)

......

  2) test doc at ExTwilio.UrlGenerator.build_url/3 (4) (ExTwilio.UrlGeneratorTest)
     test/ex_twilio/url_generator_test.exs:22
     Doctest failed
     code: ExTwilio.UrlGenerator.build_url(Resource, 1) === "https://api.twilio.com/2010-04-01/Accounts//Resources/1.json"
     lhs:  "https://api.twilio.com/2010-04-01/Resources/1.json"
     stacktrace:
       lib/ex_twilio/url_generator.ex:28: ExTwilio.UrlGenerator (module)

.

  3) test doc at ExTwilio.UrlGenerator.build_url/3 (5) (ExTwilio.UrlGeneratorTest)
     test/ex_twilio/url_generator_test.exs:22
     Doctest failed
     code: ExTwilio.UrlGenerator.build_url(Resource, nil, page: 20) === "https://api.twilio.com/2010-04-01/Accounts//Resources.json?Page=20"
     lhs:  "https://api.twilio.com/2010-04-01/Resources.json?Page=20"
     stacktrace:
       lib/ex_twilio/url_generator.ex:31: ExTwilio.UrlGenerator (module)

.

  4) test doc at ExTwilio.UrlGenerator.build_url/3 (1) (ExTwilio.UrlGeneratorTest)
     test/ex_twilio/url_generator_test.exs:22
     Doctest failed
     code: ExTwilio.UrlGenerator.build_url(Resource) === "https://api.twilio.com/2010-04-01/Accounts//Resources.json"
     lhs:  "https://api.twilio.com/2010-04-01/Resources.json"
     stacktrace:
       lib/ex_twilio/url_generator.ex:19: ExTwilio.UrlGenerator (module)

..........................

Finished in 1.3 seconds (0.3s on load, 1.0s on tests)
44 tests, 4 failures

Randomized with seed 540005

@danielberkompas
Copy link
Owner

I just pulled down the branch and tried it locally, and the tests passed. I think you just hadn't set up your .env file or hadn't sourced it.

See .env.sample for which environment variables you'd need to set.

@danielberkompas danielberkompas merged commit 1b518db into danielberkompas:master Jun 6, 2016
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