Consider .rs files for coverage by default - #3578
Open
jackmarsh wants to merge 2 commits into
Open
Conversation
SubrepoFS looks a subrepo's output tree up by label and passes whatever it
finds to remotefs.New:
tree := c.subrepoTrees[target.Label]
return remotefs.New(c.remoteFSClient, tree, root)
subrepoTrees is populated only when a target's outputs are set from a remote
build, so a subrepo built locally, served from cache, or not yet built has
no entry and tree is nil. New then dereferences it while indexing the tree's
children, segfaulting at addr=0x18 during readSubrepoConfig, which calls
Subrepo.FS() unconditionally to look for .plzconfig.
A nil tree (or one without a root) now yields an empty filesystem, so the
lookup reports that the file does not exist and parsing carries on, which is
already how a subrepo without a .plzconfig is handled.
Rust coverage output is parsed fine, but every file in it is dropped unless the repo adds .rs to this list, so coverage silently reports no data at all rather than failing in a way that points at the cause. Every other language Please knows how to cover has its extension here already. Reported after a Rust plugin's coverage support looked broken in three separate repos, each of which needed the same line of config.
Collaborator
|
Makes sense. You can of course set it in config, but I think maybe you then have to list everything out again, and there's no real cost to having this as a default. Looks like you have #3577 on this branch as well? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please parses Rust coverage output correctly (the Rust plugin emits go-cover format), but
Cover.FileExtensionhas no.rs, socollectCoverageFilesfilters every Rust file out. The result is a coverage run that reports:with every file at 0%, which reads as "coverage is broken" rather than "add a line of config". Every other language Please covers has its extension in this list already.
Adding it means Rust repos get working coverage without each of them discovering the same workaround. Three separate repos hit this before the cause was identified.
plz test //src/core:core_testis green (279 tests).