- Notifications
You must be signed in to change notification settings - Fork112
Activate code coverage tests#202
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…keypatched by pytest, then activate it only for Travis Xenial VMs.
…ports during test execution.
codecov-io commentedJul 13, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## master #202 +/- ##===========================================+ Coverage 48.78% 60.62% +11.84%=========================================== Files 36 37 +1 Lines 2843 2865 +22 ===========================================+ Hits 1387 1737 +350+ Misses 1456 1128 -328
Continue to review full report at Codecov.
|
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
… with lambdas instead.
I've had a lot of changes since, and I think I've also addressed your comments. How about another quick glance? |
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 added suggestions aroundmonkeypatch, but it is working as-is and looks fine.
| importlogging | ||
| importunittest | ||
| from_pytest.monkeypatchimportMonkeyPatch |
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.
Instead of importing this from_pytest, instantiating it, and getting a context manager, you can just use it as afixture (no import required).
def test_main(monkeypatch): monkeypatch.setattr(...)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.
Apparently, in order for me to use this, my methodhas to be a fixture. In this case, it isn't, so I guess I'll use the context manager way then.
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.
Did it not work? I'm using it in FuzzManager in tests (eg.here).
That SO is about using it in a setup method, which is not a test, so it has to be a fixture to use other fixtures. That doesn't apply to you.
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.
It works if the test_* function is not in a class:
def test_main(monkeypatch):However, it doesn't if it is in a custom class:
class RunCcoverageTests(unittest.TestCase): """"TestCase class for functions in run_ccoverage.py""" @staticmethod /snip def test_main(monkeypatch):It fails with the following error:
TypeError: test_main() missing 1 required positional argument: 'monkeypatch'
Question: IsRunCcoverageTests supposed to inherit fromunittest.TestCase for things to work?
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 tried:
import _pytest/snipclass RunCcoverageTests(_pytest.monkeypatch.MonkeyPatch): """"TestCase class for functions in run_ccoverage.py""" @staticmethod /snip def test_main(monkeypatch):and this seems to work. Is this the right intended fix?SO also mentions thatunittest.TestCase cannot be monkeypatched in the form I mentioned in the previous comment, so I think this is related.
I know this can be solved if I remove the class entirely, but I (may) have some other tests that use their own class in more elaborate ways.
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.
Another question: bynot using awith statement, wouldn't the function continue to be monkeypatch'ed for the remaining of the runs of tests by pytest?
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.
Oh no, calling_pytest.monkeypatch.MonkeyPatch as the parameter forRunCcoverageTests results in pytest not being able to discover this test automatically.
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.
Hmm, you're right. Thepytest unittest section explains why in the notes at the end. Any new tests I write are not inheriting fromunittest. The only point of it is to run existing test suites while transitioning topytest.
About the monkeypatch lifetime without using a context (with): Fixtures are cleaned up automatically. They are thepytest equivalent ofunittest.TestCase.setUp() andunittest.TestCase.tearDown(). Another useful one istmp_path which gives you apathlib.Path object to a unique temporary directory.
So here's theunittest way to use a fixture likemonkeypatch without using the internal classes in_pytest:
import timeimport unittestimport pytest@pytest.fixturedef cls_monkeypatch(request, monkeypatch): request.cls.monkeypatch = monkeypatch@pytest.mark.usefixtures('cls_monkeypatch')class ThingTest(unittest.TestCase): def test_something(self): self.monkeypatch.setattr(time, 'sleep', lambda x: None) start = time.time() time.sleep(1) end = time.time() self.assertLess(end - start, 1)... and here's thepytest way:
import timedef test_something(monkeypatch): monkeypatch.setattr(time, 'sleep', lambda x: None) start = time.time() time.sleep(1) end = time.time() assert (end - start) < 1There 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'll move entirely to pytest in another PR. Thanks!
tests/test_run_ccoverage.py Outdated
| monkey=MonkeyPatch() | ||
| withmonkey.context()asmonkey_context: | ||
| monkey_context.setattr(gatherer,"RUN_COV_TIME",3) | ||
| monkey_context.setattr("funfuzz.ccoverage.reporter.report_coverage", |
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.
You could also ensure thatreport_coverage anddisable_pool are called as you expect, instead of just logging. I usually do this by declaring adict() in the method and then setting keys to assert later. Eg.
calls = {"disable_pool": 0}monkeypatch.setattr("disable_pool", lambda: calls["disable_pool"] += 1)run_ccoverage.main()assert calls["disable_pool"] == 1There 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.
We can't do an increment assignment in a lambda, can we?lambda: calls["disable_pool"] += 1 seems to throw at+=.
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.
You're right, my bad. I am doing this somewhere, but I must have been using a non-lambda function, and I can't find it.
This activates tests for code related to coverage from#188 and#201.