-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Split out non concurrent test collections. #6937
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6937 +/- ##
========================================
Coverage 68.80% 68.81%
========================================
Files 1249 1249
Lines 249436 249644 +208
Branches 25468 25481 +13
========================================
+ Hits 171635 171783 +148
- Misses 71211 71271 +60
Partials 6590 6590
Flags with carried forward coverage won't be shown. Click here to find out more.
|
test/Microsoft.ML.TorchSharp.Tests/Microsoft.ML.TorchSharp.Tests.csproj
Outdated
Show resolved
Hide resolved
<ProjectReference Include="..\..\src\Microsoft.Data.Analysis\Microsoft.Data.Analysis.csproj" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup Condition="'$(TargetArchitecture)' != 'x64'"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider trying out @ViktorHofer's solution for excluding the entire project from the SLN traversal. dotnet/xdt@main...DoCoolFiltering
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I would do that in another PR so that we can start getting the benefits now of the faster tests/no timeouts. And I have excluded this assembly from helix testing as well for non x64, so it won't affect those tests at all either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not fully understand @ericstj comment. This looks excluding some source file when running on x64. How excluding the entire project work here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this was just a consider. I'm completely fine excluding things as you did, since it looks like we already do that for a lot of other projects. If you feel strongly about not building this then file an issue on that.
@tarekgh could you take a quick look at this when you have a minute? It just splits out some test assemblies so we can run more in parallel. |
test/Microsoft.ML.TorchSharp.Tests/Microsoft.ML.TorchSharp.Tests.csproj
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a couple of questions. LGTM!
91bb0fb
to
a8dfb40
Compare
/backport to release/3.0 |
Started backporting to release/3.0: https://github.com/dotnet/machinelearning/actions/runs/7455070236 |
/backport to release/3.0 |
Started backporting to release/3.0: https://github.com/dotnet/machinelearning/actions/runs/7464450572 |
Split out non-concurrent test collections to improve performance.