-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Basic toolchain.py unit tests #1928
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
import sys | ||
import pytest | ||
import mock | ||
from pythonforandroid.toolchain import ToolchainCL | ||
from pythonforandroid.util import BuildInterruptingException | ||
|
||
|
||
def patch_sys_argv(argv): | ||
return mock.patch('sys.argv', argv) | ||
|
||
|
||
def patch_argparse_print_help(): | ||
return mock.patch('argparse.ArgumentParser.print_help') | ||
|
||
|
||
def raises_system_exit(): | ||
return pytest.raises(SystemExit) | ||
|
||
|
||
class TestToolchainCL: | ||
|
||
def test_help(self): | ||
""" | ||
Calling with `--help` should print help and exit 0. | ||
""" | ||
argv = ['toolchain.py', '--help', '--storage-dir=/tmp'] | ||
with patch_sys_argv(argv), raises_system_exit( | ||
) as ex_info, patch_argparse_print_help() as m_print_help: | ||
ToolchainCL() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't something like this be a little easier to understand:
Or is that only me? Mock is fine but if it's not needed I would still rather use standard methods There is also a lot of repetition of this block in particular:
No matter if we end up using mock or not, maybe a helper function for that would be useful? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The subprocess is overkill to me, it's really too much top level integration test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'll let you decide but just as a note: that seems like an irrelevant point to me, because isn't this what the entire purpose is? Check if the overall tool responds fine to But keep in mind my proposal would also possibly reduce the maintenance burden if But that's just my personal opinion of course and I don't think this should hold off a merge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I definitely don't share this point of view regarding readability, I find that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's The mocking really isn't too bad I just seem to have unusual preferences here |
||
assert ex_info.value.code == 0 | ||
assert m_print_help.call_args_list == [mock.call()] | ||
|
||
@pytest.mark.skipif(sys.version_info < (3, 0), reason="requires python3") | ||
def test_unknown(self): | ||
""" | ||
Calling with unknown args should print help and exit 1. | ||
""" | ||
argv = ['toolchain.py', '--unknown'] | ||
with patch_sys_argv(argv), raises_system_exit( | ||
) as ex_info, patch_argparse_print_help() as m_print_help: | ||
ToolchainCL() | ||
assert ex_info.value.code == 1 | ||
assert m_print_help.call_args_list == [mock.call()] | ||
|
||
def test_create(self): | ||
""" | ||
Basic `create` distribution test. | ||
""" | ||
argv = [ | ||
'toolchain.py', | ||
'create', | ||
'--sdk-dir=/tmp/android-sdk', | ||
'--ndk-dir=/tmp/android-ndk', | ||
'--dist-name=test_toolchain', | ||
] | ||
with patch_sys_argv(argv), mock.patch( | ||
'pythonforandroid.build.get_available_apis' | ||
) as m_get_available_apis, mock.patch( | ||
'pythonforandroid.build.get_toolchain_versions' | ||
) as m_get_toolchain_versions, mock.patch( | ||
'pythonforandroid.build.get_ndk_platform_dir' | ||
) as m_get_ndk_platform_dir, mock.patch( | ||
'pythonforandroid.build.get_cython_path' | ||
) as m_get_cython_path, mock.patch( | ||
'pythonforandroid.toolchain.build_dist_from_args' | ||
) as m_build_dist_from_args: | ||
m_get_available_apis.return_value = [27] | ||
m_get_toolchain_versions.return_value = (['4.9'], True) | ||
m_get_ndk_platform_dir.return_value = ( | ||
'/tmp/android-ndk/platforms/android-21/arch-arm', True) | ||
ToolchainCL() | ||
assert m_get_available_apis.call_args_list == [ | ||
mock.call('/tmp/android-sdk')] | ||
assert m_get_toolchain_versions.call_args_list == [ | ||
mock.call('/tmp/android-ndk', mock.ANY)] | ||
assert m_get_cython_path.call_args_list == [mock.call()] | ||
assert m_build_dist_from_args.call_count == 1 | ||
|
||
def test_create_no_sdk_dir(self): | ||
""" | ||
The `--sdk-dir` is mandatory to `create` a distribution. | ||
""" | ||
argv = ['toolchain.py', 'create'] | ||
with mock.patch('sys.argv', argv), pytest.raises( | ||
BuildInterruptingException | ||
) as ex_info: | ||
ToolchainCL() | ||
assert ex_info.value.message == ( | ||
'Android SDK dir was not specified, exiting.') |
Uh oh!
There was an error while loading. Please reload this page.