Skip to content

Conversation

medhatiwari
Copy link
Contributor

This PR fixes a test failure on Mono runtime by removing dependency on attribute ordering in ModelAttributesTest.GetAttributesForParameter_SomeAttributes

Description

The test was failing on Mono runtime because it assumed SerializableAttribute would be the first attribute returned by GetCustomAttributes()

Failed Microsoft.AspNetCore.Mvc.ModelBinding.ModelAttributesTest.GetAttributesForParameter_SomeAttributes
Error: Assert.IsType() Failure: Value is not the exact type
Expected: typeof(System.SerializableAttribute) 
Actual:   typeof(System.Runtime.CompilerServices.IsReadOnlyAttribute)

see detail here

As pointed out by @jkotas in dotnet/runtime#117864:

"The order of attributes is not significant. [...] If the test depends on attributes being returned in a specific order, it should be fixed to not do that."

The C# Specification confirms that attribute order is not guaranteed

This change ensures the test verifies the presence of the expected attribute without depending on its position, making it compliant with the C# specification and compatible across all .NET runtimes.

cc: @jkotas

@Copilot Copilot AI review requested due to automatic review settings July 24, 2025 18:50
@medhatiwari medhatiwari requested a review from a team as a code owner July 24, 2025 18:50
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 fixes a test failure on Mono runtime by removing the dependency on attribute ordering in ModelAttributesTest.GetAttributesForParameter_SomeAttributes. The test was failing because it assumed SerializableAttribute would always be the first attribute returned by GetCustomAttributes(), but attribute order is not guaranteed by the C# specification.

Key Changes

  • Replaced order-dependent Assert.Collection with order-agnostic Assert.Contains check
  • Updated test to verify presence of SerializableAttribute without assuming its position
  • Added explanatory comment about the order-agnostic nature of the check

@github-actions github-actions bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jul 24, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 24, 2025
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM!

@captainsafia captainsafia merged commit bcfa508 into dotnet:main Jul 25, 2025
31 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-rc1 milestone Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants