-
Notifications
You must be signed in to change notification settings - Fork 53
Mock network requests for faster unit testing #13 #84
base: master
Are you sure you want to change the base?
Mock network requests for faster unit testing #13 #84
Conversation
This way, changes to the local config won't affect the tests.
Hi @hydrosquall, This is awesome! I think we will create a new issue to deal with the DB mocks. I'm not sure the best way to do that either. Perhaps if we query a few people in this repos, we might find a great solution. With regards to the failing tests, it seems we need to update the requirements files to include the missing dependencies. Could you please update those? With Best Regards, Elmer |
Hey @thinkingserious : How would you feel about dropping support for Python 3.2? Otherwise, I'll need to figure out a way to install the library below only for the build that happens on version 3.2, since |
HI @hydrosquall, I'm not opposed to requiring that dependency for 3.2 development. |
57aee5c
to
58e9d25
Compare
16d33f1
to
113b5a0
Compare
@thinkingserious : I think my latest changes should fix the build. In the future, we may want to write some separate tests that are able to test just the HTTP requests, without having the dependency on the DB mocks. As it stands, I think all of the tests might depend on the database with the exception of the "send email" test. |
Thanks @hydrosquall! |
Hey @thinkingserious , it looks like |
Hi @hydrosquall, Thanks for the further detail! Once this issue pops up on our backlog, I'll dig deeper. With Best Regards, Elmer |
number_of_watchers=0, | ||
number_of_stargazers=0, | ||
number_of_forks=0 | ||
) | ||
res = self.db.add_data(github_data_import) | ||
self.assertTrue(isinstance(res, GitHubData)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.assertIsInstance
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@42B thanks for catching these items - with that said, these lines fall outside the scope of the topics for this active PR, if you're interested, maybe those changes could go into a separate PR
php_downloads=0, | ||
python_downloads=0, | ||
ruby_downloads=0 | ||
) | ||
res = self.db.add_data(packagedata) | ||
self.assertTrue(isinstance(res, PackageManagerData)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.assertIsInstance
?
self.config.github_repos[0]) | ||
config = MOCK_DATA['github_config'] | ||
res = self.github.update_library_data(config['user'], | ||
config['repositories'][0]) | ||
self.assertTrue(isinstance(res, GitHubData)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.assertIsInstance
?
def test_update_package_manager_data(self): | ||
if os.environ.get('TRAVIS') == None: | ||
res = self.pm.update_package_manager_data( | ||
self.config.package_manager_urls) | ||
MOCK_DATA['package_urls']) | ||
self.assertTrue(isinstance(res, PackageManagerData)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.assertIsInstance
?
Description of the change:
tests/fixtures
folder. I redacted the API key that was used to get this data, and usedunittest.patch
to path the HTTP requests so that the cassettes could be reused later.Reason for the change:
Checklist
Make sure all of these items are complete, or else the PR will be ineligible for a code review.
tests/test.py
CONTRIBUTING.md
)master
branch. ( seeCONTRIBUTING.md
).