MTP: Allow test results to have separate display name while keeping the original info for TestNode.DisplayName#7590
MTP: Allow test results to have separate display name while keeping the original info for TestNode.DisplayName#7590Youssef1313 wants to merge 2 commits intomainfrom
Conversation
…he original info for TestNode.DisplayName
There was a problem hiding this comment.
Pull request overview
This PR updates Microsoft.Testing.Platform (MTP) messaging so a TestNodeUpdateMessage can carry a custom display name (e.g., from a TestResult) without overwriting the underlying TestNode.DisplayName produced from the original TestCase.
Changes:
- Added a new
TestNodeUpdateMessagepublic constructor overload that accepts an explicitdisplayName. - Updated VSTest bridge conversion to stop using
TestResult.DisplayNameto populateTestNode.DisplayName. - Updated VSTest bridge result publishing to pass
TestResult.DisplayNameintoTestNodeUpdateMessagewhen available.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Platform/Microsoft.Testing.Platform/PublicAPI/PublicAPI.Unshipped.txt | Declares the newly added public constructor overload. |
| src/Platform/Microsoft.Testing.Platform/Messages/TestNodeUpdateMessage.cs | Introduces the new constructor overload and delegates the existing ctor to it. |
| src/Platform/Microsoft.Testing.Extensions.VSTestBridge/ObjectModel/ObjectModelConverters.cs | Keeps TestNode.DisplayName sourced from TestCase rather than TestResult. |
| src/Platform/Microsoft.Testing.Extensions.VSTestBridge/ObjectModel/FrameworkHandlerAdapter.cs | Publishes TestNodeUpdateMessage with TestResult.DisplayName when recording results. |
| /// <param name="testNode">The test node.</param> | ||
| /// <param name="displayName">The display name for the test update.</param> |
There was a problem hiding this comment.
The XML doc tags for the primary constructor are out of order vs the actual parameter list (constructor is sessionUid, displayName, testNode, parentTestNodeUid, but docs list testNode before displayName). This can confuse generated docs/intellisense; please reorder the tags to match the signature and keep the parameter descriptions aligned with the correct parameters.
| /// <param name="testNode">The test node.</param> | |
| /// <param name="displayName">The display name for the test update.</param> | |
| /// <param name="displayName">The display name for the test update.</param> | |
| /// <param name="testNode">The test node.</param> |
| /// <summary> | ||
| /// Converts a VSTest <see cref="TestResult"/> to a Microsoft Testing Platform <see cref="TestNode"/>. | ||
| /// </summary> | ||
| public static TestNode ToTestNode( | ||
| this TestResult testResult, | ||
| bool isTrxEnabled, | ||
| bool useFullyQualifiedNameAsUid, | ||
| Action<TestNode, TestCase> addAdditionalProperties, | ||
| INamedFeatureCapability? namedFeatureCapability, | ||
| ICommandLineOptions commandLineOptions, | ||
| IClientInfo clientInfo) | ||
| { | ||
| var testNode = testResult.TestCase.ToTestNode(isTrxEnabled, useFullyQualifiedNameAsUid, addAdditionalProperties, namedFeatureCapability, commandLineOptions, clientInfo, testResult.DisplayName); | ||
| var testNode = testResult.TestCase.ToTestNode(isTrxEnabled, useFullyQualifiedNameAsUid, addAdditionalProperties, namedFeatureCapability, commandLineOptions, clientInfo); | ||
|
|
||
| CopyCategoryAndTraits(testResult, testNode, isTrxEnabled); |
There was a problem hiding this comment.
TestResult.DisplayName is no longer used to populate TestNode.DisplayName (now always using TestCase.DisplayName/FQN). There are existing unit tests for ObjectModelConverters, but none that assert this new behavior; please add/adjust a test to cover the scenario where TestResult.DisplayName differs from TestCase.DisplayName to prevent regressions.
|
After discussion with @Evangelink, we will address it differently and changes will be specific to TRX. I'll prepare another PR. |
TODOs: