Skip to content

Conversation

jkoritzinsky
Copy link
Member

Fixes #120722

@Copilot Copilot AI review requested due to automatic review settings October 15, 2025 06:01
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 15, 2025
Copy link
Contributor

@Copilot 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

This PR modernizes the System.Diagnostics.Debug.Tests by replacing reflection-based access to internal Debug and DebugProvider members with the more efficient and type-safe UnsafeAccessor pattern. This addresses issue #120722 and follows modern .NET best practices for accessing private/internal members in tests.

Key changes:

  • Replaces reflection calls with UnsafeAccessor methods for accessing internal Debug provider functionality
  • Adds a new public GetProvider() method to Debug class to support testing needs
  • Updates compatibility suppressions to handle the new public API

Reviewed Changes

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

File Description
src/libraries/System.Runtime/tests/System.Diagnostics.Debug.Tests/DebugTests.cs Replaces reflection-based field access with UnsafeAccessor methods for cleaner, more efficient test code
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Debug.cs Adds public GetProvider() method to expose the internal provider for testing
src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt Registers the new GetProvider method in the extra APIs list
src/coreclr/System.Private.CoreLib/CompatibilitySuppressions.xml Adds compatibility suppression for the new GetProvider method

@jkoritzinsky jkoritzinsky requested a review from sbomer October 15, 2025 17:19
@jkoritzinsky jkoritzinsky added area-System.Diagnostics and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 15, 2025
Copy link
Contributor

Tagging subscribers to this area: @steveisok, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

@jkotas
Copy link
Member

jkotas commented Oct 15, 2025

Cleanup this as well

<!-- Some tests need types like System.Diagnostics.DebugProvider which are only exposed from System.Private.CoreLib -->
<CompileUsingReferenceAssemblies>false</CompileUsingReferenceAssemblies>
<!-- Active issue: https://github.com/dotnet/runtime/issues/87740 -->
<ShouldILStrip>false</ShouldILStrip>
</PropertyGroup>
<ItemGroup>
<DefaultReferenceExclusion Include="System.Collections" />
<ProjectReference Include="$(CoreLibProject)" />
?

@jkoritzinsky jkoritzinsky enabled auto-merge (squash) October 16, 2025 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[NAOT][android] System.Diagnostics.Debug.Tests fails to build

3 participants