Skip to content

Cleanup dead TRX code#15474

Merged
nohwnd merged 3 commits intomainfrom
dev/ygerges/trx-cleanup
Mar 11, 2026
Merged

Cleanup dead TRX code#15474
nohwnd merged 3 commits intomainfrom
dev/ygerges/trx-cleanup

Conversation

@Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Mar 10, 2026

No description provided.

Copilot AI review requested due to automatic review settings March 10, 2026 18:58

XmlPersistence helper = new();
helper.SaveSimpleField(element, "@agentName", _agentName, null);
helper.SaveSimpleField(element, "@agentDisplayName", _agentDisplayName, _agentName);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What this line was saying is to set agentDisplayName attribute value to _agentDisplayName if and only if it's different from _agentName.

However, _agentDisplayName and _agentName were always the same. So, this line was always not doing anything. So simplified this class also by removing _agentDisplayName completely.

XmlPersistence helper = new();
helper.SaveSimpleField(element, "@agentName", _agentName, null);
helper.SaveSimpleField(element, "@agentDisplayName", _agentDisplayName, _agentName);
helper.SaveSimpleField(element, "@isFromRemoteAgent", _isFromRemoteAgent, false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line would only set isFromRemoteAgent attribute if and only if it's not false. However, it was always false. So removed completely to simplify.

/// <param name="resultsDirectory">The results directory to use to make paths in the data attachments relative or absolute</param>
/// <param name="useAbsolutePaths">True to use absolute paths in this instance, false to use relative paths</param>
/// <returns>A clone of the instance containing cloned attachments with file paths made absolute or relative</returns>
internal CollectorDataEntry Clone(string resultsDirectory, bool useAbsolutePaths)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was always called with useAbsolutePaths: false.
So, removing the bool parameter and renaming to CloneWithRelativePath.

/// <param name="useAbsoluteUri">True to use an absolute URI in the clone, false to use a relative URI</param>
/// <returns>A clone of the instance, with the URI made absolute</returns>
internal UriDataAttachment Clone(string baseDirectory, bool useAbsoluteUri)
internal UriDataAttachment CloneWithRelativePath(string baseDirectory)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was always called with useAbsoluteUri: false. So, removed the parameter and renamed to CloneWithRelativePath.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes unused/duplicate TRX logger fields and simplifies cloning/serialization logic for collector data and URI attachments.

Changes:

  • Simplifies CollectorDataEntry by removing agent display name / remote-agent metadata and updating construction/callers.
  • Replaces attachment cloning APIs to always produce relative paths (CloneWithRelativePath).
  • Removes duplicate constructor argument usage in Converter.ToCollectorEntry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/Microsoft.TestPlatform.Extensions.TrxLogger/Utility/Converter.cs Updates CollectorDataEntry creation to match the simplified constructor signature.
src/Microsoft.TestPlatform.Extensions.TrxLogger/ObjectModel/UriDataAttachment.cs Renames/simplifies cloning to produce relative URIs and removes unused System.IO usage.
src/Microsoft.TestPlatform.Extensions.TrxLogger/ObjectModel/TestResult.cs Updates collector entry cloning call to the new relative-path clone API.
src/Microsoft.TestPlatform.Extensions.TrxLogger/ObjectModel/CollectorDataEntry.cs Removes agent display/remote metadata, updates serialization and clone API accordingly.
Comments suppressed due to low confidence (1)

src/Microsoft.TestPlatform.Extensions.TrxLogger/ObjectModel/CollectorDataEntry.cs:1

  • This change stops emitting @agentDisplayName and @isFromRemoteAgent in the serialized TRX. That is an observable output-format change that may break consumers expecting those attributes to exist (even if defaulted). If the intent is purely internal cleanup, consider continuing to write these attributes with default values (e.g., display name = agent name, remote = false) or documenting this as a deliberate TRX schema/output breaking change.
// Copyright (c) Microsoft Corporation. All rights reserved.

@nohwnd nohwnd merged commit d63354d into main Mar 11, 2026
6 checks passed
@nohwnd nohwnd deleted the dev/ygerges/trx-cleanup branch March 11, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants