Skip to content

Align System.Reflection.Metadata CustomAttributeDecoder tests with embedded-resource scheme#127224

Closed
Copilot wants to merge 4 commits intomainfrom
copilot/align-customattributedecoder-tests
Closed

Align System.Reflection.Metadata CustomAttributeDecoder tests with embedded-resource scheme#127224
Copilot wants to merge 4 commits intomainfrom
copilot/align-customattributedecoder-tests

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 21, 2026

CustomAttributeDecoderTests was introspecting the running test assembly instead of using the embedded-resource pattern used by the rest of System.Reflection.Metadata.Tests. This made the tests incompatible with single-file, trimming, and NativeAOT, and sensitive to BCL assembly-identity differences (e.g. System.Runtime vs System.Private.CoreLib), requiring ConditionalFact(NotSingleFile) guards and assembly-identity workarounds.

Changes

  • New helper assembly (Resources/Decoding/CustomAttributeDecoder.cs + .dll): self-contained source + pre-compiled binary containing TestAttribute, all enum types, HasAttributes, GenericAttribute<T>, GenericAttribute2<K,V>, HasGenericAttributes, and HasGenericArrayAttributes
  • TestResources.cs: added internal static class Decoding exposing CustomAttributeDecoder via the existing ResourceHelper pattern
  • System.Reflection.Metadata.Tests.csproj: registered the new .cs (as None) and .dll (as EmbeddedResource)
  • CustomAttributeDecoderTests.cs: minimal changes to switch from disk-based loading to embedded-resource loading:
    • Replaced FileStream + AssemblyPathHelper.GetAssemblyLocation with MemoryStream over the embedded DLL bytes
    • Dropped ConditionalFact(HasAssemblyFiles) / ActiveIssue guards — tests are now plain [Fact]
    • Updated type lookup from TestMetadataResolver.FindTestType(reader, typeof(HasAttributes)) to a FindTestType helper that searches by namespace and name in the embedded metadata
    • Removed TestCustomAttributeDecoderUsingReflection (relied on type.GetCustomAttributesData() which requires the types to be in the running assembly)
    • Updated CustomAttributeTypeProvider to use hardcoded "[System.Runtime]System.Type" and a switch on type names instead of MetadataReaderTestHelpers.RuntimeAssemblyName / Type.GetType()
    • TestCustomAttributeDecoder retains its original 3-case structure with default: break
    • Generic attribute tests remain gated on #if NET && !TARGET_BROWSER

Note

This PR description was generated with AI assistance (GitHub Copilot).

Copilot AI self-assigned this Apr 21, 2026
Copilot AI review requested due to automatic review settings April 21, 2026 13:38
Copilot AI review requested due to automatic review settings April 21, 2026 13:38
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

- Add Resources/Decoding/CustomAttributeDecoder.cs: source for helper assembly with all types and custom attributes needed by the decoder tests
- Add Resources/Decoding/CustomAttributeDecoder.dll: pre-compiled helper assembly embedded as a test resource
- Update TestResources.cs to expose the new resource via Decoding.CustomAttributeDecoder
- Update System.Reflection.Metadata.Tests.csproj to include the new resource files
- Rewrite CustomAttributeDecoderTests.cs to load metadata from the embedded resource instead of introspecting the running test assembly:
  - Remove ConditionalFact(NotSingleFile) / ConditionalFact(HasAssemblyFiles) guards
  - Remove assembly-identity workarounds (RuntimeAssemblyName, TypeToString, etc.)
  - Remove inline type definitions (HasAttributes, TestAttribute, enums, GenericAttribute, etc.)
  - Add comprehensive assertions covering all 82 attributes on HasAttributes, 8 on HasGenericAttributes, and 2 on HasGenericArrayAttributes
  - Add FindTestType helper that looks up types by namespace+name in the embedded metadata
  - Simplify CustomAttributeTypeProvider with hardcoded type mappings for the resource assembly's enum types

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/8f883665-1cac-472f-81aa-5074b5ff4f1c

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot April 21, 2026 14:27
Copilot AI requested review from Copilot and removed request for Copilot April 21, 2026 14:30
break;

case 2:
case 2: // [Test(StringField = "0", Int32Field = 1, SByteEnumArrayProperty = new[] { SByteEnum.Value })]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot Revert all unnecessary test updates and refactorings. Keep the changes limited to the objective of this PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in ca6c958. Reverted TestCustomAttributeDecoder back to the original 3-case structure with default: break, removing the 79 extra cases that were unnecessarily added. The remaining changes are the minimum needed to switch from FileStream + AssemblyPathHelper.GetAssemblyLocation to the embedded-resource scheme: loading via MemoryStream, updated type lookup, and a CustomAttributeTypeProvider that uses hardcoded type names instead of Type.GetType() / MetadataReaderTestHelpers.RuntimeAssemblyName.

Copilot AI changed the title [WIP] Align CustomAttributeDecoder tests with resource-embedded assembly scheme Align System.Reflection.Metadata CustomAttributeDecoder tests with embedded-resource scheme Apr 21, 2026
Copilot AI requested a review from jkotas April 21, 2026 14:32
- Restore TestCustomAttributeDecoder to original 3-case structure with default: break
- Remove cases 3-81 that were added unnecessarily
- Keep all other necessary changes: loading via embedded resource, FindTestType helper, updated CustomAttributeTypeProvider, TestCustomAttributeDecoderGeneric, TestCustomAttributeDecoderGenericArray

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/139c63c4-491a-4ab0-81cc-432ef0e7b479

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot April 21, 2026 14:54
@jkotas
Copy link
Copy Markdown
Member

jkotas commented Apr 21, 2026

This does not look like an improvement.

@jkotas jkotas closed this Apr 21, 2026
@jkotas jkotas deleted the copilot/align-customattributedecoder-tests branch April 21, 2026 16:23
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.

Align System.Reflection.Metadata CustomAttributeDecoder tests with resource-embedded assembly scheme

2 participants