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

WIP Code Coverage #2180

Closed
wants to merge 10 commits into from
Closed

WIP Code Coverage #2180

wants to merge 10 commits into from

Conversation

codemzs
Copy link
Member

@codemzs codemzs commented Jan 17, 2019

No description provided.

@codemzs codemzs closed this Jan 17, 2019
@codemzs codemzs reopened this Jan 17, 2019
@codemzs codemzs closed this Jan 17, 2019
@codemzs codemzs reopened this Jan 17, 2019
@TomFinley
Copy link
Contributor

TomFinley commented Jan 18, 2019

Thanks for working on this @codemzs . My understanding from @sharwell is that we can get the coverage test build from taking 10m during the test phase down to maybe 15m. But this depends upon future releases of Coverlet. (@sharwell correct me if I say anything wrong.) If we can get it down that far, might be worth taking Sam's advice elsewhere and putting it in main build.

I don't have full context on what is going on behind the scenes.

@glebuk and @shauheen (or indeed anyone), thoughts?

@sharwell
Copy link
Member

sharwell commented Jan 18, 2019

There are two main ways to get the speed improved:

  1. Threshold coverage to a single hit (Single hit coverlet-coverage/coverlet#309)
  2. Targeted coverage exclusions using <Exclude>, starting with the following. These will be excluded for all test assemblies except the test assembly specifically intended to test the behavior of the type.
    • Microsoft.ML.Contracts
    • Microsoft.ML.Internal.Utilities
    • Microsoft.ML.Data.VBuffer<T>

@codemzs You could start by excluding the above three classes using <Exclude> in the Directory.Build.props files for all tests to observe the overall improvement, and then we can make that conditional once we confirm the improvement.

@codemzs
Copy link
Member Author

codemzs commented Jan 18, 2019

@sharwell After excluding the classes you mentioned the build time with CC for Windows x64 debug went down to ~32 minutes from ~47 minutes. The build time without CC for the same platform is ~20 minutes. Link to build logs: https://dev.azure.com/dnceng/public/_build/results?buildId=75554

@sharwell
Copy link
Member

@codemzs Those numbers should be good enough to hold us over until coverlet is updated 👍

@codemzs
Copy link
Member Author

codemzs commented Jan 18, 2019

@sharwell Just to confirm, are you saying we should update the windows x64 debug leg of main CI build with the current coverlet nuget? If yes, I hope @TomFinley is cool too! :)

@sharwell
Copy link
Member

@codemzs It could go in either leg. I'm saying this performance is enough that we can stop looking for <Exclude> items for now, and instead focus on getting the results uploaded to codecov.io.

@codemzs
Copy link
Member Author

codemzs commented Jan 18, 2019

@sharwell Either leg as in? We are sticking with just one CI right?

@sharwell
Copy link
Member

sharwell commented Jan 18, 2019

Meaning it can go in MachineLearning-CodeCoverage or in MachineLearning-CI. If it goes in MachineLearning-CI, we just remove MachineLearning-CodeCoverage.

@codemzs codemzs closed this Jan 20, 2019
@shauheen shauheen deleted the ccbuilddef2 branch January 23, 2019 22:48
@ghost ghost locked as resolved and limited conversation to collaborators Mar 25, 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