testfx icon indicating copy to clipboard operation
testfx copied to clipboard

Unify ReflectionUtility and ReflectHelper to enable caching in deployment utilities

Open Copilot opened this issue 4 months ago • 1 comments

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 microsoft/testfx#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>

  • Fixes microsoft/testfx#3043

✨ 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 avatar Oct 28 '25 14:10 Copilot

@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 avatar Oct 29 '25 09:10 Copilot