Skip to content

Conversation

@dhavlik
Copy link

@dhavlik dhavlik commented Sep 13, 2017

No description provided.

@dhavlik
Copy link
Author

dhavlik commented Sep 13, 2017

travis-ci errors will be fixed by another pull request by icemac.

Copy link
Member

@icemac icemac left a comment

Choose a reason for hiding this comment

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

Thank you for improving the coverage. 👍

__metaclass__=type,
_write_=lambda x: x,
_getattr_=getattr)
_getattr_=getattr,)
Copy link
Member

Choose a reason for hiding this comment

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

This one is not necessary.

assert restricted_globals['b'] == '2411'


class D:
Copy link
Member

Choose a reason for hiding this comment

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

The classes should have more meaningful values. If the are only needed for one test they could be moved into the test function together with the code which uses them.

assert None is restricted_globals['d1'].value


class E:
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

assert 'attribute-less object (assign or del)' in str(excinfo.value)


class F:
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

def test_Guards__safe_builtins__3(e_exec):
"""It allows use setattr and delattr when _guarded_writes is True.
`__build_class__` is only needed in Python 3.
Copy link
Member

Choose a reason for hiding this comment

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

I do not see any __build_class__ usage in this test.



@pytest.mark.parametrize(*e_exec)
def test_Guards__safe_builtins__3(e_exec):
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be called test_Guards__guarded_setattr__1 as it actually tests guarded_setattr.


@pytest.mark.parametrize(*e_exec)
def test_Guards__safe_builtins__3(e_exec):
"""It allows use setattr and delattr when _guarded_writes is True.
Copy link
Member

Choose a reason for hiding this comment

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

The delattr test could be moved to another test function.


@pytest.mark.parametrize(*e_exec)
def test_Guards__write_wrapper__1(e_exec):
"""It wraps it when it is not marked with _guarded_writes."""
Copy link
Member

Choose a reason for hiding this comment

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

"It wraps it" ... what is it?


@pytest.mark.parametrize(*e_exec)
def test_Guards__write_wrapper__2(e_exec):
"""It wraps it and it works when guarded_setattr is implemented."""
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

@pytest.mark.parametrize(*e_exec)
def test_Guards__guarded_unpack_sequence__1(e_exec, mocker):
"""It does not protect unpacking when the sequence is shorter
than expected."""
Copy link
Member

Choose a reason for hiding this comment

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

The comment should explain why this is okay.

Copy link
Author

@dhavlik dhavlik left a comment

Choose a reason for hiding this comment

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

fertig

@dhavlik dhavlik merged commit f19447f into master Sep 14, 2017
@dhavlik dhavlik deleted the increase-test-coverage-of-Guards branch September 14, 2017 09:04
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.

3 participants