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

default support for $COVERAGE_PROCESS_START #378

Closed
nedbat opened this issue Jun 23, 2015 · 8 comments
Closed

default support for $COVERAGE_PROCESS_START #378

nedbat opened this issue Jun 23, 2015 · 8 comments
Labels
duplicate This issue or pull request already exists enhancement New feature or request subprocess

Comments

@nedbat
Copy link
Owner

nedbat commented Jun 23, 2015

Originally reported by Buck Evan (Bitbucket: bukzor, GitHub: bukzor)


The essential idea is that there's this nifty feature in coverage, but it's too hard to use: http://nedbatchelder.com/code/coverage/subprocess.html

Plopping the pth file in the right spot isn't straightforward code that everyone can write without flaw. It should be a (tested) part of the coveragepy project.

I have a patch for this, but want to get your design guidance before I submit it. There's an interesting fact: there's no reliable way to uninstall a pth file in the case of easy-install or setup.py develop/pip install -e. The wheel and pip install cases work just fine, which are (I hope!) the usual cases.

Given that fact, what fallback should we (I) provide in these cases? There are three basic choices:

  1. Throw an error. I think this will break too many people, cause too much trouble, but you might like it best.
  2. Disable the $COVERAGE_PROCESS_START feature, possibly with a warning. This may be the most sane thing to do, but might be surprising.
  3. Design the .pth file such that it can be left behind (if coveragepy is uninstalled) without causing any error. This is 100% possible, but means that the install/uninstall aren't exactly inverse in the case of easy-install/develop. I believe this is the most user-friendly choice.

All three are fairly easy to implement. I just need you to pick your favorite, if you would.


@nedbat
Copy link
Owner Author

nedbat commented Jul 3, 2015

Original comment by Ionel Cristian Mărieș (Bitbucket: ionelmc, GitHub: ionelmc)


Is is correct to assume this is going to be something similar to what pytest-cov has?

PS. pytest-cov is going to get an upgrade to the pth installing/uninstalling thing, you might want to take a look over pytest-dev/pytest-cov#58

@nedbat
Copy link
Owner Author

nedbat commented Jul 8, 2015

Original comment by Buck Evan (Bitbucket: bukzor, GitHub: bukzor)


Correct, and I'm aware :)

Ideally closing this ticket would eliminate my need for pytest-cov and cov-core.
$COVERAGE_PROCESS_START already fills the use case of cov-core, it just needs enabled by some manual file munging. This ticket is to eliminate the manual process.

Note that cov-core#53 is filed by me.

@nedbat
Copy link
Owner Author

nedbat commented Jul 14, 2015

Original comment by Buck Evan (Bitbucket: bukzor, GitHub: bukzor)


@nedbat I'd really like to get your opinion on this.

@nedbat
Copy link
Owner Author

nedbat commented Jul 15, 2015

@bukzor I don't quite understand your three choices. In #1, you say, "throw an error". Throw it when? When trying to uninstall? In #2, again, disable it when?

In any case, option #3 does sound like the a fine option to me. Leaving behind a .pth file that does nothing seems OK, especially if there's a comment there apologizing and explaining why.

@nedbat
Copy link
Owner Author

nedbat commented Jul 15, 2015

Original comment by Buck Evan (Bitbucket: bukzor, GitHub: bukzor)


  1. Throw an error when trying to install something we can't uninstall, ie when using easy-install or setuptools develop.

  2. Simply don't install the pth file in those two cases. This leaves the process-coverage feature deactivated.

@nedbat
Copy link
Owner Author

nedbat commented Jul 15, 2015

@bukzor OK, thanks for clarifying. I definitely prefer #3.

@nedbat nedbat removed the 4.0 label Aug 17, 2018
@nedbat nedbat removed the major label Jan 18, 2020
@nedbat nedbat added enhancement New feature or request and removed proposal labels Oct 31, 2021
@nedbat
Copy link
Owner Author

nedbat commented Nov 4, 2022

This is related (duplicate?) of #367.

@nedbat
Copy link
Owner Author

nedbat commented Nov 6, 2022

I'll close this issue, and discussion can continue on #367.

@nedbat nedbat closed this as not planned Won't fix, can't repro, duplicate, stale Nov 6, 2022
@nedbat nedbat added duplicate This issue or pull request already exists and removed opinions needed Not sure how coverage.py should behave. Chime in! labels Nov 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists enhancement New feature or request subprocess
Projects
None yet
Development

No branches or pull requests

1 participant