Skip to content

Adds new backup_attributes context manager for tests #1948

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions pythonforandroid/toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -741,12 +741,12 @@ def _read_configuration():
def recipes(self, args):
"""
Prints recipes basic info, e.g.
```
python3 3.7.1
depends: ['hostpython3', 'sqlite3', 'openssl', 'libffi']
conflicts: ['python2']
optional depends: ['sqlite3', 'libffi', 'openssl']
```
.. code-block:: bash

python3 3.7.1
depends: ['hostpython3', 'sqlite3', 'openssl', 'libffi']
conflicts: ['python2']
optional depends: ['sqlite3', 'libffi', 'openssl']
"""
ctx = self.ctx
if args.compact:
Expand Down
8 changes: 5 additions & 3 deletions tests/test_toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import sys
import pytest
import mock
from util import backup_attributes
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if the name you give to the file can cause troubles in some import situations...since we already have a file with the same name in pythonforandroid.util 🤔...I would give it another filename to avoid strange situations...but maybe there is no need...what do you think about this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it also bugged me. Actually I usually call it utils.py, then I changed it to be consistent. But still yes it also bugs me that it looks the same and one is for tests only. I don't have preferences, tell me how you like it to be called and I can call it that way 😄

Copy link
Member

@opacam opacam Aug 1, 2019

Choose a reason for hiding this comment

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

  • tools
  • utilities
  • assets
    ...
    whatever it's fine...just be careful to not have one already in p4a source code...
    I don't have preferences for this neither 😆

Copy link

Choose a reason for hiding this comment

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

What about test_utils.py?

Copy link
Member

Choose a reason for hiding this comment

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

@Jonast, I would avoid test_utils...I think that unittests looks into the first part of the filename to determine if a file is a test or not...am I right?

Plus the mentioned file is not a test...it's a set of utilities to help us testing...I think that it's confusing...

Copy link

Choose a reason for hiding this comment

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

utils_for_testing.py? 😄

Copy link
Member

Choose a reason for hiding this comment

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

That looks better 😉

from pythonforandroid.recipe import Recipe
from pythonforandroid.toolchain import ToolchainCL
from pythonforandroid.util import BuildInterruptingException
Expand Down Expand Up @@ -121,7 +122,10 @@ def test_recipes(self):
Checks the `recipes` command prints out recipes information without crashing.
"""
argv = ['toolchain.py', 'recipes']
with patch_sys_argv(argv), patch_sys_stdout() as m_stdout:
with (
patch_sys_argv(argv)), (
patch_sys_stdout()) as m_stdout, (
backup_attributes(Recipe, {'recipes'})):
Copy link
Member Author

Choose a reason for hiding this comment

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

in Python3 that could also be used as a decorator, e.g.

@backup_attributes(Recipe, {'recipes'})
def test_recipes(self):
    pass

Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering, will this work when used alongside with mock.patch decorator?

Copy link
Member Author

Choose a reason for hiding this comment

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

Depends of what you patch and if you patch before the backup_attributes or after I guess

Copy link
Member

Choose a reason for hiding this comment

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

Ok, we'll see then, thanks!

ToolchainCL()
# check if we have common patterns in the output
expected_strings = (
Expand All @@ -134,5 +138,3 @@ def test_recipes(self):
)
for expected_string in expected_strings:
assert expected_string in m_stdout.getvalue()
# deletes static attribute to not mess with other tests
del Recipe.recipes
31 changes: 31 additions & 0 deletions tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# `Python 2` or lower than `Python 3.3` does not
# have the `unittest.mock` module built-in
import mock
import util as test_util
from pythonforandroid import util


Expand All @@ -17,6 +18,36 @@ class TestUtil(unittest.TestCase):
:mod:`~pythonforandroid.util`.
"""

def test_backup_attributes(self):
"""
Checks the helper backup_attributes context manager works as expected.
"""
class MyClass:
def __init__(self, foo, bar):
self.foo = foo
self.bar = bar

# testing trivial flat backup
foo = 'foo'
bar = 'bar'
my_object = MyClass(foo=foo, bar=bar)
with test_util.backup_attributes(my_object, {'foo'}):
my_object.foo = 'not foo'
my_object.bar = 'not bar'
assert my_object.foo == 'foo'
assert my_object.bar == 'not bar'
# testing deep backup
foo = {'foo': {1, 2, 3}}
bar = {'bar': {3, 2, 1}}
my_object = MyClass(foo=foo, bar=bar)
with test_util.backup_attributes(my_object, {'foo', 'bar'}):
# changing the reference
my_object.foo = {}
# and mutating the object the attribute is referencing to
my_object.bar['bar'] = None
assert my_object.foo == {'foo': {1, 2, 3}}
assert my_object.bar == {'bar': {3, 2, 1}}

@mock.patch("pythonforandroid.util.makedirs")
def test_ensure_dir(self, mock_makedirs):
"""
Expand Down
19 changes: 19 additions & 0 deletions tests/util.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from copy import deepcopy
from contextlib import contextmanager


@contextmanager
def backup_attributes(obj, attributes):
"""
Makes a backup of the object attributes that gets restored later.
"""
obj_dict = obj.__dict__
# creates a subset dictionary of the attributes we're interested in
attributes_backup = dict(
(k, obj_dict[k]) for k in attributes if k in obj_dict)
attributes_backup = deepcopy(attributes_backup)
try:
yield
finally:
for attribute in attributes:
setattr(obj, attribute, attributes_backup[attribute])