Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce unnecessary dotnet restore work during acceptance/integration test asset preparation, primarily by skipping restores when inputs appear unchanged and adding more timing/diagnostic logging.
Changes:
- Added additional stopwatch timing logs around test asset builds.
- Skipped restoring TestPlatform/TestPlatform.CLI when the corresponding NuGet cache version directories already exist.
- Removed the restore step when the compatibility-matrix cache id matches, and added logs for “up to date / generating” states.
| { | ||
| // Project cache is up-to-date, just rebuilding solution. | ||
| ExecuteApplication2(Dotnet, $"""restore --packages {nugetCache} {nugetFeeds} --source "{IntegrationTestEnvironment.LocalPackageSource}" "{generatedSln}" """); | ||
| ExecuteApplication2(Dotnet, $"build {generatedSln} --no-restore --configuration {IntegrationTestEnvironment.BuildConfiguration} -v:minimal"); |
There was a problem hiding this comment.
Removing the dotnet restore here can break builds in cases where the cache-id matches but restore outputs are missing (e.g., cleared/expired NuGet cache, deleted obj/project.assets.json, or a partially-populated cache). Consider keeping a restore step, or gating --no-restore on the presence of required restore artifacts (e.g., solution/project assets files) before skipping restore.
| ExecuteApplication2(Dotnet, $"build {generatedSln} --no-restore --configuration {IntegrationTestEnvironment.BuildConfiguration} -v:minimal"); | |
| ExecuteApplication2(Dotnet, $"build {generatedSln} --configuration {IntegrationTestEnvironment.BuildConfiguration} -v:minimal"); |
| var netTestSdkVersionDir = netTestSdkVersion.TrimStart('[').TrimEnd(']'); | ||
| if (Directory.Exists(Path.Combine(nugetCache, "microsoft.testplatform", netTestSdkVersionDir)) && Directory.Exists(Path.Combine(nugetCache, "microsoft.testplatform.cli", netTestSdkVersionDir))) | ||
| { | ||
| Debug.WriteLine($"Version {netTestSdkVersion} of TestPlatform and TestPlatform.CLI is already in the cache, skipping restore."); |
There was a problem hiding this comment.
Grammatically, this refers to two packages, so it should use plural agreement (e.g., 'are already in the cache').
| Debug.WriteLine($"Version {netTestSdkVersion} of TestPlatform and TestPlatform.CLI is already in the cache, skipping restore."); | |
| Debug.WriteLine($"Version {netTestSdkVersion} of TestPlatform and TestPlatform.CLI are already in the cache, skipping restore."); |
Description
Please add a meaningful description for this change.
Ensure the PR has required unit tests.
Related issue
Kindly link any related issues. E.g. Fixes #xyz.