Skip to content

Conversation

@jonathanpeppers
Copy link
Member

prop.Value.GetRawText () returns a string value foo as "foo".

Let's instead switch on JsonValueKind and handle each type appropriately.

The existing AppContextTests should verify this when we enable the RuntimeConfig category.

`prop.Value.GetRawText ()` returns a string value `foo` as `"foo"`.

Let's instead switch on `JsonValueKind` and handle each type
appropriately.

The existing `AppContextTests` should verify this when we enable the
`RuntimeConfig` category.
@jonathanpeppers jonathanpeppers marked this pull request as ready for review January 8, 2026 16:52
@jonathanpeppers jonathanpeppers mentioned this pull request Jan 8, 2026
8 tasks
Copy link

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 bug in RuntimePropertiesParser where JSON string values from runtimeconfig.json were being parsed with surrounding quotes (e.g., "foo" instead of foo) due to using GetRawText() for all value types. The fix switches on JsonValueKind to handle each JSON type appropriately, and enables the RuntimeConfig test category to verify the fix.

Key Changes:

  • Implemented GetJsonValueAsString() helper method that properly handles String, Boolean, Number, Object, Array, Null, and Undefined JSON types
  • Enabled RuntimeConfig test category for CoreCLR builds to activate AppContextTests
  • Fixed null value handling by skipping properties with null/undefined values

Reviewed changes

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

File Description
src/Xamarin.Android.Build.Tasks/Utilities/RuntimePropertiesParser.cs Added GetJsonValueAsString() method to properly convert JSON values to strings based on their JsonValueKind, fixing the string value parsing bug
tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj Removed RuntimeConfig from the excluded test categories for CoreCLR, enabling AppContextTests to verify the fix

@jonathanpeppers
Copy link
Member Author

Two failures are known flaky, being addressed in other PRs:

image

@jonathanpeppers jonathanpeppers merged commit c4b72d3 into main Jan 8, 2026
62 of 65 checks passed
@jonathanpeppers jonathanpeppers deleted the dev/peppers/coreclr-runtimeconfig branch January 8, 2026 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants