AArnott / Library.Template

A template for a NuGet package with tests, stylecop, fxcop, versioning, and Azure Pipelines build ready to go.
MIT License
125 stars 26 forks source link

Two copies of code coverage files are being created. #161

Closed SteveBush closed 2 years ago

SteveBush commented 2 years ago

Two copies of a code coverage file are being created with the most recent changes. This is causing the report generator tool do double the work to dedupe the code coverage data.

Proposed fix in coverageResults.ps1 to remove duplicates from /In/ folder:

$coverageFiles = @(Get-ChildItem "$RepoRoot/test/*.cobertura.xml" -Recurse | Where {$_.FullName -notlike "*/In/*" -and $_.FullName -notlike "*\In\*" })

There may be a more elegant way to do this but this unblocked me.

AArnott commented 2 years ago

Thanks for reporting.

this unblocked me.

Was this actually blocking in some way? What led you to discover this in the first place? Was the added time actually significant in your build?

remove duplicates from /In/ folder

I'd prefer to not even generate the duplicates in the first place. @MarcoRossignoli any ideas?

SteveBush commented 2 years ago

Originally, the mergecoverage step timed out in my build (over 30 minutes). My investigation led me to discover the recent changes to the library template I picked up were the root cause. I first noticed the duplicate coverage files.

Unfortunately, removing the duplicate code coverage files didn't fixed the performance of my build as the mergecoverage step is still taking 15 minutes vs 30 seconds previously. I excluded some external packages (e.g. Moq) from my coverage results so it's just my project but that didn't help.

Across all of my agent and platform builds, I have 24 code coverage files, totaling roughly 70 MB worth of coverage data.

Currently, I'm debugging whether the substitution or call to report generator is the source of the delay by adding a Write-Host.

    Write-Host 'Substituting {reporoot} with $(System.DefaultWorkingDirectory)'
    $reports = Get-ChildItem -Recurse '$(Pipeline.Workspace)/*.cobertura.xml'
    $reports |% {
        # In addition to replacing {reporoot}, we also normalize on one kind of slash so that the report aggregates data for a file whether data was collected on Windows or not.
        $content = Get-Content -Path $_ |% { [Regex]::Replace($_, '{reporoot}([^"]+)', { '$(System.DefaultWorkingDirectory)' + $args[0].groups[1].value.replace([IO.Path]::AltDirectorySeparatorChar, [IO.Path]::DirectorySeparatorChar) }) }
        Set-Content -Path $_ -Value $content -Encoding UTF8
    }

    $Inputs = [string]::join(';', ($reports |% { Resolve-Path -relative $_ }))
    Write-Host 'Calling report generator'
AArnott commented 2 years ago

Wow. How large were your code coverage files before this change?

MarcoRossignoli commented 2 years ago

I think it's related to the by design behavior of trx logger with datacollectors inside CI(previous will be using coverlet msbuild integration, ignored by AzDo) https://github.com/microsoft/vstest/issues/2334#issuecomment-1140330331 solved here with a custom filtering https://developercommunity.visualstudio.com/t/vstestconsoleexe-produces-two-folders-/10041800

For the performance issue is it possible that now reports are bigger because they're containing more instrumented libraries? Coverlet was using a different heuristics to exclude files to instrument.

cc: @jakubch1 @fhnaseer

SteveBush commented 2 years ago

@MarcoRossignoli

The proposed fix only works with there is one level of test projects (e.g $RepoRoot/test/MyTestProject/TestResults. I have subfolders of test projects so I need to use -Recurse. The following code worked for me. It excludes any "In" folder on Windows and non-Windows which has the duplicate code coverage file.

@AArnott

The report generation takes 3 seconds. The path fixups is what is taking so long.

Get-ChildItem "$RepoRoot/test/*.cobertura.xml" -Recurse | Where {$_.FullName -notlike "*/In/*"  -and $_.FullName -notlike "*\In\*" }
AArnott commented 2 years ago

I'm working on the change now. Thank you so much for the feedback, @SteveBush and the proposed fix. I hope you'll take a look at the PR when it comes out.

AArnott commented 2 years ago

The path fixups is what is taking so long.

Are you talking about the new regex replacements in publish-codecoverage.yml? I wonder how cutting the file count by half drops time from minutes to just 3 seconds.