Skip to content
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

also produce standard .coverage file with .testmondata #86

Closed
webknjaz opened this issue Jan 7, 2018 · 23 comments
Closed

also produce standard .coverage file with .testmondata #86

webknjaz opened this issue Jan 7, 2018 · 23 comments

Comments

@webknjaz
Copy link
Contributor

webknjaz commented Jan 7, 2018

I've added --testmon flag to addopts in pytest.ini. This immediately reduced coverage from 60% to 18%:
https://codecov.io/gh/cherrypy/cheroot/branch/master/commits

P.S. Related feature request would be to support --no-testmon disabling the plugin.
P.P.S. Coverage is calculated using pytest-cov plugin

@tarpas
Copy link
Owner

tarpas commented Jan 8, 2018

Yeah, unfortunatelly coveragepy doesn't handle reporting coverage of the same run to more independent callers.

P.S. there is --testmon-off for exactly your use case.
P.P.S.: I doubt using testmon in CI environment is worth it. You'll run into edge cases and you'll have doubt if testmon is the cause. I think there would be stricter test isolation needed in the runs where it would be very useful.

@tarpas
Copy link
Owner

tarpas commented Jan 8, 2018

Which brings me to revelation that making coveragepy properly "re-entrant" would help also with issue #11 "dogfooding" (using testmon when developing testmon) which would be beautiful. A quick search doesn't find anything in coveragepy issue tracker: https://bitbucket.org/ned/coveragepy/issues?q=re-entrant I'll try to bring it up when I have time.

@webknjaz
Copy link
Contributor Author

But how does pytest-cov solve this?

@webknjaz
Copy link
Contributor Author

Any development on this so far?

@webknjaz
Copy link
Contributor Author

Any progress on this?

@blueyed
Copy link
Contributor

blueyed commented Apr 10, 2018

Just for reference, there's -p no:testmon with pytest also.

@tarpas tarpas changed the title Coverage data is corrupted with --testmon flag also produce standard .coverage file with .testmondata Apr 21, 2018
@tarpas
Copy link
Owner

tarpas commented Apr 21, 2018

#97 would solve the issue in general way

@tarpas
Copy link
Owner

tarpas commented Apr 21, 2018

@webknjaz pytest-cov doesn't solve it either. You have to pick one of pytest-cov, pytest-testmon, Pycharm debugger. I renamed the issue, because it is a requested feature but, I don't have capacity to work on it in the near future.

@webknjaz
Copy link
Contributor Author

I see, thanks for the update!

webknjaz added a commit to cherrypy/cheroot that referenced this issue Apr 21, 2018
pytest-testmon is incompatible with pytest-cov now, because they
compete on using sys.settrace hook

Refs:
* tarpas/pytest-testmon#86
* tarpas/pytest-testmon#97
* https://coverage.readthedocs.io/en/coverage-4.5.1/trouble.html#things-that-don-t-work
@webknjaz
Copy link
Contributor Author

I've decided to go for workaround suggested by @blueyed for now (cherrypy/cheroot@26eea5ca).
I think you can close this issue in favor of #97 if you want.

@webknjaz
Copy link
Contributor Author

@blueyed -p no:testmon doesn't work for some reason: https://travis-ci.org/cherrypy/cheroot/jobs/369489431#L472

@blueyed
Copy link
Contributor

blueyed commented Apr 21, 2018

@webknjaz

pytest: error: unrecognized arguments: --testmon

With -p no:testmon this makes sense.

@blueyed
Copy link
Contributor

blueyed commented Apr 21, 2018

@webknjaz
Copy link
Contributor Author

I wanted to keep it enabled via addopts and disable in one place in cli

@blueyed
Copy link
Contributor

blueyed commented Apr 21, 2018

@webknjaz
Then you might want to use --testmon-off instead. -p no:testmon skips testmon entirely, and then the --testmon option is not available.
But I would not enable it by default myself - I am using a ptm alias to run tests using testmon, and otherwise often use -k or selection via files / ids directly.

@webknjaz
Copy link
Contributor Author

Thanks for the hint! I've completely missed that.

@AbdealiLoKo
Copy link

Hi, I had evaluated testmon for my project when I was looking for a solution to improve my pytest CI speed by using some form of caching across PullRequest runs and came across testmon.

This issue was a blocker for me at that time too.
We generate a coverage report for each PullRequest and ensure developers never reduce coverage compared to master using tools like codecov.

I recall that coverage 5 and how it creates a database and how that impacts stuff was uncertain at that time.

I went through multiple issue here and on coverage.py github. But thought I'd ask on what is the final status here ...

Today: Is it possible for my PullRequest runs to run coverage and testmon together ? So that:

  • Testmon can speed up my CI
  • and Coverage can ensure developers are writing sufficient tests ?

@tarpas
Copy link
Owner

tarpas commented Oct 24, 2022

With testmon v 1.4.0+ it's easy to produce .testmondata and .coverage within the same run. However, there are still 2 issues, which are more on the side of coveragepy:

  • coverage with contexts is not the default setting and it doesn't have satisfactory performance of merging .coverage files
  • even if you accept that, any line additions or deletions make the .coverage for subsequent lines invalid

Depending on your project it might still be a good tradeoff to give up on coverage reporting and gain a boost in performance for some branches/commits.

@AbdealiLoKo
Copy link

AbdealiLoKo commented Oct 25, 2022

I see - the point on line adds/dels making .coverage for subsequent lines invalid makes sense and seems like a core issue tough to solve.

Then is it possible to run the tests for the altered lines "first" and then run the remaining tests ?
That way CI tests can fail fast in case of errors and run the entire suite + generate the entire coverage if no failures

(In my experience 80% of CI runs at a PR level are failures)

NOTE: Even better would be to just invalidate that file's coverage and rerun all tests for that file... But I assume that is not possible right now.

@tarpas
Copy link
Owner

tarpas commented Oct 25, 2022

Then is it possible to run the tests for the altered lines "first" and then run the remaining tests ?

Yes. --testmon-noselect does exactly that.

That way CI tests can fail fast in case of errors and run the entire suite + generate the entire coverage if no failures

Yes, this is the sweetspot of using coverage + testmon as it's designed with current limits.

NOTE: Even better would be to just invalidate that file's coverage and rerun all tests for that file... But I assume that is not possible right now.

Not possible now but a neat idea. Tougher challenge is the .coverage data merge performance.. I would like to get feedback on the approach as outlined above, then think about implementation of "execute all test for affected files"

@indiVar0508
Copy link

Hi @tarpas ,

Yes. --testmon-noselect does exactly that. - is there a way to order all tests and prioritize them in order of most likely to fail along with flag no-collection (--testmon-nocollect), the way i understand till now is that
--testmon-noselect - executes all test but order most likely to fail tests first
--testmon-nocollect - executes affected tests but doesn't do collection in testmon files?

what i want to achieve is kind combination of both like do all selection execute all tests order them into most likely to fail first but do no collection

i can't just use --testmon-noselect as it doesn't work with branch coverage and using both noselect and nocollect disables testmon itself

@tarpas
Copy link
Owner

tarpas commented Nov 7, 2023

It's been possible to use --cov and --testmon simultaneously for a long time. Now it's also possible to only reorder tests without selecting or collecting .testmondata. (re later discussion)

@tarpas tarpas closed this as completed Nov 7, 2023
@webknjaz
Copy link
Contributor Author

webknjaz commented Nov 7, 2023

Oh, really? I must've missed when that happened.. Will have to try integrating this again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants