Fix MVC Core tests for CoreCLR GetCustomAttributes consistency #62872

medhatiwari · 2025-07-23T05:10:08Z

Fix MVC Core tests for CoreCLR GetCustomAttributes consistency

You've read the Contributor Guide and Code of Conduct.
You've included unit or integration tests for your change, where applicable.
You've included inline docs for your change, where applicable.
There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Fix MVC Core tests for CoreCLR GetCustomAttributes consistency

Description

This PR updates two test methods in the MVC Core test suite that were expecting CoreCLR's old inconsistent behavior for typeof(object).GetCustomAttributes().

Background:
CoreCLR had a bug where typeof(object).GetCustomAttributes(inherit: true) and typeof(object).GetCustomAttributes(inherit: false) returned different numbers of attributes (1 vs 5). This inconsistency caused ASP.NET Core tests to fail when running on Mono, which correctly returned 5 attributes for both cases.

Changes:
Updates two test methods that were written expecting the old buggy behavior:

ModelAttributesTest.GetAttributesForParameter_NoAttributes
DefaultModelMetadataProviderTest.GetMetadataForParameter_SuppliesEmptyAttributes_WhenParameterHasNoAttributes

Both tests now correctly expect 5 attributes (SerializableAttribute, NullableContextAttribute, ClassInterfaceAttribute, ComVisibleAttribute, TypeForwardedFromAttribute) instead of just 1.

Impact:

Ensures ASP.NET Core tests pass with the fixed CoreCLR runtime

Related to dotnet/runtime#117864

Copilot

Pull Request Overview

This PR fixes two test methods in the MVC Core test suite to align with CoreCLR's corrected behavior for typeof(object).GetCustomAttributes(). Previously, CoreCLR had inconsistent behavior where GetCustomAttributes(inherit: true) and GetCustomAttributes(inherit: false) returned different numbers of attributes for the object type. The fix ensures both calls consistently return all 5 pseudo-attributes.

Updates test assertions to expect 5 attributes instead of 1
Adds explicit checks for all pseudo-attributes on the object type
Ensures ASP.NET Core tests pass with the fixed CoreCLR runtime

Reviewed Changes

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

File	Description
ModelAttributesTest.cs	Updates `GetAttributesForParameter_NoAttributes` test to expect all 5 pseudo-attributes
DefaultModelMetadataProviderTest.cs	Updates `GetMetadataForParameter_SuppliesEmptyAttributes_WhenParameterHasNoAttributes` test to expect all 5 pseudo-attributes

Copilot · 2025-07-23T05:11:44Z

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ModelAttributesTest.cs

+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");


Using string comparison with GetType().Name for attribute type checking is fragile and could break if the attribute type is renamed or moved to a different namespace. Consider using typeof() comparison or checking if the attribute is assignable from the expected type.

Suggested change

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType() == Type.GetType("System.Runtime.CompilerServices.NullableContextAttribute"));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(System.Runtime.InteropServices.ClassInterfaceAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(System.Runtime.InteropServices.ComVisibleAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(System.Runtime.Serialization.TypeForwardedFromAttribute));

Copilot · 2025-07-23T05:11:44Z

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ModelAttributesTest.cs

+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");


Using string comparison with GetType().Name for attribute type checking is fragile and could break if the attribute type is renamed or moved to a different namespace. Consider using typeof() comparison or checking if the attribute is assignable from the expected type.

Suggested change

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(NullableContextAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(ClassInterfaceAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(ComVisibleAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(TypeForwardedFromAttribute));

Copilot · 2025-07-23T05:11:44Z

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ModelAttributesTest.cs

+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");


Using string comparison with GetType().Name for attribute type checking is fragile and could break if the attribute type is renamed or moved to a different namespace. Consider using typeof() comparison or checking if the attribute is assignable from the expected type.

Suggested change

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(NullableContextAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(ClassInterfaceAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(ComVisibleAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(TypeForwardedFromAttribute));

Copilot · 2025-07-23T05:11:45Z

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ModelAttributesTest.cs

+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");
+        Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");


Using string comparison with GetType().Name for attribute type checking is fragile and could break if the attribute type is renamed or moved to a different namespace. Consider using typeof() comparison or checking if the attribute is assignable from the expected type.

Suggested change

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");

Assert.Contains(attributes.Attributes, attr => attr.GetType() == Type.GetType("System.Runtime.CompilerServices.NullableContextAttribute"));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(System.Runtime.InteropServices.ClassInterfaceAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(System.Runtime.InteropServices.ComVisibleAttribute));

Assert.Contains(attributes.Attributes, attr => attr.GetType() == typeof(System.Runtime.CompilerServices.TypeForwardedFromAttribute));

Copilot · 2025-07-23T05:11:45Z

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs

+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");


Using string comparison with GetType().Name for attribute type checking is fragile and could break if the attribute type is renamed or moved to a different namespace. Consider using typeof() comparison or checking if the attribute is assignable from the expected type.

Suggested change

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is NullableContextAttribute);

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is ClassInterfaceAttribute);

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is ComVisibleAttribute);

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is TypeForwardedFromAttribute);

Copilot · 2025-07-23T05:11:45Z

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs

+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");


Using string comparison with GetType().Name for attribute type checking is fragile and could break if the attribute type is renamed or moved to a different namespace. Consider using typeof() comparison or checking if the attribute is assignable from the expected type.

Suggested change

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType() == typeof(NullableContextAttribute));

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType() == typeof(ClassInterfaceAttribute));

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType() == typeof(ComVisibleAttribute));

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType() == typeof(TypeForwardedFromAttribute));

Copilot · 2025-07-23T05:11:45Z

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs

+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");


Using string comparison with GetType().Name for attribute type checking is fragile and could break if the attribute type is renamed or moved to a different namespace. Consider using typeof() comparison or checking if the attribute is assignable from the expected type.

Suggested change

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is NullableContextAttribute);

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is ClassInterfaceAttribute);

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is ComVisibleAttribute);

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is TypeForwardedFromAttribute);

Copilot · 2025-07-23T05:11:46Z

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs

+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");
+        Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");


Using string comparison with GetType().Name for attribute type checking is fragile and could break if the attribute type is renamed or moved to a different namespace. Consider using typeof() comparison or checking if the attribute is assignable from the expected type.

Suggested change

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "NullableContextAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ClassInterfaceAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "ComVisibleAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr.GetType().Name == "TypeForwardedFromAttribute");

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is System.Runtime.CompilerServices.NullableContextAttribute);

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is System.Runtime.InteropServices.ClassInterfaceAttribute);

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is System.Runtime.InteropServices.ComVisibleAttribute);

Assert.Contains(defaultMetadata.Attributes.Attributes, attr => attr is System.Runtime.CompilerServices.TypeForwardedFromAttribute);

jkotas · 2025-07-23T05:46:18Z

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs

-        // Not exactly "no attributes" due to SerializableAttribute on object.
-        Assert.IsType<SerializableAttribute>(Assert.Single(defaultMetadata.Attributes.Attributes));
+        // Not exactly "no attributes" due to pseudo-attributes on object.
+        // After CoreCLR consistency fix, object.GetCustomAttributes() returns all pseudo-attributes.


Should this test be deleted instead? It does not make sense for ASP.NET to test internal implementation details of CoreCLR.

but looking at the name of the testcase
GetMetadataForParameter_SuppliesEmptyAttributes_WhenParameterHasNoAttributes and GetAttributesForParameter_NoAttributes the intent seems to be testing that a parameter with "no attributes" behaves correctly, so in this situation we either could just remove this testscase or Change the test to just verify that ParameterAttributes is empty (thats what I feel is the actual ASP.NET Core concern)

Either way sounds good to me.

…assertions Signed-off-by: Medha Tiwari <medhavns1@gmail.com>

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs

…taProviderTest.cs

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ModelAttributesTest.cs

…t.cs

jkotas

LGTM

@Minimal-APIs PTLA

captainsafia

LGTM -- thanks!

Fixing ASP .NET test as discussed here dotnet/runtime#117864

2752b68

Copilot AI review requested due to automatic review settings July 23, 2025 05:10

medhatiwari requested a review from a team as a code owner July 23, 2025 05:10

github-actions bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jul 23, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 23, 2025

Copilot AI reviewed Jul 23, 2025

View reviewed changes

medhatiwari mentioned this pull request Jul 23, 2025

Fix System.Object.GetCustomAttributes() returning extra internal attributes on Mono dotnet/runtime#117864

Merged

4 tasks

jkotas reviewed Jul 23, 2025

View reviewed changes

build-analysis bot mentioned this pull request Jul 23, 2025

Known Build Error: error : Cannot create a file when that file already exists. #61178

Open

Replace hardcoded attribute count checks and specific attribute type …

0222c7c

…assertions Signed-off-by: Medha Tiwari <medhavns1@gmail.com>

jkotas reviewed Jul 23, 2025

View reviewed changes

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs Outdated Show resolved Hide resolved

Update src/Mvc/Mvc.Core/test/ModelBinding/Metadata/DefaultModelMetada…

4eabce6

…taProviderTest.cs

jkotas reviewed Jul 23, 2025

View reviewed changes

src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ModelAttributesTest.cs Outdated Show resolved Hide resolved

Update src/Mvc/Mvc.Core/test/ModelBinding/Metadata/ModelAttributesTes…

efd8183

…t.cs

jkotas approved these changes Jul 23, 2025

View reviewed changes

captainsafia approved these changes Jul 23, 2025

View reviewed changes

captainsafia merged commit 90236e1 into dotnet:main Jul 23, 2025
30 checks passed

dotnet-policy-service bot added this to the 10.0-preview7 milestone Jul 23, 2025

wtgodbe modified the milestones: 10.0-preview7, 10.0-rc1 Jul 23, 2025

Fix MVC Core tests for CoreCLR GetCustomAttributes consistency #62872

Fix MVC Core tests for CoreCLR GetCustomAttributes consistency #62872

Uh oh!

Conversation

medhatiwari commented Jul 23, 2025

Fix MVC Core tests for CoreCLR GetCustomAttributes consistency

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

jkotas Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

medhatiwari Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

jkotas Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jkotas left a comment

Choose a reason for hiding this comment

Uh oh!

captainsafia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!