Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged

Conversation

@nth10sd
Copy link
Contributor

This activates tests for code related to coverage from#188 and#201.

@codecov-io
Copy link

codecov-io commentedJul 13, 2018
edited
Loading

Codecov Report

Merging#202 intomaster willincrease coverage by11.84%.
The diff coverage is100%.

Impacted file tree graph

@@             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
Impacted FilesCoverage Δ
tests/test_run_ccoverage.py100% <100%> (+15.38%)⬆️
src/funfuzz/ccoverage/gatherer.py100% <100%> (+64.51%)⬆️
tests/ccoverage/test_gatherer.py100% <100%> (ø)
src/funfuzz/js/link_fuzzer.py100% <0%> (+6.66%)⬆️
src/funfuzz/js/shell_flags.py95.36% <0%> (+7.94%)⬆️
src/funfuzz/util/os_ops.py35.71% <0%> (+18.68%)⬆️
src/funfuzz/js/js_interesting.py54.08% <0%> (+36.22%)⬆️
src/funfuzz/util/create_collector.py80% <0%> (+50%)⬆️
... and4 more

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update6869db9...7a6366b. Read thecomment docs.

@nth10sd
Copy link
ContributorAuthor

I've had a lot of changes since, and I think I've also addressed your comments. How about another quick glance?

Copy link
Collaborator

@jschwartzentruberjschwartzentruber left a 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
Copy link
Collaborator

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(...)

Copy link
ContributorAuthor

@nth10sdnth10sdJan 29, 2019
edited
Loading

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.

Copy link
Collaborator

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.

Copy link
ContributorAuthor

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?

Copy link
ContributorAuthor

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.

Copy link
ContributorAuthor

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?

Copy link
ContributorAuthor

@nth10sdnth10sdJan 30, 2019
edited
Loading

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.

Copy link
Collaborator

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) < 1

Copy link
ContributorAuthor

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!

monkey=MonkeyPatch()
withmonkey.context()asmonkey_context:
monkey_context.setattr(gatherer,"RUN_COV_TIME",3)
monkey_context.setattr("funfuzz.ccoverage.reporter.report_coverage",
Copy link
Collaborator

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"] == 1

Copy link
ContributorAuthor

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+=.

Copy link
Collaborator

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.

@nth10sdnth10sd merged commit0c76245 intoMozillaSecurity:masterJan 30, 2019
@nth10sdnth10sd deleted the activate-ccoverage-test branchJanuary 30, 2019 22:04
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jschwartzentruberjschwartzentruberjschwartzentruber approved these changes

Assignees

@nth10sdnth10sd

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nth10sd@codecov-io@jschwartzentruber

[8]ページ先頭

©2009-2025 Movatter.jp