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

Avoid using source keyword when starting coverage #19

Open
ghackebeil opened this issue Mar 13, 2018 · 3 comments
Open

Avoid using source keyword when starting coverage #19

ghackebeil opened this issue Mar 13, 2018 · 3 comments

Comments

@ghackebeil
Copy link

ghackebeil commented Mar 13, 2018

First, thanks for putting together this package. It is great!

Context

I'm running into an issue trying to expand the coverage output to include my examples and tests directories. My package layout currently looks like:

mypackage/run-mpitests.py
mypackage/.coveragerc
mypackage/src/mypackage
mypackage/src/tests
mypackage/examples

I am executing my tests from the top-level directory as follows:

python run-mpitests.py --no-build --with-coverage --cov-config=.coveragerc

And my .coveragerc file looks like:

[run]
source =
    # pytest execution through run-mpitests.py
    # (tests are executed within ./build/tests)
    ../../src/mypackage
    ../../src/tests
    ../../examples

Issue

The final coverage report only includes files under mypackage/src/mypackage. I believe the issue is that the source keyword is being assigned something when coverage.coverage is started in runtests.coverage.py.__enter__. If I change that method to the following, I can obtain the coverage report I need.

    def __enter__(self):

	if not self.with_coverage:
            self.cov = None
            return
        else:
           # changed here: source is not set
            self.cov = coverage.coverage(config_file=self.config_file)
            self.cov.start()

Thoughts? Is there a better way to get around this issue?

@rainwoodman
Copy link
Member

@nickhand knows better of the coverage module. It sounds like runtests overrides your coveragerc settings of source directory. We can probably get rid of that argument to coverage as you suggested. It appears the only additional requirement is that a .coveragerc becomes necessary?

I wonder what is your run-mpitest.py like? self.source should point to the 'module' argument (last argument) of the Tester constructor. In this case it sounds like it should have been the src directory? We never used in a src/package directory layout before, so I do worry this implies something deeper that needs to be fixed.

@ghackebeil
Copy link
Author

My current run-mpitests.py looks like:

import sys
from runtests.mpi import Tester
import os.path
tester = Tester(os.path.join(os.path.abspath(__file__)),
                "mypackage",
                extra_path=None)
tester.main(sys.argv[1:])

Changing "mypackage" to "../../src" in the above does seem to solve half the issue, but I'm still missing coverage for "examples". Taking this a little further, you could allow something like

tester = Tester(os.path.join(os.path.abspath(__file__)),
                ["../../src","../../examples"],
                extra_path=None)

if you changed runtests.coverage.py.__enter__ to something like:

    def __enter__(self):

	if not self.with_coverage:
            self.cov = None
            return
        else:
            source = None
            if self.source is not None:
                if isinstance(self.source, six.string_types):
                    source = [self.source]
                else:
                    # assume it is a list
                    source = self.source
            self.cov = coverage.coverage(source=source, config_file=self.config_file)
            self.cov.start()

I suppose there is still the fundamental issue of source lines in a user-provided .coveragerc being ignored though.

@rainwoodman
Copy link
Member

I thought the main purpose of the "mypackage" argument was to allow importing the module for testing in the case that a testing module is not specified -- it goes back to the days when we were using nosetests. It means it wasn't intended to take a path name.

But with pytest the module name use case is likely no longer relevant. It still looks weird to have '../' in the command-line argument when the actual directory you care relative to the script is not in ../.

I am leaning towards the solution that doesn't override coveragerc with self.source at all. We may break a few downstream packages, but it is an easy enough fix. We can probably even monkey patch the paths after coveragerc is loaded to avoid '../'s there.

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

No branches or pull requests

2 participants