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

Improve py.test covered paths reporting. #5534

Merged
merged 1 commit into from
Mar 1, 2018

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented Mar 1, 2018

Use a coverage plugin instead of coverage combine to open the door to
using a single PEX test source chroot even when testing against multiple
repo source roots at once.

Fixes #5314
Needed for #5426
Depends on #5420

@jsirois
Copy link
Contributor Author

jsirois commented Mar 1, 2018

Reviewers: e7afb7f is the relevant diff against #5420

jsirois added a commit to jsirois/pants that referenced this pull request Mar 1, 2018
This change reverts the majority of pantsbuild#5400 (GatherSources) and adjusts
the new coverage plugin path mapping system to handle mapping a single
source chroot back to potentially multiple source roots. The existing
tests of this  capability introduced in pantsbuild#5400 still pass and as a result
the previously conflicting use cases represented by pantsbuild#5314 (multiple
library/binary source roots with interdependencies under test) and
pantsbuild#5426 (parallel test packages in a test source root) are now both
satisfied.

Fixes pantsbuild#5314
Fixes pantsbuild#5426
Depends on pantsbuild#5534
Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. The My prefixes are fun, but easy to resolve later.

# environment.
pythonpath = env.pop('PYTHONPATH', None)
if pythonpath:
self.context.log.warn('scrubbed PYTHONPATH={} from py.test environment'.format(pythonpath))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably there is there some other scrubbing happening higher in the stack that will prevent this warning from triggering spuriously in a venv, for example? Easy to remove later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is not! When you pass a custom env to pex.run it accepts you know what you're doing.

wisechengyi pushed a commit that referenced this pull request Mar 1, 2018
This change reverts the majority of #5400 (GatherSources) and adjusts
the new coverage plugin path mapping system to handle mapping a single
source chroot back to potentially multiple source roots. The existing
tests of this  capability introduced in #5400 still pass and as a result
the previously conflicting use cases represented by #5314 (multiple
library/binary source roots with interdependencies under test) and
#5426 (parallel test packages in a test source root) are now both
satisfied.

Fixes #5314
Fixes #5426
Depends on #5534
Use a coverage plugin instead of `coverage combine` to open the door to
using a single PEX test source chroot even when testing against multiple
repo source roots at once.

Needed for pantsbuild#5426
Depends on pantsbuild#5420
@jsirois jsirois merged commit 052bdbb into pantsbuild:master Mar 1, 2018
@jsirois jsirois deleted the issues/5426 branch March 1, 2018 22:17
jsirois added a commit to jsirois/pants that referenced this pull request Mar 1, 2018
This change reverts the majority of pantsbuild#5400 (GatherSources) and adjusts
the new coverage plugin path mapping system to handle mapping a single
source chroot back to potentially multiple source roots. The existing
tests of this  capability introduced in pantsbuild#5400 still pass and as a result
the previously conflicting use cases represented by pantsbuild#5314 (multiple
library/binary source roots with interdependencies under test) and
pantsbuild#5426 (parallel test packages in a test source root) are now both
satisfied.

Fixes pantsbuild#5314
Fixes pantsbuild#5426
Depends on pantsbuild#5534
Copy link
Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having shallow-dived into coverage at one point, I appreciate the pain of this deep dive... Thanks!

jsirois added a commit that referenced this pull request Mar 2, 2018
This change reverts the majority of #5400 (GatherSources) and adjusts
the new coverage plugin path mapping system to handle mapping a single
source chroot back to potentially multiple source roots. The existing
tests of this  capability introduced in #5400 still pass and as a result
the previously conflicting use cases represented by #5314 (multiple
library/binary source roots with interdependencies under test) and
#5426 (parallel test packages in a test source root) are now both
satisfied.

Fixes #5314
Fixes #5426
Depends on #5534
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

Successfully merging this pull request may close these issues.

3 participants