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.

Sign up for GitHub

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

Do not generate reports with no coverage #17293

Merged
merged 1 commit into from
Dec 3, 2020
+23 −8
Merged

Do not generate reports with no coverage #17293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions eng/CodeCoverage.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,34 @@
</ItemGroup>

<!-- Clean up previous TestResults so reports are recent. -->
<Target Name="CleanPreviousCodeCoverage"
<Target Name="_CleanPreviousCodeCoverage"
BeforeTargets="VSTest"
Condition="'$(_CollectCoverage)' == 'true'">
<RemoveDir Directories="$(_TestResultsDirectory)" />
</Target>

<!-- Set a variable to indicate that a report can be generated. -->
<Target Name="_SetCoverageCollected"
AfterTargets="VSTest"
Condition="'$(_CollectCoverage)' == 'true'">
<ItemGroup>
<_CoverageFiles Include="$(_TestResultsDirectory)\**\coverage.cobertura.xml" />
</ItemGroup>
<PropertyGroup>
<_CoverageCollected Condition="'@(_CoverageFiles)' != ''">true</_CoverageCollected>
</PropertyGroup>

<!-- Indicate to the pipeline a report can be generated. -->
<Message Importance="high"
Condition="'$(_CoverageCollected)' == 'true' and '$(TF_BUILD)' == 'true'"
Text="##vso[task.setvariable variable=coverage.collected]true" />
</Target>

<!-- Should be similar to what's in the pipelines, though generate a full HTML report. -->
<Target Name="GenerateCodeCoverageReport"
AfterTargets="VSTest"
Condition="'$(_CollectCoverage)' == 'true' and '$(SkipCoverageReport)' != 'true'">
DependsOnTargets="_SetCoverageCollected"
Condition="'$(_CollectCoverage)' == 'true' and '$(SkipCoverageReport)' != 'true' and '$(_CoverageCollected)' == 'true'">
<PropertyGroup>
<CoverageReportCommandLine>dotnet tool run reportgenerator --</CoverageReportCommandLine>
<CoverageReportCommandLine>$(CoverageReportCommandLine) "-reports:$(_TestResultsDirectory)\**\coverage.cobertura.xml"</CoverageReportCommandLine>
Expand Down
9 changes: 3 additions & 6 deletions eng/pipelines/templates/jobs/archetype-sdk-client.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ jobs:
condition: and(succeededOrFailed(), ne(variables['Skip.Test'], true))
variables:
- template: ../variables/globals.yml
- name: disable.coverage.autogenerate
value: true
strategy:
maxParallel: $[ variables['MaxParallelTestJobs'] ]
matrix:
Expand Down Expand Up @@ -167,18 +165,17 @@ jobs:
testResultsFormat: "VSTest"
mergeTestResults: true
- task: Palmmedia.reportgenerator.reportgenerator-build-release-task.reportgenerator@4
condition: and(succeededOrFailed(), eq(variables['CollectCoverage'], 'true'))
condition: and(succeededOrFailed(), eq(variables['CollectCoverage'], 'true'), eq(variables['coverage.collected'], 'true'))
displayName: Generate Code Coverage Reports
inputs:
reports: $(Build.SourcesDirectory)\sdk\${{parameters.ServiceDirectory}}\**\coverage.cobertura.xml
targetdir: $(Build.ArtifactStagingDirectory)\coverage
reporttypes: Cobertura;HtmlInline_AzurePipelines
reporttypes: Cobertura
filefilters: +$(Build.SourcesDirectory)\sdk\${{parameters.ServiceDirectory}}\**
verbosity: Verbose
- task: PublishCodeCoverageResults@1
condition: and(succeededOrFailed(), eq(variables['CollectCoverage'], 'true'))
condition: and(succeededOrFailed(), eq(variables['CollectCoverage'], 'true'), eq(variables['coverage.collected'], 'true'))
displayName: Publish Code Coverage Reports
inputs:
codeCoverageTool: Cobertura
summaryFileLocation: $(Build.ArtifactStagingDirectory)\coverage\Cobertura.xml
reportDirectory: $(Build.ArtifactStagingDirectory)\coverage
Copy link
Member

Choose a reason for hiding this comment

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

Did you remove this intentionally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. Just a bit of clean up while I was at it (it didn't have the desired effect).

Copy link
Member Author

Choose a reason for hiding this comment

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

...to note, the idea behind it was to generate a better report than what Azure did because Azure used to use an older version of reportgenerator implicitly in the publish step, but turns out it as new now. Somehow, this also caused an "empty" Azure.Core.TestFramework assembly to show up. It didn't impact the overall coverage numbers, but was a waste of space and potentially confusing since I did a bunch of work to exclude it from coverage.