Adds Test Execution Caching #26
Conversation
|
|
||
| if (!nodeContext.TargetNames.SetEquals(buildRequest.TargetNames)) | ||
| bool isBuildAndTestRequest = IsBuildAndTestRequest(buildRequest); | ||
| if (!nodeContext.TargetNames.SetEquals(buildRequest.TargetNames) && !isBuildAndTestRequest) |
There was a problem hiding this comment.
If the target names don't align, this seems like a correctness issue
There was a problem hiding this comment.
The targets don't align, MSBuildCache seems to ignore the Test target. That is, it is not included in nodeContext.TargetNames but it is in buildRequest.TargetNames
There was a problem hiding this comment.
reference: dotnet/msbuild#9569
src/Common/MSBuildCachePluginBase.cs
Outdated
| await FinishNodeAsync(logger, nodeContext, pathSet, nodeBuildResult); | ||
| // In the case of a build-and-test request, we want to cache the build results but and the test execution (i.e, only execute tests on cache misses). | ||
| _ = bool.TryParse(projectInstance.GetPropertyValue("IsTestProject"), out bool isTestProject); | ||
| if (isBuildAndTestRequest && isTestProject) |
There was a problem hiding this comment.
if (isBuildAndTestRequest
&& bool.TryParse(projectInstance.GetPropertyValue("IsTestProject"), out bool isTestProject)
&& isTestProject)
| if (isBuildAndTestRequest && isTestProject) | ||
| { | ||
| // this property can be read by downstream tasks, such as a Test Target, to determine if the test execution should be skipped | ||
| _ = projectInstance.SetProperty("SkipExecution", "true"); |
There was a problem hiding this comment.
This seems dangerous for the cache plugin to mutate the project instance. I'm actually not even sure it even works in practice considering this is happening in the scheduler node and not the worker node.
There was a problem hiding this comment.
Also, this is a property of the runvstest task/sdk not the test project
src/Common/MSBuildCachePluginBase.cs
Outdated
| /// <returns>A bool value representing whether the build request is a build and test request.</returns> | ||
| private static bool IsBuildAndTestRequest(BuildRequestData buildRequest) | ||
| { | ||
| if (buildRequest == null) |
src/Common/MSBuildCachePluginBase.cs
Outdated
| return false; | ||
| } | ||
| // Check if the build request includes both "Build" and "Test" targets | ||
| bool isBuildAndTestRequest = buildRequest.TargetNames.Contains("Build") && buildRequest.TargetNames.Contains("Test"); |
|
I don't think I understand why the Test target is called out specifically, especially since the Build target isn't called out specifically today. |
2 participants
Adds test execution caching when used with
Microsoft.Build.RunVSTestmicrosoft/MSBuildSdks#473.If a test project is a cache hit, test execution will be skipped and a message indicating that the test was skipped is logged.
Usage:
MSBuild /restore /graph /m /nr:false /reportfileaccesses /t:"Build;Test" /p:UseMSBuildTestInfrastructure="true"Sample output
