Add File Coverage to coverage data #4556
Conversation
f1f2aa4 to
f96d5e4
Compare
|
the test failures seem to be flaky tests |
|
I've triggered AppVeyor to run again |
| { | ||
| param ( | ||
| [Parameter(Position=0,Mandatory=$true,ValueFromPipeline=$true)]$FileCoverageData, | ||
| [Parameter()][string]$oldBase = "", |
There was a problem hiding this comment.
Should be have a default value for this?
There was a problem hiding this comment.
under most circumstances (for example, when running on the system where the data was collected), it's not needed at all, so I don't want to do any substitution. By setting it to default of "" and "" (for old and new) no substitution takes place and everything works as expected.
There was a problem hiding this comment.
There is precedence here with Invoke-OpenCover, we try to find powershell root directory and build path.
There was a problem hiding this comment.
I believe it's the wrong thing to do. This may be used completely outside of a repo unlike invoke-opencover. consider the scenario inspecting coverage on a remote machine where the paths are not the same as what is in the file. so I can say -oldbase f:/github/blah -newwbase \\system\share\blah If you're on the system where you collected the data, you don't to substitute anything, doing a replacement is unneeded. In this scenario (which is quite real, since I work on mac). The path in the log is wrong for me, and my repo may not be the same as where the coverage was gathered. I don't need a lot of complicated, fragile logic when I can just supply it.
| param ( | ||
| [Parameter(Position=0,Mandatory=$true,ValueFromPipeline=$true)]$FileCoverageData, | ||
| [Parameter()][string]$oldBase = "", | ||
| [Parameter()][string]$newBase = "", |
There was a problem hiding this comment.
Maybe use git to find project root for the default value?
There was a problem hiding this comment.
see above - generally don't want to change the path, but sometimes I do. setting this to "" makes it work nearly all the time, but when I'm working on my mac which has different path structure, I can still look at the coverage data. In this case I may not be in a repo at all.
There was a problem hiding this comment.
See above about precedence in Invoke-OpenCover.
test/tools/OpenCover/OpenCover.psm1
Outdated
| ) | ||
|
|
||
| $files = @($fileCoverageData.Keys) | Where-Object { $_ -match $filter } | ||
| write-verbose -verbose $files |
There was a problem hiding this comment.
Do we always want a verbose message or a Write-Host -Fore Yellow?
There was a problem hiding this comment.
probably not - removed
f96d5e4 to
889551a
Compare
|
@adityapatwardhan updated and pushed |
|
restarted appveyor due to a myget failure |
|
@TravisEz13 would you be having a look at this? |
test/tools/OpenCover/OpenCover.psm1
Outdated
| @@ -134,7 +299,8 @@ function Get-CoverageData($xmlPath) | |||
| CoverageLogFile = $xmlPath | |||
| CoverageSummary = (Get-CoverageSummary -element $CoverageXml.CoverageSession.Summary) | |||
| Assembly = $assemblies | |||
| } | |||
| FileCoverage = Get-FileCoverageData $CoverageXml | |||
| } | |||
There was a problem hiding this comment.
Can you please fix this indentation?
There was a problem hiding this comment.
certainly - commit amended and pushed
There was a problem hiding this comment.
@daxian-dbw Can you take a quick look?
It is now possible to see the coverage based on the file and then format the data so a report generator is not needed. You can map the files from one location to another, so if you have a copy of the repo, but in a different location, you can still format the output in a way so it may be studied
889551a to
6913de8
Compare
It is now possible to see the coverage based on the file and then format
the data so a report generator is not needed. You can map the files from
one location to another, so if you have a copy of the repo, but in a different
location, you can still format the output in a way so it may be studied
Also, help is available for Format-FileCoverage!