Skip to content

Conversation

dlc37
Copy link
Contributor

@dlc37 dlc37 commented Mar 1, 2017

Inspired by python3 test failures before @Roguelazer modernized them. The order of 'all()' changes.

@dlc37 dlc37 requested a review from Roguelazer March 1, 2017 23:07
@Roguelazer
Copy link
Contributor

I actually manually added the message field to our Exception subclass in #50, since otherwise the APIs were incompatible between Python 2 and Python 3.

I'm fine with this if tests pass, though.

@dlc37
Copy link
Contributor Author

dlc37 commented Mar 2, 2017

One of the tests failed; I've fixed the test. Not sure if Travis is rerunning the tests automagically...

@dlc37
Copy link
Contributor Author

dlc37 commented Mar 2, 2017

Tests pass.

# Index webhooks
webhooks = easypost.Webhook.all(url=expected_url)
assert webhooks["webhooks"][len(webhooks["webhooks"]) - 1].id == webhook.id
for wh in webhooks["webhooks"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

how about just

assert any(wh.id == webhook.id for wh in webhooks['webhooks'])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks.


assert report.object == "ShipmentReport"
assert report.status == "available"
assert report.status in ("available", "new")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think this is sufficient, or will the retrieve() call fail for new reports? I can't reproduce new reports.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's sufficient. If not, we'll fix it next time it breaks.

@Roguelazer
Copy link
Contributor

🛳it

@dlc37 dlc37 merged commit b011d1f into master Mar 8, 2017
@dlc37 dlc37 deleted the dlc37_unite_python2_and_3 branch March 8, 2017 19:47
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