Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 28, 2025

Unify ReflectionUtility and ReflectHelper to enable caching in deployment utilities

DeploymentUtility was using direct reflection without caching, while ReflectHelper provided cached attribute lookups via ConcurrentDictionary. The utilities couldn't use ReflectHelper due to namespace/access constraints.

Changes

  • Consolidated reflection logic: Moved ReflectionUtility implementation into ReflectionOperations service layer (handles reflection-only load scenarios on .NET Framework)
  • Updated DeploymentItemUtility: Now uses ReflectHelper.Instance instead of ReflectionUtility, enabling attribute caching
  • Removed ReflectionUtility.cs: Eliminated 282 lines of duplicate code
  • Fixed GetCustomAttributes: Use parameterless overload GetCustomAttributes(inherit: true) instead of GetCustomAttributes(typeof(Attribute), inherit: true) to match original behavior on non-NETFRAMEWORK

Before/After

// Before: No caching, separate utility class
internal DeploymentItemUtility(ReflectionUtility reflectionUtility)
{
    IReadOnlyList<object> attrs = _reflectionUtility.GetCustomAttributes(type, typeof(DeploymentItemAttribute));
}

// After: Uses ReflectHelper with ConcurrentDictionary caching
internal DeploymentItemUtility(ReflectHelper reflectHelper)
{
    IEnumerable<DeploymentItemAttribute> attrs = _reflectHelper.GetAttributes<DeploymentItemAttribute>(type);
}

Test updates reflect the API changes from GetCustomAttributes(memberInfo, typeof(T)) to GetAttributes<T>(memberInfo).

Fixes #3043

Original prompt

This section details on the original issue you should resolve

<issue_title>Unify ReflectionUtility and ReflectionHelper classes</issue_title>
<issue_description>DeploymentUtility in PlatformServices uses reflection to access attributes, but it does not use reflection helper because it does not have access to it. see if we can move the cached part of reflection helper closer to it, or the service close to the helper.</issue_description>

Comments on the Issue (you are @copilot in this section)

@Youssef1313 ReflectHelper is relying on ReflectionOptions which I think can't easily be moved to PlatformServices. This also mean that DeploymentUtility may end up not using source-generated reflection operation when it should. @Evangelink Now stale, I think it got fixed when I moved files in some past PR. @Youssef1313 Still relevant as we still have both:

https://github.com/microsoft/testfx/blob/main/src/Adapter/MSTestAdapter.PlatformServices/Utilities/ReflectionUtility.cs

https://github.com/microsoft/testfx/blob/main/src/Adapter/MSTestAdapter.PlatformServices/Helpers/ReflectHelper.cs

But now we can easily unify</comment_new>


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Unify ReflectionUtility and ReflectionHelper classes Unify ReflectionUtility and ReflectHelper to enable caching in deployment utilities Oct 28, 2025
Copilot AI requested a review from Evangelink October 28, 2025 14:59
Copilot finished work on behalf of Evangelink October 28, 2025 14:59
Copy link
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

@copilot some tests are broken, fix them:

Check failure on line 50 in test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.testfx (Build Windows Debug)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs#L50

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs(50,1): error : Verification failed for attributes.Length == 1 at line 50 of method 'GetCustomAttributesOnTypeShouldReturnAllAttributes' in file '/_/test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs'.
Check failure on line 63 in test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.testfx (Build Windows Debug)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs#L63

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs(63,1): error : Verification failed for attributes.Length == 2 at line 63 of method 'GetCustomAttributesOnTypeShouldReturnAllAttributesWithBaseInheritance' in file '/_/test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs'.
Check failure on line 39 in test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopReflectionOperationsTests.cs

@azure-pipelines
azure-pipelines
/ microsoft.testfx (Build Windows Debug)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopReflectionOperationsTests.cs#L39

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopReflectionOperationsTests.cs(39,1): error : Verification failed for attributes.Length == 1 at line 39 of method 'GetCustomAttributesOnTypeShouldReturnAllAttributes' in file '/_/test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopReflectionOperationsTests.cs'.
Check failure on line 165 in test/Utilities/TestFramework.ForTestingMSTest/TestContainer.cs

@azure-pipelines
azure-pipelines
/ microsoft.testfx (Build Windows Debug)
test/Utilities/TestFramework.ForTestingMSTest/TestContainer.cs#L165

test/Utilities/TestFramework.ForTestingMSTest/TestContainer.cs(165,1): error : Verification failed for attributes.Length == 1 at line 50 of method 'GetCustomAttributesOnTypeShouldReturnAllAttributes' in file '/_/test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs'.
Check failure on line 165 in test/Utilities/TestFramework.ForTestingMSTest/TestContainer.cs

@azure-pipelines
azure-pipelines
/ microsoft.testfx (Build Windows Debug)
test/Utilities/TestFramework.ForTestingMSTest/TestContainer.cs#L165

test/Utilities/TestFramework.ForTestingMSTest/TestContainer.cs(165,1): error : Verification failed for attributes.Length == 2 at line 63 of method 'GetCustomAttributesOnTypeShouldReturnAllAttributesWithBaseInheritance' in file '/_/test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs'.

Copy link
Contributor Author

Copilot AI commented Oct 29, 2025

@copilot some tests are broken, fix them:

[Check failure on line 50 in test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Utilities/ns13ReflectionUtilityTests.cs](https://github.com/microsoft/te...

Fixed in dd36600. The issue was that on non-NETFRAMEWORK, I was calling GetCustomAttributes(typeof(Attribute), inherit: true) instead of the parameterless GetCustomAttributes(inherit: true). Changed to match the original behavior.

Copilot finished work on behalf of Evangelink October 29, 2025 09:23
Copilot AI requested a review from Evangelink October 29, 2025 09:23
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.

Unify ReflectionUtility and ReflectionHelper classes

2 participants