-
Notifications
You must be signed in to change notification settings - Fork 32
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
Use only pytest #51
Use only pytest #51
Conversation
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.
Thanks, this looks very cool!
I think they're not necessary any more since we don't use travis. |
fed5298
to
e1f2838
Compare
I rebased it and checked for LOAD_CONST (... see unit tests) should I exclude _pytest stuff from the test_module_files? The tests failed once for some pytest source file. |
tests/test_pytest.py
Outdated
from executing.executing import is_rewritten_by_pytest,get_instructions | ||
import dis |
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.
Please put these at the top
That sounds like a bug to investigate. Besides, I see a new failure after that. Also, the previous build failed to report the incorrect node: |
I minimized the failure down to the following. (currently working on the issue minimization) class Block(ABC):
def __init__(self, container: Optional["Container"] = None):
self._lines: List[str] = [] The annotated assignment seems to be the problem. |
I think all you need to do is check |
Co-authored-by: Alex Hall <[email protected]>
I looked at the failure and the problem is that the pre 3.11 test code does not analyse deadcode. example: class A:
def __cmp__(self, other):
if type(other) is self.__class__:
if self is other:
return 0
return -1
return NotImplemented
raise TypeError("unorderable types: %s() and %s()" % (self.__class__.__name__, other.__class__.__name__)) There is no bytecode for the TypeError ast-node. I added the test also to the samples for now but I think I will remove it later again. Should I continue here and fix this problems? |
How hard do you think it'd be to get deadcode analysis working for older versions? |
Assuming 3.11 is the most optimised, I think it'd be a good compromise to always ignore nodes marked as deadcode when they don't match bytecode, but only complain about bytecode matching deadcode in 3.11. That might make things easier. |
Therefore I would have to make the deadcode analysis work down to python 2.7 ... I would like to improve the tests before I start to change there anything again (see #55). I try to ignore the enum module for now, if this is also ok. |
tests/test_main.py
Outdated
@@ -708,6 +711,9 @@ def test_module_files(self): | |||
if ( | |||
# The sentinel actually appearing in code messes things up | |||
'executing' in filename | |||
# ignore pytest files because they break the SentinelNodeFinder | |||
or '_pytest' in filename |
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 think this file stopped being a problem once annotations were considered in 3.6+
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.
no problem any more.
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.
So these lines should be removed, right?
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.
sorry you are right. I forgot that.
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.
Ah, it just hit a genuine failure because it doesn't know how to distinguish between the comprehension codes in {t[0] for t in lines2} - {t[0] for t in lines1}
. This is covered in the normal tests. So pytester.py
specifically should be excluded.
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.
sigh, another pypy optimisation. can you skip if PYPY and '__debug__' in source.text'
?
if it fails again you can go back to skipping all pytest files
this expression gets evaluated at compile time ... PYC_EXT = ".py" + (__debug__ and "c" or "o") |
Thanks! |
I was able to execute all tests with pytest.
However assertion rewriting is still disabled for
test_main.py
.But this has already several benefits:
tox -e py310 -- --sw
worksI also converted the
test_sample_files
to an parametrized test which makes it more useful forpytest --sw
I also don't know how I should handle the timeouts.
I removed them for
test_sample_files
because they are not really useful for single file checks.I kept them for
test_module_files
because this takes already a bit longer.What's your opinion there?
I wanted to get your feedback before I continue.