Skip to content

new code coverage #5169

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

Merged
merged 11 commits into from
May 30, 2020
Merged

new code coverage #5169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
189cbda
test new code coverage
frank-dong-ms-zz May 27, 2020
a6fb2bc
change coverage path
frank-dong-ms-zz May 27, 2020
c66440c
fix failure
frank-dong-ms-zz May 27, 2020
fd8af95
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
frank-dong-ms-zz May 27, 2020
3aa52de
try test code coverage
frank-dong-ms-zz May 27, 2020
4136e38
fix build
frank-dong-ms-zz May 28, 2020
a55a2e0
try new file filters
frank-dong-ms-zz May 28, 2020
d7e608f
fix coverage report path
frank-dong-ms-zz May 29, 2020
19902f5
fix coverage report path
frank-dong-ms-zz May 29, 2020
4b5a213
try remove GC.Collect
frank-dong-ms-zz May 29, 2020
6ec9759
remove unnecessary new line
frank-dong-ms-zz May 29, 2020
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
13 changes: 13 additions & 0 deletions build.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
$(TraversalBuildDependsOn);
DownloadExternalTestFiles;
DownloadTensorflowMetaFiles;
DeleteTestHost;
</TraversalBuildDependsOn>
</PropertyGroup>

Expand Down Expand Up @@ -116,6 +117,18 @@
</DownloadFile>
</Target>

<!-- Delete testhost.dll and testhost.exe from output folder,
start test from dotnet.exe to keep consistent behavior with older version of Microsoft.NET.Test.Sdk -->
<Target Name="DeleteTestHost">
<Message Importance="High" Text="Delete testhost.dll and testhost.exe from output folder..." />
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Why is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also related to upgrade package "Microsoft.NET.Test.Sdk". Before upgrade the package, testhost.dll and testhost.exe are not copied to output path and tests are all starting from dotnet.exe. After upgrade package, tests will starts from testhost which will cause 2 issues:

  1. RemoteExecutor will not able to start new process as below line will get testhost instead of dotnet: https://github.com/dotnet/machinelearning/blob/master/test/Microsoft.ML.TestFramework/RemoteExecutor.cs#L29
  2. For x86 tests, we should use testhost.x86 instead of testhost directly, this should be achieved by set /p:Configuration instead of buildArch(which is ML.NET currently using), but setting Configuration will modify the build behavior a lot and corrupt the ML.NET build system.

So, based on these 2 reason, I'm add this target to remove testhost after build to keep consistent behavior as before upgrade test sdk package.

In reply to: 432262763 [](ancestors = 432262763)

<ItemGroup>
<FilesToClean Include="$(MSBuildThisFileDirectory)\bin\**\testhost.dll" />
<FilesToClean Include="$(MSBuildThisFileDirectory)\bin\**\testhost.exe" />
</ItemGroup>
<Delete Files="@(FilesToClean)"/>
<RemoveDir Directories="@(FoldersToClean)" />
</Target>

<Target Name="RunTests">
<MSBuild Projects="test\run-tests.proj"
Targets="RunTests" />
Expand Down
4 changes: 2 additions & 2 deletions build/Codecoverage.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
<_ReportGeneratorPath>$(PkgReportGenerator)\tools\net47\ReportGenerator.exe</_ReportGeneratorPath>
</PropertyGroup>

<Message Importance="high" Text="&quot;$(_ReportGeneratorPath)&quot; -reports:$(BaseOutputPath)$(PlatformConfig)\coverage\*.coverage -targetdir:$(BaseOutputPath)$(PlatformConfig)\coverage -filefilters:+https*;+*.fs -reporttypes:Cobertura" />
<Exec Command="&quot;$(_ReportGeneratorPath)&quot; -reports:$(BaseOutputPath)$(PlatformConfig)\coverage\*.coverage -targetdir:$(BaseOutputPath)$(PlatformConfig)\coverage -filefilters:+https*;+*.fs -reporttypes:Cobertura" />
<Message Importance="high" Text="&quot;$(_ReportGeneratorPath)&quot; -reports:$(BaseOutputPath)$(PlatformConfig)\*\*\coverage.opencover.xml -targetdir:$(BaseOutputPath)$(PlatformConfig)\coverage -filefilters:+*.cs;+*.fs -reporttypes:Cobertura" />
<Exec Command="&quot;$(_ReportGeneratorPath)&quot; -reports:$(BaseOutputPath)$(PlatformConfig)\*\*\coverage.opencover.xml -targetdir:$(BaseOutputPath)$(PlatformConfig)\coverage -filefilters:+*.cs;+*.fs -reporttypes:Cobertura" />

<ItemGroup>
<_CodecovArgs Include="-f;$(BaseOutputPath)$(PlatformConfig)\coverage\Cobertura.xml" />
Expand Down
2 changes: 1 addition & 1 deletion build/Dependencies.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
<PropertyGroup>
<PublishSymbolsPackageVersion>1.0.0-beta-62824-02</PublishSymbolsPackageVersion>
<CodecovVersion>1.9.0</CodecovVersion>
<CoverletVersion>2.7.0</CoverletVersion>
<CoverletCollectorVersion>1.2.1</CoverletCollectorVersion>
<ReportGeneratorVersion>4.3.6</ReportGeneratorVersion>
<MicrosoftDotNetApiCompatPackageVersion>1.0.0-beta.19225.5</MicrosoftDotNetApiCompatPackageVersion>
</PropertyGroup>
Expand Down
22 changes: 2 additions & 20 deletions test/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.8.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.6.1" />
<PackageReference Include="MSTest.TestFramework" Version="2.1.0" />
<PackageReference Include="MSTest.TestAdapter" Version="2.1.0" />
<PackageReference Include="xunit" Version="2.4.0" />
<PackageReference Include="Xunit.Combinatorial" Version="$(XunitCombinatorialVersion)" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.0" />
<PackageReference Include="Microsoft.DotNet.XUnitExtensions" Version="2.4.0-prerelease-63213-02" />
<PackageReference Include="coverlet.msbuild" Version="$(CoverletVersion)" />
<PackageReference Include="coverlet.collector" Version="$(CoverletCollectorVersion)" />
</ItemGroup>

<ItemGroup>
Expand All @@ -54,22 +54,4 @@
</ProjectReference>
</ItemGroup>

<PropertyGroup Condition="'$(Coverage)' == 'true'">
<!-- https://github.com/tonerdo/coverlet/issues/618 -->
<IncludeTestAssembly>true</IncludeTestAssembly>

<CollectCoverage>true</CollectCoverage>
<SingleHit>true</SingleHit>
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies> <!-- https://github.com/tonerdo/coverlet/issues/72 -->
<CoverletOutputFormat>opencover</CoverletOutputFormat>
<CoverletOutput>$(BaseOutputPath)$(PlatformConfig)\coverage\$(MSBuildProjectName).coverage</CoverletOutput>
<Include>[Microsoft.ML.*]*</Include>
<!-- Excluding for perf reasons. These classes have tests that can be run conditionally
but they need to be migrated. Excluding these classes should have very minimal effect on code coverage.
-->
<Exclude>[*]Microsoft.ML.*Contracts*,[*]Microsoft.ML.Internal.Utilities*,[*]Microsoft.ML.Data.VBuffer*</Exclude>
<ExcludeByAttribute>Obsolete,ExcludeFromCodeCoverage</ExcludeByAttribute>
<ExcludeByFile>$(RepoRoot)src\Microsoft.ML.OnnxConverter\OnnxMl.cs,$(RepoRoot)src\Microsoft.ML.TensorFlow\TensorFlow\Buffer.cs,$(RepoRoot)src\Microsoft.ML.TensorFlow\TensorFlow\Tensor.cs,$(RepoRoot)src\Microsoft.ML.TensorFlow\TensorFlow\Tensorflow.cs</ExcludeByFile>
</PropertyGroup>

</Project>
18 changes: 18 additions & 0 deletions test/coverlet.runsettings
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8" ?>
<RunSettings>
<DataCollectionRunSettings>
<DataCollectors>
<DataCollector friendlyName="XPlat code coverage">
<Configuration>
<Format>opencover</Format>
<Exclude>[*]Microsoft.ML.*Contracts*,[*]Microsoft.ML.Internal.Utilities*,[*]Microsoft.ML.Data.VBuffer*</Exclude> <!-- [Assembly-Filter]Type-Filter -->
<Include>[Microsoft.ML.*]*</Include> <!-- [Assembly-Filter]Type-Filter -->
<ExcludeByAttribute>Obsolete,ExcludeFromCodeCoverage</ExcludeByAttribute>
<ExcludeByFile>$(RepoRoot)src\Microsoft.ML.OnnxConverter\OnnxMl.cs,$(RepoRoot)src\Microsoft.ML.TensorFlow\TensorFlow\Buffer.cs,$(RepoRoot)src\Microsoft.ML.TensorFlow\TensorFlow\Tensor.cs,$(RepoRoot)src\Microsoft.ML.TensorFlow\TensorFlow\Tensorflow.cs</ExcludeByFile> <!-- Globbing filter -->
<SingleHit>true</SingleHit>
<IncludeTestAssembly>true</IncludeTestAssembly>
</Configuration>
</DataCollector>
</DataCollectors>
</DataCollectionRunSettings>
</RunSettings>
7 changes: 6 additions & 1 deletion test/run-tests.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@

<Target Name="RunTests">
<Message Importance="High" Text="Running tests ..." />
<MSBuild Targets="VSTest"
<MSBuild Condition="'$(Coverage)' == 'true'"
Targets="VSTest"
Projects="@(Project)"
Properties="VSTestNoBuild=true;VSTestBlame=true;VSTestCollect=XPlat Code Coverage;VSTestSetting=$(MSBuildThisFileDirectory)\coverlet.runsettings" />
<MSBuild Condition="'$(Coverage)' == 'false'"
Targets="VSTest"
Projects="@(Project)"
Properties="VSTestNoBuild=true;VSTestBlame=true" />
</Target>
Expand Down