Skip to content

[clr-ios] Enable Text.Json tests on CoreCLR Apple mobile#127497

Open
Copilot wants to merge 7 commits intomainfrom
copilot/increase-job-timeout-and-capture-failures
Open

[clr-ios] Enable Text.Json tests on CoreCLR Apple mobile#127497
Copilot wants to merge 7 commits intomainfrom
copilot/increase-job-timeout-and-capture-failures

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 28, 2026

Description

This PR enables System.Text.Json.Tests that were excluded across all CoreCLR Apple mobile targets.

Copilot AI and others added 3 commits April 27, 2026 14:45
…imeout to 360min

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/c3f2dcb7-6d17-4f5a-9556-33d8131f72d9

Co-authored-by: kotlarmilos <11523312+kotlarmilos@users.noreply.github.com>
…stener / Text.Json tests

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/4430fd2d-8a70-450f-88c4-4b7930c3e58d

Co-authored-by: kotlarmilos <11523312+kotlarmilos@users.noreply.github.com>
Copilot AI self-assigned this Apr 28, 2026
Copilot AI review requested due to automatic review settings April 28, 2026 09:50
Copilot AI review requested due to automatic review settings April 28, 2026 09:50
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to 'os-ios': @vitek-karas, @kotlarmilos, @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

@kotlarmilos
Copy link
Copy Markdown
Member

/azp run runtime-ioslike

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI review requested due to automatic review settings April 28, 2026 14:11
@kotlarmilos kotlarmilos changed the title Re-enable Concurrent / HttpListener / Text.Json tests on CoreCLR Apple mobile Re-enable Concurrent and Text.Json tests on CoreCLR Apple mobile Apr 28, 2026
@kotlarmilos kotlarmilos marked this pull request as ready for review April 28, 2026 14:13
Copy link
Copy Markdown
Contributor

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 re-enables System.Collections.Concurrent.Tests and System.Text.Json.Tests for CoreCLR Apple mobile test runs by removing their project-level exclusions, while adding a targeted [ActiveIssue] skip for a known-failing ConcurrentDictionary test on CoreCLR Apple mobile.

Changes:

  • Removed CoreCLR Apple mobile exclusions for System.Collections.Concurrent.Tests and System.Text.Json.Tests from src/libraries/tests.proj.
  • Added an [ActiveIssue] annotation to skip a specific ConcurrentDictionary theory on IsAppleMobile && IsCoreCLR.
Show a summary per file
File Description
src/libraries/tests.proj Stops excluding Concurrent and Text.Json test projects for CoreCLR Apple mobile targets, allowing them to run again.
src/libraries/System.Collections.Concurrent/tests/ConcurrentDictionary/ConcurrentDictionary.Generic.Tests.cs Adds a targeted active-issue skip for a specific failing test on CoreCLR Apple mobile.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@github-actions

This comment has been minimized.

@kotlarmilos
Copy link
Copy Markdown
Member

/azp run runtime-ioslike

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

@kotlarmilos
Copy link
Copy Markdown
Member

/azp run runtime-ioslike

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

…bile

Reverts the project-level re-enable of System.Collections.Concurrent.Tests:
  exclusions in the TargetsAppleMobile && CoreCLR ItemGroup in tests.proj.
- Drop the [ActiveIssue] on NonRandomizedToRandomizedUpgrade_FunctionsCorrectly
  since the entire project is excluded on CoreCLR Apple mobile.
- Revert the work-item timeout bump in sendtohelixhelp.proj back to 01:15:00.

System.Text.Json.Tests stays re-enabled on CoreCLR Apple mobile.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 29, 2026 13:25
@kotlarmilos kotlarmilos changed the title Re-enable Concurrent and Text.Json tests on CoreCLR Apple mobile Re-enable Text.Json tests on CoreCLR Apple mobile Apr 29, 2026
@kotlarmilos kotlarmilos changed the title Re-enable Text.Json tests on CoreCLR Apple mobile [clr-ios] Enable Text.Json tests on CoreCLR Apple mobile Apr 29, 2026
Copy link
Copy Markdown
Contributor

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.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127497

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR removes System.Text.Json.Tests from the project exclusion list for CoreCLR Apple mobile targets. The test was previously excluded due to timeouts, and the commit history shows the author iteratively verified that this specific test suite now passes within acceptable time limits while keeping other timeout-prone tests (Concurrent, HttpListener) excluded.

Approach: Removing the exclusion line from the TargetsAppleMobile && CoreCLR && !NativeAOT ItemGroup is the correct and minimal approach to re-enable these tests. The iterative commit history demonstrates due diligence — timeouts were bumped, tests were re-enabled in batches, and tests that still timed out were re-excluded.

Summary: ✅ LGTM. This is a well-scoped, minimal infrastructure change that re-enables a previously-excluded test suite. The commit history shows the author validated that System.Text.Json.Tests passes on CoreCLR Apple mobile while appropriately keeping other problematic test suites excluded. CI will provide final confirmation.


Detailed Findings

✅ Change is correct and well-scoped

The net diff removes a single ProjectExclusions line for System.Text.Json.Tests.csproj from the ItemGroup conditioned on '$(TargetsAppleMobile)' == 'true' and '$(RuntimeFlavor)' == 'CoreCLR' and '$(UseNativeAOTRuntime)' != 'true'. The surrounding exclusions for System.Collections.Concurrent.Tests and System.Net.HttpListener.Tests (which still timeout) are appropriately retained under the <!-- Timeouts --> comment.

✅ Commit history shows iterative validation

The PR went through multiple iterations: initially re-enabling several test suites and bumping timeouts, then reverting changes for tests that still timed out, arriving at the focused change of only enabling Text.Json tests. This demonstrates proper validation rather than a speculative change.

💡 Minor observation — no tracking issue linked

The "Timeouts" exclusions do not reference a specific tracking issue (unlike the <!-- https://github.com/dotnet/runtime/issues/124344 --> exclusions above). Consider adding a brief comment or linking to a tracking issue for the remaining timeout exclusions so future contributors know when to revisit them. This is non-blocking and could be addressed separately.

Generated by Code Review for issue #127497 ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants