-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[WIP] Reduce test target complexity [and running time] #5403
[WIP] Reduce test target complexity [and running time] #5403
Conversation
88c3d44
to
b56c0e0
Compare
Remove deprecated tests Make Microsoft.DotNet.Tools.Tests.Utilities portable-only Remove MSI tests from the solution as they are the only tests that currently require dekstop.
7ef8cb8
to
4f466fb
Compare
4f466fb
to
0facfa2
Compare
@@ -346,7 +346,7 @@ public IEnumerable<FileInfo> LoadInventory(FileInfo file) | |||
file.Refresh(); | |||
if (!file.Exists) | |||
{ | |||
throw new ArgumentException("Inventory file should exist."); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
49c27a2
to
d6208d3
Compare
@MattGertz FYI, this infrastructure PR eliminates a bunch of the custom incremental build logic that CLI depended on while migrating CLI's build from PJ to CSProj. This greatly reduces CLI build complexity and, more importantly, build times. The PR also removes a deprecated and long-running test library [performance validation for project.json system which, after recent re-evaluation, adds little value for MSBuild/SDK scenarios. These will need to be replaced with propper performance tests for csproj in the near future]. The combined benefit was observed in this PR as a reduction from ~33 minute builds to ~18 minute builds. Wohoo! |
:-) |
A lot of red in this PR, which is goodness in my book! |
I was inspired by @blackdwarf's work to reduce the many targets of 'test restore' into a single sln and single restore invocation. Greatly reduced concept count and a slight perf improvement.
This change eliminates a bunch of duplicate test orchestration code, resulting in modest perf gains and lots of simplification. Included:
dotnet test
to prove that the test library is up-to-datedotnet build
. This was necessary when we were wrapping project.json but is no longer needed.The PR is a WiP. I expect issues due to
/
vs\
. I'm also not able to test locally due to pending reboot, so will see what CI has to say./cc @livarcocc