8000 Migrate TestTree from tests/test_git.py to tests/integration + tests/unit by itsmemattchung · Pull Request #467 · sigmavirus24/github3.py · GitHub
[go: up one dir, main page]

Skip to content

Migrate TestTree from tests/test_git.py to tests/integration + tests/unit #467

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

Merged
merged 5 commits into from
Nov 8, 2015

Conversation

itsmemattchung
Copy link
Contributor

Summary

Created a unit and integration test for github3.git.Tree. This is continuation of the ongoing objective of removing all tests that are not under unit or integration

In order to call self.instance.recurse(), we need to add an integration test. After, we can then remove the class TestTree completely
self.instance.recurse()
self.session.get.assert_called_once_with(
url_for(),
params={u'recursive': u'1'}
Copy link
Owner

Choose a reason for hiding this comment

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

If you remove the u will this pass the tests on the other versions of Python? It should. This is causing the failure on Python 3.2.

@sigmavirus24
Copy link
Owner

A couple comments otherwise this looks 💯 @itsmemattchung!

@itsmemattchung
Copy link
Contributor Author

@sigmavirus24 just removed the unicode and now tests are passing. Going to look into changing the test data from the documentation and will ping you once I'm done with that.

@itsmemattchung
Copy link
Contributor Author

@sigmavirus24 I went ahead and updated the example with octocat/Hello-World.

sigmavirus24 added a commit that referenced this pull request Nov 8, 2015
Migrate TestTree from tests/test_git.py to tests/integration + tests/unit
@sigmavirus24 sigmavirus24 merged commit 9870032 into sigmavirus24:develop Nov 8, 2015
@sigmavirus24
Copy link
Owner

🍰 Thanks @itsmemattchung !

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
0