-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 unittest for module pythonforandroid.distribution
#1847
Conversation
2d81491
to
ad877a2
Compare
Looks sensible thank you. I'll try to spend more time this evening digging into it a bit more. But having more tests and better coverage cannot harm I guess 😄 |
] | ||
|
||
dists = self.ctx.bootstrap.distribution.get_distributions(self.ctx) | ||
self.assertEqual(dists[0].ndk_api, None) |
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.
just a random idea, we could also have mocked warning
and asserted its call
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.
You are right we could do that, but I more concerned to mock the stuff that could slow down our tests (mostly file operations: copy, paste, pack, unpack, read files...)
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.
yes I just throw that idea, because I may have mocked it, just to show we're throwing a warning in this specific case. But you're covering the case anyway asserting ndk_api is None. By the way if you like self.assert you also have self.assertIsNone()
.
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.
I took a deeper look and it's pretty well written. You're actually unit testing each method in a clear way.
Awesome job, thanks!
To enhance our code coverage...a little more
Note: In order to optimize the tests, I
mocked
any file creation