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.

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

Update to coverlet 2.6.0 #2843

Merged
merged 2 commits into from
Mar 12, 2019
Merged

Update to coverlet 2.6.0 #2843

merged 2 commits into from
Mar 12, 2019

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Mar 5, 2019

No description provided.

@codecov
Copy link

codecov bot commented Mar 5, 2019

Codecov Report

Merging #2843 into master will increase coverage by 0.35%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2843      +/-   ##
==========================================
+ Coverage   71.81%   72.17%   +0.35%     
==========================================
  Files         812      796      -16     
  Lines      142659   141949     -710     
  Branches    16090    16042      -48     
==========================================
  Hits       102450   102450              
+ Misses      35826    35117     -709     
+ Partials     4383     4382       -1
Flag Coverage Δ
#Debug 72.17% <ø> (+0.35%) ⬆️
#production 67.96% <ø> (ø) ⬆️
#test 88.32% <ø> (+2.08%) ⬆️
Impacted Files Coverage Δ
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 84.7% <0%> (-0.21%) ⬇️
test/Microsoft.ML.Benchmarks/Numeric/Ranking.cs
...st/Microsoft.ML.Benchmarks.Tests/BenchmarksTest.cs
...t/Microsoft.ML.Benchmarks/PredictionEngineBench.cs
...s/StochasticDualCoordinateAscentClassifierBench.cs
...est/Microsoft.ML.Benchmarks/Helpers/EmptyWriter.cs
test/Microsoft.ML.Benchmarks/CacheDataViewBench.cs
test/Microsoft.ML.Benchmarks/Harness/Metrics.cs
test/Microsoft.ML.Benchmarks/Helpers/Errors.cs
test/Microsoft.ML.Benchmarks/RffTransform.cs
... and 8 more

@sharwell sharwell force-pushed the update-coverlet branch 2 times, most recently from e7a3e4b to 953ae39 Compare March 5, 2019 13:57
@TomFinley TomFinley requested a review from codemzs March 5, 2019 15:17
@TomFinley
Copy link
Contributor

TomFinley commented Mar 5, 2019

If I pick a code coverage run arbitrarily, I see #2808 finished in 38m 13s. Yours finished in 23m 36s. That's a very positive perf difference!

Copy link
Contributor

@TomFinley TomFinley left a comment

Choose a reason for hiding this comment

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

Thank you @sharwell, at least at first glance this seems quite positive!

@sharwell
Copy link
Member Author

sharwell commented Mar 5, 2019

@TomFinley it's a substantial improvement in time, but some of the data seems to be missing. I'm still investigating.

zsd4yr added a commit to dotnet/winforms that referenced this pull request Mar 8, 2019
@sharwell sharwell force-pushed the update-coverlet branch 2 times, most recently from c032056 to e9758e2 Compare March 9, 2019 23:22
<Message Importance="high" Text="&quot;$(_ReportGeneratorPath)&quot; -reports:$(BaseOutputPath)$(PlatformConfig)\coverage\*.coverage -targetdir:$(BaseOutputPath)$(PlatformConfig)\coverage -reporttypes:Cobertura" />
<Exec Command="&quot;$(_ReportGeneratorPath)&quot; -reports:$(BaseOutputPath)$(PlatformConfig)\coverage\*.coverage -targetdir:$(BaseOutputPath)$(PlatformConfig)\coverage -reporttypes:Cobertura" />
<Message Importance="high" Text="&quot;$(_ReportGeneratorPath)&quot; -reports:$(BaseOutputPath)$(PlatformConfig)\coverage\*.coverage -targetdir:$(BaseOutputPath)$(PlatformConfig)\coverage -filefilters:+https* -reporttypes:Cobertura" />
<Exec Command="&quot;$(_ReportGeneratorPath)&quot; -reports:$(BaseOutputPath)$(PlatformConfig)\coverage\*.coverage -targetdir:$(BaseOutputPath)$(PlatformConfig)\coverage -filefilters:+https* -reporttypes:Cobertura" />
Copy link
Member Author

@sharwell sharwell Mar 10, 2019

Choose a reason for hiding this comment

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

📝 This is an attempt to address a change in the way numbers were appearing in the new reports. For some reason, the file submitted to codecov had two copies of each source file. The first copy referenced the file using the build machine path, and the second referenced the file using a Source Link path. Prior reports submitted to codecov appeared to only contain the latter, so I'm attempting to use a filter to address the problem.

<class name="Microsoft.ML.StaticPipe.LdaFitResult" filename="D:\a\1\s\src\Microsoft.ML.StaticPipe\LdaStaticExtensions.cs" line-rate="0" branch-rate="1" complexity="NaN">
  <methods>
    <method name=".ctor" signature="(Microsoft.ML.Transforms.Text.LatentDirichletAllocationTransformer/LdaSummary)" line-rate="0" branch-rate="1">
      <line number="24" hits="0" branch="false" />
      <line number="25" hits="0" branch="false" />
      <line number="26" hits="0" branch="false" />
      <line number="27" hits="0" branch="false" />
    </method>
  </methods>
  <lines>
    <line number="24" hits="0" branch="false" />
    <line number="25" hits="0" branch="false" />
    <line number="26" hits="0" branch="false" />
    <line number="27" hits="0" branch="false" />
  </lines>
</class>
<class name="Microsoft.ML.StaticPipe.LdaFitResult" filename="https://raw.githubusercontent.com/dotnet/machinelearning/d495bb4c7a58c459e753b9de3d0895c188468b58/src/Microsoft.ML.StaticPipe/LdaStaticExtensions.cs" line-rate="1" branch-rate="1" complexity="NaN">
  <methods>
    <method name=".ctor" signature="(Microsoft.ML.Transforms.Text.LatentDirichletAllocationTransformer/LdaSummary)" line-rate="1" branch-rate="1">
      <line number="24" hits="1" branch="false" />
      <line number="25" hits="1" branch="false" />
      <line number="26" hits="1" branch="false" />
      <line number="27" hits="1" branch="false" />
    </method>
  </methods>
  <lines>
    <line number="24" hits="1" branch="false" />
    <line number="25" hits="1" branch="false" />
    <line number="26" hits="1" branch="false" />
    <line number="27" hits="1" branch="false" />
  </lines>
</class>

@sharwell
Copy link
Member Author

sharwell commented Mar 10, 2019

This should be ready for review. The changes to coverage were primarily the following:

  1. Microsoft.ML.Benchmarks and Microsoft.ML.Benchmarks.Tests are no longer reported. These files had no coverage previously.
  2. Microsoft.ML.FSharp.Tests is no longer reported; the reportgenerator file filter might need to be updated to include it Should be fixed now

The other coverage changes appear to be normal non-determinism in the coverage runs.

@sharwell
Copy link
Member Author

For some reason the button to move this from a draft pull request to a normal pull request is missing here...

@TomFinley TomFinley marked this pull request as ready for review March 10, 2019 23:56
@TomFinley
Copy link
Contributor

For some reason the button to move this from a draft pull request to a normal pull request is missing here...

Interesting. Maybe it's one of those things that somehow needs write access? But that doesn't make sense. Anyway, I pushed the button...

build/ci/phase-template.yml Outdated Show resolved Hide resolved
Copy link
Member

@codemzs codemzs left a comment

Choose a reason for hiding this comment

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

:shipit:

@codemzs
Copy link
Member

codemzs commented Mar 12, 2019

Thanks, @sharwell !

@codemzs codemzs merged commit 6e1291a into dotnet:master Mar 12, 2019
@sharwell sharwell deleted the update-coverlet branch March 12, 2019 22:49
zsd4yr added a commit to dotnet/winforms that referenced this pull request Mar 15, 2019
* include coverlet.msbuild 2.5.1

* add targets and coverage group

* mock dotnet/machinelearning#2843

* Hook coverlet up to the 'RunTests' target

* Upload data to codecov.io

* Add code coverage badge to the README

* Add clarifying comments for code coverage configuration

* Move coverage upload after pick/sign/publish
@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants