Enable combined coverage reports under tox and branch coverage #95
Conversation
Fixes #91 Remove many (or most) now-redundant 'pragma: no cover'. A few minor test changes to get full branch coverage.
src/zope/interface/declarations.py
Outdated
| # the coverage for this block there. :( | ||
| if PYTHON3: # pragma: no cover | ||
| raise TypeError('Class advice impossible in Python3') | ||
| assert not PYTHON3, "Class advice impossible in Python 3" |
There was a problem hiding this comment.
Please don't convert TypeError to AssertionError (we only raise them in test code).
There was a problem hiding this comment.
The rationale is that python -O suppresses assertions: we don't want to remove that check in that case.
There was a problem hiding this comment.
This is a private implementation method for this module. Its callers already perform this same check and raise a TypeError before calling it. So the if/raise statement was dead code.
I could completely remove the assert?
There was a problem hiding this comment.
I'm sure. Even if we take the assert out and manually try to use the implementation method, we get hit with another explicit TypeError:
Python 3.6.1 (default, Jun 1 2017, 07:41:42)
[GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.38)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from zope.interface.declarations import _implements, classImplements
>>> from zope.interface import Interface
>>> class Foo(object):
... _implements('implements', Interface, classImplements)
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<stdin>", line 2, in Foo
File "//.tox/py36/lib/python3.6/site-packages/zope/interface/declarations.py", line 446, in _implements
raise TypeError(name+" can be used only from a class definition.")
TypeError: implements can be used only from a class definition.|
|
||
| import sys | ||
| if sys.version[0] == '2': # This test only makes sense under Python 2.x | ||
| if sys.version_info[0] < 3: # This test only makes sense under Python 2.x |
There was a problem hiding this comment.
Rather than testing sys.version_info, this is probably better as a conditional import:
try:
from types import ClassType
except ImportError:
pass
else:
assert not isinstance(C, (type, ClassType))There was a problem hiding this comment.
Yeah, it probably is :) It started out as a copy of the doctest in odd.py, I just fixed this when I was globally fixing the same thing in other files.
…all its callers already do this guard; be more explicit about ClassType in test_odd_declarations
|
Thank you! I'm going to push 4.4.2 to PyPI now unless there are objections. |
Fixes #91
Remove many (or most) now-redundant 'pragma: no cover'.
A few minor test changes to get full branch coverage.