Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes - #126006

Merged
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests
Mar 27, 2026
Merged

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes#126006
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests

Conversation

CopilotAI commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

EdgeCaseTests.CanReadArrayOfAnySize repeatedly times out (~14+ min) on the checked coreclr windows x64 Release CI leg. The 2 GB array case (Array.MaxLength = 2,147,483,591) was previously gated by #if RELEASE && NET, enabling it for Release library builds. However, checked coreclr's extra assertions/validation make processing a 2 GB array prohibitively slow.

Description

Instead of skipping all test cases on non-release runtimes, the fix adds a targeted in-method skip guard for only the 2 GB case. The #if RELEASE && NET preprocessor guard and [ConditionalTheory] with platform conditions are replaced with [ConditionalTheory] (no parameters, to allow SkipTestException to be treated as a skip rather than a failure) and a runtime check inside the method body:

[ConditionalTheory][InlineData(100)][InlineData(64_001)][InlineData(127_000)][InlineData(2147483591)]// Array.MaxLengthpublicvoidCanReadArrayOfAnySize(intlength){if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore)){thrownewSkipTestException("It would take too much time to execute.");}// ...}

The smaller cases (100, 64_001, 127_000) continue to run on all platforms and runtime configurations. The 2 GB case is skipped only when:

  • The process is not 64-bit, or
  • The runtime is not a release runtime (!IsReleaseRuntime, which covers both checked and debug coreclr of any bitness), or
  • The runtime is not .NET Core (i.e. running on .NET Framework)

!PlatformDetection.IsReleaseRuntime is used instead of SlowRuntimeTimeoutModifier != 1 because SlowRuntimeTimeoutModifier returns 1 for checked (non-release, non-debug) builds, meaning it would not correctly skip the 2 GB test case on checked runtimes.

Using [ConditionalTheory] without parameters (instead of plain [Theory]) ensures that SkipTestException is correctly treated as a skip rather than a test failure.

Original prompt

This section details on the original issue you should resolve

<issue_title>System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr</issue_title>
<issue_description>## Build Information
Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Leg Name: Libraries Test Run checked coreclr windows x64 Release

Error Message

{
"ErrorMessage": ["CanReadArrayOfAnySize", "timed out"],
"BuildRetry": false,
"ExcludeConsoleLog": false
}

Description

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out after 14+ minutes on the checked coreclr windows x64 Release leg. The test runs as a long-running test until the Helix work item is killed.

Helix log: https://helix.dot.net/api/2019-06-17/jobs/c6faf158-7c28-4acb-bec6-ddcc415ba5e5/workitems/System.Formats.Nrbf.Tests/console

Previous occurrence: #110285 (closed December 2024)

Pull request where observed: #125961 (codeflow update, unrelated to the failure)

Known issue validation

Build: 🔎https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Error message validated:[CanReadArrayOfAnySize timed out]
Result validation: ✅ Known issue matched with the provided build.
Validation performed at: 3/23/2026 11:03:41 PM UTC

Report

BuildDefinitionTestPull Request
1348938dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125881
1348882dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125174
1348844dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125129
1348773dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125983
1348315dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125961
1348754dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125981
1348752dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125083
1348706dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125439
1348612dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125973
1348585dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125835
1348578dotnet/runtime[System.Formats.Nrbf.Tests.WorkItemExecution](https://dev.azure.co...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…aseRuntime condition
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/185c955f-9d8a-4286-94bf-f0af9fc6b65f
CopilotAI changed the title [WIP] Fix timeout issue in CanReadArrayOfAnySize testFix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesMar 24, 2026
CopilotAI requested a review from danmoseleyMarch 24, 2026 02:19
@danmoseley
danmoseley marked this pull request as ready for review March 24, 2026 02:29
CopilotAI review requested due to automatic review settings March 24, 2026 02:29

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 updates a resource-intensive System.Formats.Nrbf test to avoid timing out when executed on non-Release runtimes (notably Checked CoreCLR), where additional runtime validation makes the existing workload too slow.

Changes:

  • Add PlatformDetection.IsReleaseRuntime to the ConditionalTheory gating EdgeCaseTests.CanReadArrayOfAnySize, skipping it on Checked/Debug runtimes.

@adamsitnikadamsitnik left a comment

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 apply my suggestion, but make sure it builds and tests are passing before you push the commit

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@adamsitnikadamsitnik added the test-enhancement Improvements of test source code label Mar 25, 2026
…f ConditionalTheory
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/127eb19e-8537-496a-aeb8-6a23a106f8cd
auto-merge was automatically disabled March 25, 2026 14:19

Head branch was pushed to by a user without write access

CopilotAI commented Mar 25, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot apply my suggestion, but make sure it builds and tests are passing before you push the commit

Applied in 6a4f571 — build confirmed clean (0 warnings, 0 errors).

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesMar 25, 2026
CopilotAI requested a review from adamsitnikMarch 25, 2026 14:22

@adamsitnikadamsitnik left a comment

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.

LGTM, thank you @danmoseley !

[Theory] does not handle SkipTestException - it treats the throw as a
test failure. [ConditionalTheory] wires up a custom test invoker that
catches SkipTestException and reports the test as skipped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #126006

Holistic Assessment

Motivation: The PR addresses a real, recurring CI timeout — CanReadArrayOfAnySize with the 2 GB Array.MaxLength case takes 14+ minutes on the checked coreclr windows x64 Release leg, killing the Helix work item. The original #if RELEASE && NET compile-time guard was ineffective because it checks the library build configuration (always Release), not the runtime configuration. The fix is well-motivated.

Approach: Replacing the compile-time guard with a runtime skip is the right approach, and using [ConditionalTheory] (no parameters) to enable SkipTestException handling is correct. However, the specific runtime condition has a gap that prevents it from fixing the reported issue.

Summary: ❌ Needs Changes. The skip condition uses PlatformDetection.SlowRuntimeTimeoutModifier != 1, but SlowRuntimeTimeoutModifier returns 1 for Checked runtimes on x64 — the exact configuration that's timing out. The 2 GB test case will still run and timeout on checked coreclr x64.


Detailed Findings

❌ Skip condition does not fire on checked coreclr x64 — the reported CI leg

The condition at line 71:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||PlatformDetection.SlowRuntimeTimeoutModifier!=1||!PlatformDetection.IsNetCore))

On checked coreclr windows x64 (the failing CI leg):

Sub-conditionValueReason
!Is64BitProcessfalseIt's x64
SlowRuntimeTimeoutModifier != 1falseSee trace below
!IsNetCorefalseIt's .NET Core

Result: true && (false \|\| false \|\| false)false — test is NOT skipped, timeout persists.

SlowRuntimeTimeoutModifier trace for Checked runtime on x64:

if(IsReleaseRuntime)// false — it's Checkedreturn1;if(IsRiscV64Process)// false — it's x64returnIsDebugRuntime?10:2;elsereturnIsDebugRuntime?5:1;// IsDebugRuntime is false (Checked ≠ Debug) → returns 1

The three runtime modes (Debug, Checked, Release) are mutually exclusive via AssemblyConfigurationEquals. A Checked runtime is neither Debug nor Release, so SlowRuntimeTimeoutModifier falls through to the else branch and returns 1 — the same as Release.

Note: SlowRuntimeTimeoutModifier's own doc comment says "For use as needed on tests that time out when run on a Debug or Checked runtime", but the implementation doesn't account for Checked on non-RISC-V. That appears to be a pre-existing gap in SlowRuntimeTimeoutModifier itself.

Suggested fix — revert to @adamsitnik's first suggestion which correctly handled this:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore))

Or, if SlowRuntimeTimeoutModifier is preferred, fix it first to return > 1 for Checked runtimes (e.g., return IsDebugRuntime ? 5 : IsCheckedRuntime ? 2 : 1;).

(This finding was independently confirmed by both Claude Opus 4.6 and GPT-5.4.)

[ConditionalTheory] without parameters — Correct

Changing from [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] to bare [ConditionalTheory] is correct:

  • The old attribute skipped all test cases (100, 64K, 127K) on 32-bit, which was unnecessarily restrictive — those small arrays work fine on 32-bit.
  • The 32-bit guard is now applied only to the 2 GB case via the in-method check.
  • [ConditionalTheory] (vs plain [Theory]) is required for SkipTestException to be reported as a skip rather than a failure, as @danmoseley correctly identified.

✅ Smaller test cases now run on all platforms — Good improvement

The smaller InlineData cases (100, 64_001, 127_000) are no longer gated by #if or 64-bit conditions, expanding test coverage.

Note

This review was generated by Copilot (Claude Opus 4.6), with a parallel review from GPT-5.4.

Generated by Code Review for issue #126006 ·

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot address feedback

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

… of any bitness
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/558ab2d2-fd9c-4914-a0aa-7032325fc90c
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
auto-merge was automatically disabled March 26, 2026 15:37

Head branch was pushed to by a user without write access

CopilotAI commented Mar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

Fixed in ff355fb. SlowRuntimeTimeoutModifier returns 1 for checked builds (it only returns >1 for debug), so it didn't catch the timeout case. Switched to !PlatformDetection.IsReleaseRuntime which correctly skips on checked and debug runtimes of any bitness.

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimesMar 26, 2026
@danmoseley

Copy link
Copy Markdown
Contributor

/ba-g one issue opened, the others "cffi_backend" already have issue

@danmoseley
danmoseley merged commit 8204c34 into mainMar 27, 2026
86 of 91 checks passed
@danmoseley
danmoseley deleted the copilot/fix-timeout-edge-case-tests branch March 27, 2026 04:36
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 26, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Formats.Nrbftest-enhancementImprovements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr

4 participants

@danmoseley@adamsitnik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes - #126006

Merged
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests
Mar 27, 2026
Merged

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes#126006
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests

Conversation

CopilotAI commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

EdgeCaseTests.CanReadArrayOfAnySize repeatedly times out (~14+ min) on the checked coreclr windows x64 Release CI leg. The 2 GB array case (Array.MaxLength = 2,147,483,591) was previously gated by #if RELEASE && NET, enabling it for Release library builds. However, checked coreclr's extra assertions/validation make processing a 2 GB array prohibitively slow.

Description

Instead of skipping all test cases on non-release runtimes, the fix adds a targeted in-method skip guard for only the 2 GB case. The #if RELEASE && NET preprocessor guard and [ConditionalTheory] with platform conditions are replaced with [ConditionalTheory] (no parameters, to allow SkipTestException to be treated as a skip rather than a failure) and a runtime check inside the method body:

[ConditionalTheory][InlineData(100)][InlineData(64_001)][InlineData(127_000)][InlineData(2147483591)]// Array.MaxLengthpublicvoidCanReadArrayOfAnySize(intlength){if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore)){thrownewSkipTestException("It would take too much time to execute.");}// ...}

The smaller cases (100, 64_001, 127_000) continue to run on all platforms and runtime configurations. The 2 GB case is skipped only when:

  • The process is not 64-bit, or
  • The runtime is not a release runtime (!IsReleaseRuntime, which covers both checked and debug coreclr of any bitness), or
  • The runtime is not .NET Core (i.e. running on .NET Framework)

!PlatformDetection.IsReleaseRuntime is used instead of SlowRuntimeTimeoutModifier != 1 because SlowRuntimeTimeoutModifier returns 1 for checked (non-release, non-debug) builds, meaning it would not correctly skip the 2 GB test case on checked runtimes.

Using [ConditionalTheory] without parameters (instead of plain [Theory]) ensures that SkipTestException is correctly treated as a skip rather than a test failure.

Original prompt

This section details on the original issue you should resolve

<issue_title>System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr</issue_title>
<issue_description>## Build Information
Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Leg Name: Libraries Test Run checked coreclr windows x64 Release

Error Message

{
"ErrorMessage": ["CanReadArrayOfAnySize", "timed out"],
"BuildRetry": false,
"ExcludeConsoleLog": false
}

Description

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out after 14+ minutes on the checked coreclr windows x64 Release leg. The test runs as a long-running test until the Helix work item is killed.

Helix log: https://helix.dot.net/api/2019-06-17/jobs/c6faf158-7c28-4acb-bec6-ddcc415ba5e5/workitems/System.Formats.Nrbf.Tests/console

Previous occurrence: #110285 (closed December 2024)

Pull request where observed: #125961 (codeflow update, unrelated to the failure)

Known issue validation

Build: 🔎https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Error message validated:[CanReadArrayOfAnySize timed out]
Result validation: ✅ Known issue matched with the provided build.
Validation performed at: 3/23/2026 11:03:41 PM UTC

Report

BuildDefinitionTestPull Request
1348938dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125881
1348882dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125174
1348844dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125129
1348773dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125983
1348315dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125961
1348754dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125981
1348752dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125083
1348706dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125439
1348612dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125973
1348585dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125835
1348578dotnet/runtime[System.Formats.Nrbf.Tests.WorkItemExecution](https://dev.azure.co...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…aseRuntime condition
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/185c955f-9d8a-4286-94bf-f0af9fc6b65f
CopilotAI changed the title [WIP] Fix timeout issue in CanReadArrayOfAnySize testFix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesMar 24, 2026
CopilotAI requested a review from danmoseleyMarch 24, 2026 02:19
@danmoseley
danmoseley marked this pull request as ready for review March 24, 2026 02:29
CopilotAI review requested due to automatic review settings March 24, 2026 02:29

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 updates a resource-intensive System.Formats.Nrbf test to avoid timing out when executed on non-Release runtimes (notably Checked CoreCLR), where additional runtime validation makes the existing workload too slow.

Changes:

  • Add PlatformDetection.IsReleaseRuntime to the ConditionalTheory gating EdgeCaseTests.CanReadArrayOfAnySize, skipping it on Checked/Debug runtimes.

@adamsitnikadamsitnik left a comment

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 apply my suggestion, but make sure it builds and tests are passing before you push the commit

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@adamsitnikadamsitnik added the test-enhancement Improvements of test source code label Mar 25, 2026
…f ConditionalTheory
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/127eb19e-8537-496a-aeb8-6a23a106f8cd
auto-merge was automatically disabled March 25, 2026 14:19

Head branch was pushed to by a user without write access

CopilotAI commented Mar 25, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot apply my suggestion, but make sure it builds and tests are passing before you push the commit

Applied in 6a4f571 — build confirmed clean (0 warnings, 0 errors).

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesMar 25, 2026
CopilotAI requested a review from adamsitnikMarch 25, 2026 14:22

@adamsitnikadamsitnik left a comment

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.

LGTM, thank you @danmoseley !

[Theory] does not handle SkipTestException - it treats the throw as a
test failure. [ConditionalTheory] wires up a custom test invoker that
catches SkipTestException and reports the test as skipped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #126006

Holistic Assessment

Motivation: The PR addresses a real, recurring CI timeout — CanReadArrayOfAnySize with the 2 GB Array.MaxLength case takes 14+ minutes on the checked coreclr windows x64 Release leg, killing the Helix work item. The original #if RELEASE && NET compile-time guard was ineffective because it checks the library build configuration (always Release), not the runtime configuration. The fix is well-motivated.

Approach: Replacing the compile-time guard with a runtime skip is the right approach, and using [ConditionalTheory] (no parameters) to enable SkipTestException handling is correct. However, the specific runtime condition has a gap that prevents it from fixing the reported issue.

Summary: ❌ Needs Changes. The skip condition uses PlatformDetection.SlowRuntimeTimeoutModifier != 1, but SlowRuntimeTimeoutModifier returns 1 for Checked runtimes on x64 — the exact configuration that's timing out. The 2 GB test case will still run and timeout on checked coreclr x64.


Detailed Findings

❌ Skip condition does not fire on checked coreclr x64 — the reported CI leg

The condition at line 71:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||PlatformDetection.SlowRuntimeTimeoutModifier!=1||!PlatformDetection.IsNetCore))

On checked coreclr windows x64 (the failing CI leg):

Sub-conditionValueReason
!Is64BitProcessfalseIt's x64
SlowRuntimeTimeoutModifier != 1falseSee trace below
!IsNetCorefalseIt's .NET Core

Result: true && (false \|\| false \|\| false)false — test is NOT skipped, timeout persists.

SlowRuntimeTimeoutModifier trace for Checked runtime on x64:

if(IsReleaseRuntime)// false — it's Checkedreturn1;if(IsRiscV64Process)// false — it's x64returnIsDebugRuntime?10:2;elsereturnIsDebugRuntime?5:1;// IsDebugRuntime is false (Checked ≠ Debug) → returns 1

The three runtime modes (Debug, Checked, Release) are mutually exclusive via AssemblyConfigurationEquals. A Checked runtime is neither Debug nor Release, so SlowRuntimeTimeoutModifier falls through to the else branch and returns 1 — the same as Release.

Note: SlowRuntimeTimeoutModifier's own doc comment says "For use as needed on tests that time out when run on a Debug or Checked runtime", but the implementation doesn't account for Checked on non-RISC-V. That appears to be a pre-existing gap in SlowRuntimeTimeoutModifier itself.

Suggested fix — revert to @adamsitnik's first suggestion which correctly handled this:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore))

Or, if SlowRuntimeTimeoutModifier is preferred, fix it first to return > 1 for Checked runtimes (e.g., return IsDebugRuntime ? 5 : IsCheckedRuntime ? 2 : 1;).

(This finding was independently confirmed by both Claude Opus 4.6 and GPT-5.4.)

[ConditionalTheory] without parameters — Correct

Changing from [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] to bare [ConditionalTheory] is correct:

  • The old attribute skipped all test cases (100, 64K, 127K) on 32-bit, which was unnecessarily restrictive — those small arrays work fine on 32-bit.
  • The 32-bit guard is now applied only to the 2 GB case via the in-method check.
  • [ConditionalTheory] (vs plain [Theory]) is required for SkipTestException to be reported as a skip rather than a failure, as @danmoseley correctly identified.

✅ Smaller test cases now run on all platforms — Good improvement

The smaller InlineData cases (100, 64_001, 127_000) are no longer gated by #if or 64-bit conditions, expanding test coverage.

Note

This review was generated by Copilot (Claude Opus 4.6), with a parallel review from GPT-5.4.

Generated by Code Review for issue #126006 ·

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot address feedback

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

… of any bitness
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/558ab2d2-fd9c-4914-a0aa-7032325fc90c
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
auto-merge was automatically disabled March 26, 2026 15:37

Head branch was pushed to by a user without write access

CopilotAI commented Mar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

Fixed in ff355fb. SlowRuntimeTimeoutModifier returns 1 for checked builds (it only returns >1 for debug), so it didn't catch the timeout case. Switched to !PlatformDetection.IsReleaseRuntime which correctly skips on checked and debug runtimes of any bitness.

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimesMar 26, 2026
@danmoseley

Copy link
Copy Markdown
Contributor

/ba-g one issue opened, the others "cffi_backend" already have issue

@danmoseley
danmoseley merged commit 8204c34 into mainMar 27, 2026
86 of 91 checks passed
@danmoseley
danmoseley deleted the copilot/fix-timeout-edge-case-tests branch March 27, 2026 04:36
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 26, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Formats.Nrbftest-enhancementImprovements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr

4 participants

@danmoseley@adamsitnik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes - #126006

Merged
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests
Mar 27, 2026
Merged

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes#126006
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests

Conversation

CopilotAI commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

EdgeCaseTests.CanReadArrayOfAnySize repeatedly times out (~14+ min) on the checked coreclr windows x64 Release CI leg. The 2 GB array case (Array.MaxLength = 2,147,483,591) was previously gated by #if RELEASE && NET, enabling it for Release library builds. However, checked coreclr's extra assertions/validation make processing a 2 GB array prohibitively slow.

Description

Instead of skipping all test cases on non-release runtimes, the fix adds a targeted in-method skip guard for only the 2 GB case. The #if RELEASE && NET preprocessor guard and [ConditionalTheory] with platform conditions are replaced with [ConditionalTheory] (no parameters, to allow SkipTestException to be treated as a skip rather than a failure) and a runtime check inside the method body:

[ConditionalTheory][InlineData(100)][InlineData(64_001)][InlineData(127_000)][InlineData(2147483591)]// Array.MaxLengthpublicvoidCanReadArrayOfAnySize(intlength){if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore)){thrownewSkipTestException("It would take too much time to execute.");}// ...}

The smaller cases (100, 64_001, 127_000) continue to run on all platforms and runtime configurations. The 2 GB case is skipped only when:

  • The process is not 64-bit, or
  • The runtime is not a release runtime (!IsReleaseRuntime, which covers both checked and debug coreclr of any bitness), or
  • The runtime is not .NET Core (i.e. running on .NET Framework)

!PlatformDetection.IsReleaseRuntime is used instead of SlowRuntimeTimeoutModifier != 1 because SlowRuntimeTimeoutModifier returns 1 for checked (non-release, non-debug) builds, meaning it would not correctly skip the 2 GB test case on checked runtimes.

Using [ConditionalTheory] without parameters (instead of plain [Theory]) ensures that SkipTestException is correctly treated as a skip rather than a test failure.

Original prompt

This section details on the original issue you should resolve

<issue_title>System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr</issue_title>
<issue_description>## Build Information
Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Leg Name: Libraries Test Run checked coreclr windows x64 Release

Error Message

{
"ErrorMessage": ["CanReadArrayOfAnySize", "timed out"],
"BuildRetry": false,
"ExcludeConsoleLog": false
}

Description

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out after 14+ minutes on the checked coreclr windows x64 Release leg. The test runs as a long-running test until the Helix work item is killed.

Helix log: https://helix.dot.net/api/2019-06-17/jobs/c6faf158-7c28-4acb-bec6-ddcc415ba5e5/workitems/System.Formats.Nrbf.Tests/console

Previous occurrence: #110285 (closed December 2024)

Pull request where observed: #125961 (codeflow update, unrelated to the failure)

Known issue validation

Build: 🔎https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Error message validated:[CanReadArrayOfAnySize timed out]
Result validation: ✅ Known issue matched with the provided build.
Validation performed at: 3/23/2026 11:03:41 PM UTC

Report

BuildDefinitionTestPull Request
1348938dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125881
1348882dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125174
1348844dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125129
1348773dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125983
1348315dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125961
1348754dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125981
1348752dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125083
1348706dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125439
1348612dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125973
1348585dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125835
1348578dotnet/runtime[System.Formats.Nrbf.Tests.WorkItemExecution](https://dev.azure.co...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…aseRuntime condition
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/185c955f-9d8a-4286-94bf-f0af9fc6b65f
CopilotAI changed the title [WIP] Fix timeout issue in CanReadArrayOfAnySize testFix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesMar 24, 2026
CopilotAI requested a review from danmoseleyMarch 24, 2026 02:19
@danmoseley
danmoseley marked this pull request as ready for review March 24, 2026 02:29
CopilotAI review requested due to automatic review settings March 24, 2026 02:29

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 updates a resource-intensive System.Formats.Nrbf test to avoid timing out when executed on non-Release runtimes (notably Checked CoreCLR), where additional runtime validation makes the existing workload too slow.

Changes:

  • Add PlatformDetection.IsReleaseRuntime to the ConditionalTheory gating EdgeCaseTests.CanReadArrayOfAnySize, skipping it on Checked/Debug runtimes.

@adamsitnikadamsitnik left a comment

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 apply my suggestion, but make sure it builds and tests are passing before you push the commit

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@adamsitnikadamsitnik added the test-enhancement Improvements of test source code label Mar 25, 2026
…f ConditionalTheory
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/127eb19e-8537-496a-aeb8-6a23a106f8cd
auto-merge was automatically disabled March 25, 2026 14:19

Head branch was pushed to by a user without write access

CopilotAI commented Mar 25, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot apply my suggestion, but make sure it builds and tests are passing before you push the commit

Applied in 6a4f571 — build confirmed clean (0 warnings, 0 errors).

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesMar 25, 2026
CopilotAI requested a review from adamsitnikMarch 25, 2026 14:22

@adamsitnikadamsitnik left a comment

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.

LGTM, thank you @danmoseley !

[Theory] does not handle SkipTestException - it treats the throw as a
test failure. [ConditionalTheory] wires up a custom test invoker that
catches SkipTestException and reports the test as skipped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #126006

Holistic Assessment

Motivation: The PR addresses a real, recurring CI timeout — CanReadArrayOfAnySize with the 2 GB Array.MaxLength case takes 14+ minutes on the checked coreclr windows x64 Release leg, killing the Helix work item. The original #if RELEASE && NET compile-time guard was ineffective because it checks the library build configuration (always Release), not the runtime configuration. The fix is well-motivated.

Approach: Replacing the compile-time guard with a runtime skip is the right approach, and using [ConditionalTheory] (no parameters) to enable SkipTestException handling is correct. However, the specific runtime condition has a gap that prevents it from fixing the reported issue.

Summary: ❌ Needs Changes. The skip condition uses PlatformDetection.SlowRuntimeTimeoutModifier != 1, but SlowRuntimeTimeoutModifier returns 1 for Checked runtimes on x64 — the exact configuration that's timing out. The 2 GB test case will still run and timeout on checked coreclr x64.


Detailed Findings

❌ Skip condition does not fire on checked coreclr x64 — the reported CI leg

The condition at line 71:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||PlatformDetection.SlowRuntimeTimeoutModifier!=1||!PlatformDetection.IsNetCore))

On checked coreclr windows x64 (the failing CI leg):

Sub-conditionValueReason
!Is64BitProcessfalseIt's x64
SlowRuntimeTimeoutModifier != 1falseSee trace below
!IsNetCorefalseIt's .NET Core

Result: true && (false \|\| false \|\| false)false — test is NOT skipped, timeout persists.

SlowRuntimeTimeoutModifier trace for Checked runtime on x64:

if(IsReleaseRuntime)// false — it's Checkedreturn1;if(IsRiscV64Process)// false — it's x64returnIsDebugRuntime?10:2;elsereturnIsDebugRuntime?5:1;// IsDebugRuntime is false (Checked ≠ Debug) → returns 1

The three runtime modes (Debug, Checked, Release) are mutually exclusive via AssemblyConfigurationEquals. A Checked runtime is neither Debug nor Release, so SlowRuntimeTimeoutModifier falls through to the else branch and returns 1 — the same as Release.

Note: SlowRuntimeTimeoutModifier's own doc comment says "For use as needed on tests that time out when run on a Debug or Checked runtime", but the implementation doesn't account for Checked on non-RISC-V. That appears to be a pre-existing gap in SlowRuntimeTimeoutModifier itself.

Suggested fix — revert to @adamsitnik's first suggestion which correctly handled this:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore))

Or, if SlowRuntimeTimeoutModifier is preferred, fix it first to return > 1 for Checked runtimes (e.g., return IsDebugRuntime ? 5 : IsCheckedRuntime ? 2 : 1;).

(This finding was independently confirmed by both Claude Opus 4.6 and GPT-5.4.)

[ConditionalTheory] without parameters — Correct

Changing from [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] to bare [ConditionalTheory] is correct:

  • The old attribute skipped all test cases (100, 64K, 127K) on 32-bit, which was unnecessarily restrictive — those small arrays work fine on 32-bit.
  • The 32-bit guard is now applied only to the 2 GB case via the in-method check.
  • [ConditionalTheory] (vs plain [Theory]) is required for SkipTestException to be reported as a skip rather than a failure, as @danmoseley correctly identified.

✅ Smaller test cases now run on all platforms — Good improvement

The smaller InlineData cases (100, 64_001, 127_000) are no longer gated by #if or 64-bit conditions, expanding test coverage.

Note

This review was generated by Copilot (Claude Opus 4.6), with a parallel review from GPT-5.4.

Generated by Code Review for issue #126006 ·

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot address feedback

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

… of any bitness
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/558ab2d2-fd9c-4914-a0aa-7032325fc90c
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
auto-merge was automatically disabled March 26, 2026 15:37

Head branch was pushed to by a user without write access

CopilotAI commented Mar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

Fixed in ff355fb. SlowRuntimeTimeoutModifier returns 1 for checked builds (it only returns >1 for debug), so it didn't catch the timeout case. Switched to !PlatformDetection.IsReleaseRuntime which correctly skips on checked and debug runtimes of any bitness.

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimesMar 26, 2026
@danmoseley

Copy link
Copy Markdown
Contributor

/ba-g one issue opened, the others "cffi_backend" already have issue

@danmoseley
danmoseley merged commit 8204c34 into mainMar 27, 2026
86 of 91 checks passed
@danmoseley
danmoseley deleted the copilot/fix-timeout-edge-case-tests branch March 27, 2026 04:36
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 26, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Formats.Nrbftest-enhancementImprovements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr

4 participants

@danmoseley@adamsitnik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes - #126006

Merged
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests
Mar 27, 2026
Merged

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes#126006
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests

Conversation

CopilotAI commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

EdgeCaseTests.CanReadArrayOfAnySize repeatedly times out (~14+ min) on the checked coreclr windows x64 Release CI leg. The 2 GB array case (Array.MaxLength = 2,147,483,591) was previously gated by #if RELEASE && NET, enabling it for Release library builds. However, checked coreclr's extra assertions/validation make processing a 2 GB array prohibitively slow.

Description

Instead of skipping all test cases on non-release runtimes, the fix adds a targeted in-method skip guard for only the 2 GB case. The #if RELEASE && NET preprocessor guard and [ConditionalTheory] with platform conditions are replaced with [ConditionalTheory] (no parameters, to allow SkipTestException to be treated as a skip rather than a failure) and a runtime check inside the method body:

[ConditionalTheory][InlineData(100)][InlineData(64_001)][InlineData(127_000)][InlineData(2147483591)]// Array.MaxLengthpublicvoidCanReadArrayOfAnySize(intlength){if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore)){thrownewSkipTestException("It would take too much time to execute.");}// ...}

The smaller cases (100, 64_001, 127_000) continue to run on all platforms and runtime configurations. The 2 GB case is skipped only when:

  • The process is not 64-bit, or
  • The runtime is not a release runtime (!IsReleaseRuntime, which covers both checked and debug coreclr of any bitness), or
  • The runtime is not .NET Core (i.e. running on .NET Framework)

!PlatformDetection.IsReleaseRuntime is used instead of SlowRuntimeTimeoutModifier != 1 because SlowRuntimeTimeoutModifier returns 1 for checked (non-release, non-debug) builds, meaning it would not correctly skip the 2 GB test case on checked runtimes.

Using [ConditionalTheory] without parameters (instead of plain [Theory]) ensures that SkipTestException is correctly treated as a skip rather than a test failure.

Original prompt

This section details on the original issue you should resolve

<issue_title>System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr</issue_title>
<issue_description>## Build Information
Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Leg Name: Libraries Test Run checked coreclr windows x64 Release

Error Message

{
"ErrorMessage": ["CanReadArrayOfAnySize", "timed out"],
"BuildRetry": false,
"ExcludeConsoleLog": false
}

Description

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out after 14+ minutes on the checked coreclr windows x64 Release leg. The test runs as a long-running test until the Helix work item is killed.

Helix log: https://helix.dot.net/api/2019-06-17/jobs/c6faf158-7c28-4acb-bec6-ddcc415ba5e5/workitems/System.Formats.Nrbf.Tests/console

Previous occurrence: #110285 (closed December 2024)

Pull request where observed: #125961 (codeflow update, unrelated to the failure)

Known issue validation

Build: 🔎https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Error message validated:[CanReadArrayOfAnySize timed out]
Result validation: ✅ Known issue matched with the provided build.
Validation performed at: 3/23/2026 11:03:41 PM UTC

Report

BuildDefinitionTestPull Request
1348938dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125881
1348882dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125174
1348844dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125129
1348773dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125983
1348315dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125961
1348754dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125981
1348752dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125083
1348706dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125439
1348612dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125973
1348585dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125835
1348578dotnet/runtime[System.Formats.Nrbf.Tests.WorkItemExecution](https://dev.azure.co...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…aseRuntime condition
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/185c955f-9d8a-4286-94bf-f0af9fc6b65f
CopilotAI changed the title [WIP] Fix timeout issue in CanReadArrayOfAnySize testFix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesMar 24, 2026
CopilotAI requested a review from danmoseleyMarch 24, 2026 02:19
@danmoseley
danmoseley marked this pull request as ready for review March 24, 2026 02:29
CopilotAI review requested due to automatic review settings March 24, 2026 02:29

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 updates a resource-intensive System.Formats.Nrbf test to avoid timing out when executed on non-Release runtimes (notably Checked CoreCLR), where additional runtime validation makes the existing workload too slow.

Changes:

  • Add PlatformDetection.IsReleaseRuntime to the ConditionalTheory gating EdgeCaseTests.CanReadArrayOfAnySize, skipping it on Checked/Debug runtimes.

@adamsitnikadamsitnik left a comment

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 apply my suggestion, but make sure it builds and tests are passing before you push the commit

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@adamsitnikadamsitnik added the test-enhancement Improvements of test source code label Mar 25, 2026
…f ConditionalTheory
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/127eb19e-8537-496a-aeb8-6a23a106f8cd
auto-merge was automatically disabled March 25, 2026 14:19

Head branch was pushed to by a user without write access

CopilotAI commented Mar 25, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot apply my suggestion, but make sure it builds and tests are passing before you push the commit

Applied in 6a4f571 — build confirmed clean (0 warnings, 0 errors).

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesMar 25, 2026
CopilotAI requested a review from adamsitnikMarch 25, 2026 14:22

@adamsitnikadamsitnik left a comment

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.

LGTM, thank you @danmoseley !

[Theory] does not handle SkipTestException - it treats the throw as a
test failure. [ConditionalTheory] wires up a custom test invoker that
catches SkipTestException and reports the test as skipped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #126006

Holistic Assessment

Motivation: The PR addresses a real, recurring CI timeout — CanReadArrayOfAnySize with the 2 GB Array.MaxLength case takes 14+ minutes on the checked coreclr windows x64 Release leg, killing the Helix work item. The original #if RELEASE && NET compile-time guard was ineffective because it checks the library build configuration (always Release), not the runtime configuration. The fix is well-motivated.

Approach: Replacing the compile-time guard with a runtime skip is the right approach, and using [ConditionalTheory] (no parameters) to enable SkipTestException handling is correct. However, the specific runtime condition has a gap that prevents it from fixing the reported issue.

Summary: ❌ Needs Changes. The skip condition uses PlatformDetection.SlowRuntimeTimeoutModifier != 1, but SlowRuntimeTimeoutModifier returns 1 for Checked runtimes on x64 — the exact configuration that's timing out. The 2 GB test case will still run and timeout on checked coreclr x64.


Detailed Findings

❌ Skip condition does not fire on checked coreclr x64 — the reported CI leg

The condition at line 71:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||PlatformDetection.SlowRuntimeTimeoutModifier!=1||!PlatformDetection.IsNetCore))

On checked coreclr windows x64 (the failing CI leg):

Sub-conditionValueReason
!Is64BitProcessfalseIt's x64
SlowRuntimeTimeoutModifier != 1falseSee trace below
!IsNetCorefalseIt's .NET Core

Result: true && (false \|\| false \|\| false)false — test is NOT skipped, timeout persists.

SlowRuntimeTimeoutModifier trace for Checked runtime on x64:

if(IsReleaseRuntime)// false — it's Checkedreturn1;if(IsRiscV64Process)// false — it's x64returnIsDebugRuntime?10:2;elsereturnIsDebugRuntime?5:1;// IsDebugRuntime is false (Checked ≠ Debug) → returns 1

The three runtime modes (Debug, Checked, Release) are mutually exclusive via AssemblyConfigurationEquals. A Checked runtime is neither Debug nor Release, so SlowRuntimeTimeoutModifier falls through to the else branch and returns 1 — the same as Release.

Note: SlowRuntimeTimeoutModifier's own doc comment says "For use as needed on tests that time out when run on a Debug or Checked runtime", but the implementation doesn't account for Checked on non-RISC-V. That appears to be a pre-existing gap in SlowRuntimeTimeoutModifier itself.

Suggested fix — revert to @adamsitnik's first suggestion which correctly handled this:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore))

Or, if SlowRuntimeTimeoutModifier is preferred, fix it first to return > 1 for Checked runtimes (e.g., return IsDebugRuntime ? 5 : IsCheckedRuntime ? 2 : 1;).

(This finding was independently confirmed by both Claude Opus 4.6 and GPT-5.4.)

[ConditionalTheory] without parameters — Correct

Changing from [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] to bare [ConditionalTheory] is correct:

  • The old attribute skipped all test cases (100, 64K, 127K) on 32-bit, which was unnecessarily restrictive — those small arrays work fine on 32-bit.
  • The 32-bit guard is now applied only to the 2 GB case via the in-method check.
  • [ConditionalTheory] (vs plain [Theory]) is required for SkipTestException to be reported as a skip rather than a failure, as @danmoseley correctly identified.

✅ Smaller test cases now run on all platforms — Good improvement

The smaller InlineData cases (100, 64_001, 127_000) are no longer gated by #if or 64-bit conditions, expanding test coverage.

Note

This review was generated by Copilot (Claude Opus 4.6), with a parallel review from GPT-5.4.

Generated by Code Review for issue #126006 ·

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot address feedback

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

… of any bitness
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/558ab2d2-fd9c-4914-a0aa-7032325fc90c
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
auto-merge was automatically disabled March 26, 2026 15:37

Head branch was pushed to by a user without write access

CopilotAI commented Mar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

Fixed in ff355fb. SlowRuntimeTimeoutModifier returns 1 for checked builds (it only returns >1 for debug), so it didn't catch the timeout case. Switched to !PlatformDetection.IsReleaseRuntime which correctly skips on checked and debug runtimes of any bitness.

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimesMar 26, 2026
@danmoseley

Copy link
Copy Markdown
Contributor

/ba-g one issue opened, the others "cffi_backend" already have issue

@danmoseley
danmoseley merged commit 8204c34 into mainMar 27, 2026
86 of 91 checks passed
@danmoseley
danmoseley deleted the copilot/fix-timeout-edge-case-tests branch March 27, 2026 04:36
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 26, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Formats.Nrbftest-enhancementImprovements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr

4 participants

@danmoseley@adamsitnik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes - #126006

Merged
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests
Mar 27, 2026
Merged

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes#126006
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests

Conversation

CopilotAI commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

EdgeCaseTests.CanReadArrayOfAnySize repeatedly times out (~14+ min) on the checked coreclr windows x64 Release CI leg. The 2 GB array case (Array.MaxLength = 2,147,483,591) was previously gated by #if RELEASE && NET, enabling it for Release library builds. However, checked coreclr's extra assertions/validation make processing a 2 GB array prohibitively slow.

Description

Instead of skipping all test cases on non-release runtimes, the fix adds a targeted in-method skip guard for only the 2 GB case. The #if RELEASE && NET preprocessor guard and [ConditionalTheory] with platform conditions are replaced with [ConditionalTheory] (no parameters, to allow SkipTestException to be treated as a skip rather than a failure) and a runtime check inside the method body:

[ConditionalTheory][InlineData(100)][InlineData(64_001)][InlineData(127_000)][InlineData(2147483591)]// Array.MaxLengthpublicvoidCanReadArrayOfAnySize(intlength){if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore)){thrownewSkipTestException("It would take too much time to execute.");}// ...}

The smaller cases (100, 64_001, 127_000) continue to run on all platforms and runtime configurations. The 2 GB case is skipped only when:

  • The process is not 64-bit, or
  • The runtime is not a release runtime (!IsReleaseRuntime, which covers both checked and debug coreclr of any bitness), or
  • The runtime is not .NET Core (i.e. running on .NET Framework)

!PlatformDetection.IsReleaseRuntime is used instead of SlowRuntimeTimeoutModifier != 1 because SlowRuntimeTimeoutModifier returns 1 for checked (non-release, non-debug) builds, meaning it would not correctly skip the 2 GB test case on checked runtimes.

Using [ConditionalTheory] without parameters (instead of plain [Theory]) ensures that SkipTestException is correctly treated as a skip rather than a test failure.

Original prompt

This section details on the original issue you should resolve

<issue_title>System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr</issue_title>
<issue_description>## Build Information
Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Leg Name: Libraries Test Run checked coreclr windows x64 Release

Error Message

{
"ErrorMessage": ["CanReadArrayOfAnySize", "timed out"],
"BuildRetry": false,
"ExcludeConsoleLog": false
}

Description

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out after 14+ minutes on the checked coreclr windows x64 Release leg. The test runs as a long-running test until the Helix work item is killed.

Helix log: https://helix.dot.net/api/2019-06-17/jobs/c6faf158-7c28-4acb-bec6-ddcc415ba5e5/workitems/System.Formats.Nrbf.Tests/console

Previous occurrence: #110285 (closed December 2024)

Pull request where observed: #125961 (codeflow update, unrelated to the failure)

Known issue validation

Build: 🔎https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Error message validated:[CanReadArrayOfAnySize timed out]
Result validation: ✅ Known issue matched with the provided build.
Validation performed at: 3/23/2026 11:03:41 PM UTC

Report

BuildDefinitionTestPull Request
1348938dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125881
1348882dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125174
1348844dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125129
1348773dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125983
1348315dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125961
1348754dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125981
1348752dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125083
1348706dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125439
1348612dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125973
1348585dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125835
1348578dotnet/runtime[System.Formats.Nrbf.Tests.WorkItemExecution](https://dev.azure.co...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…aseRuntime condition
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/185c955f-9d8a-4286-94bf-f0af9fc6b65f
CopilotAI changed the title [WIP] Fix timeout issue in CanReadArrayOfAnySize testFix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesMar 24, 2026
CopilotAI requested a review from danmoseleyMarch 24, 2026 02:19
@danmoseley
danmoseley marked this pull request as ready for review March 24, 2026 02:29
CopilotAI review requested due to automatic review settings March 24, 2026 02:29

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 updates a resource-intensive System.Formats.Nrbf test to avoid timing out when executed on non-Release runtimes (notably Checked CoreCLR), where additional runtime validation makes the existing workload too slow.

Changes:

  • Add PlatformDetection.IsReleaseRuntime to the ConditionalTheory gating EdgeCaseTests.CanReadArrayOfAnySize, skipping it on Checked/Debug runtimes.

@adamsitnikadamsitnik left a comment

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 apply my suggestion, but make sure it builds and tests are passing before you push the commit

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@adamsitnikadamsitnik added the test-enhancement Improvements of test source code label Mar 25, 2026
…f ConditionalTheory
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/127eb19e-8537-496a-aeb8-6a23a106f8cd
auto-merge was automatically disabled March 25, 2026 14:19

Head branch was pushed to by a user without write access

CopilotAI commented Mar 25, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot apply my suggestion, but make sure it builds and tests are passing before you push the commit

Applied in 6a4f571 — build confirmed clean (0 warnings, 0 errors).

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesMar 25, 2026
CopilotAI requested a review from adamsitnikMarch 25, 2026 14:22

@adamsitnikadamsitnik left a comment

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.

LGTM, thank you @danmoseley !

[Theory] does not handle SkipTestException - it treats the throw as a
test failure. [ConditionalTheory] wires up a custom test invoker that
catches SkipTestException and reports the test as skipped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #126006

Holistic Assessment

Motivation: The PR addresses a real, recurring CI timeout — CanReadArrayOfAnySize with the 2 GB Array.MaxLength case takes 14+ minutes on the checked coreclr windows x64 Release leg, killing the Helix work item. The original #if RELEASE && NET compile-time guard was ineffective because it checks the library build configuration (always Release), not the runtime configuration. The fix is well-motivated.

Approach: Replacing the compile-time guard with a runtime skip is the right approach, and using [ConditionalTheory] (no parameters) to enable SkipTestException handling is correct. However, the specific runtime condition has a gap that prevents it from fixing the reported issue.

Summary: ❌ Needs Changes. The skip condition uses PlatformDetection.SlowRuntimeTimeoutModifier != 1, but SlowRuntimeTimeoutModifier returns 1 for Checked runtimes on x64 — the exact configuration that's timing out. The 2 GB test case will still run and timeout on checked coreclr x64.


Detailed Findings

❌ Skip condition does not fire on checked coreclr x64 — the reported CI leg

The condition at line 71:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||PlatformDetection.SlowRuntimeTimeoutModifier!=1||!PlatformDetection.IsNetCore))

On checked coreclr windows x64 (the failing CI leg):

Sub-conditionValueReason
!Is64BitProcessfalseIt's x64
SlowRuntimeTimeoutModifier != 1falseSee trace below
!IsNetCorefalseIt's .NET Core

Result: true && (false \|\| false \|\| false)false — test is NOT skipped, timeout persists.

SlowRuntimeTimeoutModifier trace for Checked runtime on x64:

if(IsReleaseRuntime)// false — it's Checkedreturn1;if(IsRiscV64Process)// false — it's x64returnIsDebugRuntime?10:2;elsereturnIsDebugRuntime?5:1;// IsDebugRuntime is false (Checked ≠ Debug) → returns 1

The three runtime modes (Debug, Checked, Release) are mutually exclusive via AssemblyConfigurationEquals. A Checked runtime is neither Debug nor Release, so SlowRuntimeTimeoutModifier falls through to the else branch and returns 1 — the same as Release.

Note: SlowRuntimeTimeoutModifier's own doc comment says "For use as needed on tests that time out when run on a Debug or Checked runtime", but the implementation doesn't account for Checked on non-RISC-V. That appears to be a pre-existing gap in SlowRuntimeTimeoutModifier itself.

Suggested fix — revert to @adamsitnik's first suggestion which correctly handled this:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore))

Or, if SlowRuntimeTimeoutModifier is preferred, fix it first to return > 1 for Checked runtimes (e.g., return IsDebugRuntime ? 5 : IsCheckedRuntime ? 2 : 1;).

(This finding was independently confirmed by both Claude Opus 4.6 and GPT-5.4.)

[ConditionalTheory] without parameters — Correct

Changing from [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] to bare [ConditionalTheory] is correct:

  • The old attribute skipped all test cases (100, 64K, 127K) on 32-bit, which was unnecessarily restrictive — those small arrays work fine on 32-bit.
  • The 32-bit guard is now applied only to the 2 GB case via the in-method check.
  • [ConditionalTheory] (vs plain [Theory]) is required for SkipTestException to be reported as a skip rather than a failure, as @danmoseley correctly identified.

✅ Smaller test cases now run on all platforms — Good improvement

The smaller InlineData cases (100, 64_001, 127_000) are no longer gated by #if or 64-bit conditions, expanding test coverage.

Note

This review was generated by Copilot (Claude Opus 4.6), with a parallel review from GPT-5.4.

Generated by Code Review for issue #126006 ·

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot address feedback

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

… of any bitness
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/558ab2d2-fd9c-4914-a0aa-7032325fc90c
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
auto-merge was automatically disabled March 26, 2026 15:37

Head branch was pushed to by a user without write access

CopilotAI commented Mar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

Fixed in ff355fb. SlowRuntimeTimeoutModifier returns 1 for checked builds (it only returns >1 for debug), so it didn't catch the timeout case. Switched to !PlatformDetection.IsReleaseRuntime which correctly skips on checked and debug runtimes of any bitness.

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimesMar 26, 2026
@danmoseley

Copy link
Copy Markdown
Contributor

/ba-g one issue opened, the others "cffi_backend" already have issue

@danmoseley
danmoseley merged commit 8204c34 into mainMar 27, 2026
86 of 91 checks passed
@danmoseley
danmoseley deleted the copilot/fix-timeout-edge-case-tests branch March 27, 2026 04:36
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 26, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Formats.Nrbftest-enhancementImprovements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr

4 participants

@danmoseley@adamsitnik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes - #126006

Merged
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests
Mar 27, 2026
Merged

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes#126006
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests

Conversation

CopilotAI commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

EdgeCaseTests.CanReadArrayOfAnySize repeatedly times out (~14+ min) on the checked coreclr windows x64 Release CI leg. The 2 GB array case (Array.MaxLength = 2,147,483,591) was previously gated by #if RELEASE && NET, enabling it for Release library builds. However, checked coreclr's extra assertions/validation make processing a 2 GB array prohibitively slow.

Description

Instead of skipping all test cases on non-release runtimes, the fix adds a targeted in-method skip guard for only the 2 GB case. The #if RELEASE && NET preprocessor guard and [ConditionalTheory] with platform conditions are replaced with [ConditionalTheory] (no parameters, to allow SkipTestException to be treated as a skip rather than a failure) and a runtime check inside the method body:

[ConditionalTheory][InlineData(100)][InlineData(64_001)][InlineData(127_000)][InlineData(2147483591)]// Array.MaxLengthpublicvoidCanReadArrayOfAnySize(intlength){if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore)){thrownewSkipTestException("It would take too much time to execute.");}// ...}

The smaller cases (100, 64_001, 127_000) continue to run on all platforms and runtime configurations. The 2 GB case is skipped only when:

  • The process is not 64-bit, or
  • The runtime is not a release runtime (!IsReleaseRuntime, which covers both checked and debug coreclr of any bitness), or
  • The runtime is not .NET Core (i.e. running on .NET Framework)

!PlatformDetection.IsReleaseRuntime is used instead of SlowRuntimeTimeoutModifier != 1 because SlowRuntimeTimeoutModifier returns 1 for checked (non-release, non-debug) builds, meaning it would not correctly skip the 2 GB test case on checked runtimes.

Using [ConditionalTheory] without parameters (instead of plain [Theory]) ensures that SkipTestException is correctly treated as a skip rather than a test failure.

Original prompt

This section details on the original issue you should resolve

<issue_title>System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr</issue_title>
<issue_description>## Build Information
Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Leg Name: Libraries Test Run checked coreclr windows x64 Release

Error Message

{
"ErrorMessage": ["CanReadArrayOfAnySize", "timed out"],
"BuildRetry": false,
"ExcludeConsoleLog": false
}

Description

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out after 14+ minutes on the checked coreclr windows x64 Release leg. The test runs as a long-running test until the Helix work item is killed.

Helix log: https://helix.dot.net/api/2019-06-17/jobs/c6faf158-7c28-4acb-bec6-ddcc415ba5e5/workitems/System.Formats.Nrbf.Tests/console

Previous occurrence: #110285 (closed December 2024)

Pull request where observed: #125961 (codeflow update, unrelated to the failure)

Known issue validation

Build: 🔎https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Error message validated:[CanReadArrayOfAnySize timed out]
Result validation: ✅ Known issue matched with the provided build.
Validation performed at: 3/23/2026 11:03:41 PM UTC

Report

BuildDefinitionTestPull Request
1348938dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125881
1348882dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125174
1348844dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125129
1348773dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125983
1348315dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125961
1348754dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125981
1348752dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125083
1348706dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125439
1348612dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125973
1348585dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125835
1348578dotnet/runtime[System.Formats.Nrbf.Tests.WorkItemExecution](https://dev.azure.co...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…aseRuntime condition
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/185c955f-9d8a-4286-94bf-f0af9fc6b65f
CopilotAI changed the title [WIP] Fix timeout issue in CanReadArrayOfAnySize testFix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesMar 24, 2026
CopilotAI requested a review from danmoseleyMarch 24, 2026 02:19
@danmoseley
danmoseley marked this pull request as ready for review March 24, 2026 02:29
CopilotAI review requested due to automatic review settings March 24, 2026 02:29

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 updates a resource-intensive System.Formats.Nrbf test to avoid timing out when executed on non-Release runtimes (notably Checked CoreCLR), where additional runtime validation makes the existing workload too slow.

Changes:

  • Add PlatformDetection.IsReleaseRuntime to the ConditionalTheory gating EdgeCaseTests.CanReadArrayOfAnySize, skipping it on Checked/Debug runtimes.

@adamsitnikadamsitnik left a comment

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 apply my suggestion, but make sure it builds and tests are passing before you push the commit

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@adamsitnikadamsitnik added the test-enhancement Improvements of test source code label Mar 25, 2026
…f ConditionalTheory
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/127eb19e-8537-496a-aeb8-6a23a106f8cd
auto-merge was automatically disabled March 25, 2026 14:19

Head branch was pushed to by a user without write access

CopilotAI commented Mar 25, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot apply my suggestion, but make sure it builds and tests are passing before you push the commit

Applied in 6a4f571 — build confirmed clean (0 warnings, 0 errors).

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesMar 25, 2026
CopilotAI requested a review from adamsitnikMarch 25, 2026 14:22

@adamsitnikadamsitnik left a comment

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.

LGTM, thank you @danmoseley !

[Theory] does not handle SkipTestException - it treats the throw as a
test failure. [ConditionalTheory] wires up a custom test invoker that
catches SkipTestException and reports the test as skipped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #126006

Holistic Assessment

Motivation: The PR addresses a real, recurring CI timeout — CanReadArrayOfAnySize with the 2 GB Array.MaxLength case takes 14+ minutes on the checked coreclr windows x64 Release leg, killing the Helix work item. The original #if RELEASE && NET compile-time guard was ineffective because it checks the library build configuration (always Release), not the runtime configuration. The fix is well-motivated.

Approach: Replacing the compile-time guard with a runtime skip is the right approach, and using [ConditionalTheory] (no parameters) to enable SkipTestException handling is correct. However, the specific runtime condition has a gap that prevents it from fixing the reported issue.

Summary: ❌ Needs Changes. The skip condition uses PlatformDetection.SlowRuntimeTimeoutModifier != 1, but SlowRuntimeTimeoutModifier returns 1 for Checked runtimes on x64 — the exact configuration that's timing out. The 2 GB test case will still run and timeout on checked coreclr x64.


Detailed Findings

❌ Skip condition does not fire on checked coreclr x64 — the reported CI leg

The condition at line 71:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||PlatformDetection.SlowRuntimeTimeoutModifier!=1||!PlatformDetection.IsNetCore))

On checked coreclr windows x64 (the failing CI leg):

Sub-conditionValueReason
!Is64BitProcessfalseIt's x64
SlowRuntimeTimeoutModifier != 1falseSee trace below
!IsNetCorefalseIt's .NET Core

Result: true && (false \|\| false \|\| false)false — test is NOT skipped, timeout persists.

SlowRuntimeTimeoutModifier trace for Checked runtime on x64:

if(IsReleaseRuntime)// false — it's Checkedreturn1;if(IsRiscV64Process)// false — it's x64returnIsDebugRuntime?10:2;elsereturnIsDebugRuntime?5:1;// IsDebugRuntime is false (Checked ≠ Debug) → returns 1

The three runtime modes (Debug, Checked, Release) are mutually exclusive via AssemblyConfigurationEquals. A Checked runtime is neither Debug nor Release, so SlowRuntimeTimeoutModifier falls through to the else branch and returns 1 — the same as Release.

Note: SlowRuntimeTimeoutModifier's own doc comment says "For use as needed on tests that time out when run on a Debug or Checked runtime", but the implementation doesn't account for Checked on non-RISC-V. That appears to be a pre-existing gap in SlowRuntimeTimeoutModifier itself.

Suggested fix — revert to @adamsitnik's first suggestion which correctly handled this:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore))

Or, if SlowRuntimeTimeoutModifier is preferred, fix it first to return > 1 for Checked runtimes (e.g., return IsDebugRuntime ? 5 : IsCheckedRuntime ? 2 : 1;).

(This finding was independently confirmed by both Claude Opus 4.6 and GPT-5.4.)

[ConditionalTheory] without parameters — Correct

Changing from [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] to bare [ConditionalTheory] is correct:

  • The old attribute skipped all test cases (100, 64K, 127K) on 32-bit, which was unnecessarily restrictive — those small arrays work fine on 32-bit.
  • The 32-bit guard is now applied only to the 2 GB case via the in-method check.
  • [ConditionalTheory] (vs plain [Theory]) is required for SkipTestException to be reported as a skip rather than a failure, as @danmoseley correctly identified.

✅ Smaller test cases now run on all platforms — Good improvement

The smaller InlineData cases (100, 64_001, 127_000) are no longer gated by #if or 64-bit conditions, expanding test coverage.

Note

This review was generated by Copilot (Claude Opus 4.6), with a parallel review from GPT-5.4.

Generated by Code Review for issue #126006 ·

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot address feedback

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

… of any bitness
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/558ab2d2-fd9c-4914-a0aa-7032325fc90c
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
auto-merge was automatically disabled March 26, 2026 15:37

Head branch was pushed to by a user without write access

CopilotAI commented Mar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

Fixed in ff355fb. SlowRuntimeTimeoutModifier returns 1 for checked builds (it only returns >1 for debug), so it didn't catch the timeout case. Switched to !PlatformDetection.IsReleaseRuntime which correctly skips on checked and debug runtimes of any bitness.

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimesMar 26, 2026
@danmoseley

Copy link
Copy Markdown
Contributor

/ba-g one issue opened, the others "cffi_backend" already have issue

@danmoseley
danmoseley merged commit 8204c34 into mainMar 27, 2026
86 of 91 checks passed
@danmoseley
danmoseley deleted the copilot/fix-timeout-edge-case-tests branch March 27, 2026 04:36
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 26, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Formats.Nrbftest-enhancementImprovements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr

4 participants

@danmoseley@adamsitnik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes - #126006

Merged
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests
Mar 27, 2026
Merged

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes#126006
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests

Conversation

CopilotAI commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

EdgeCaseTests.CanReadArrayOfAnySize repeatedly times out (~14+ min) on the checked coreclr windows x64 Release CI leg. The 2 GB array case (Array.MaxLength = 2,147,483,591) was previously gated by #if RELEASE && NET, enabling it for Release library builds. However, checked coreclr's extra assertions/validation make processing a 2 GB array prohibitively slow.

Description

Instead of skipping all test cases on non-release runtimes, the fix adds a targeted in-method skip guard for only the 2 GB case. The #if RELEASE && NET preprocessor guard and [ConditionalTheory] with platform conditions are replaced with [ConditionalTheory] (no parameters, to allow SkipTestException to be treated as a skip rather than a failure) and a runtime check inside the method body:

[ConditionalTheory][InlineData(100)][InlineData(64_001)][InlineData(127_000)][InlineData(2147483591)]// Array.MaxLengthpublicvoidCanReadArrayOfAnySize(intlength){if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore)){thrownewSkipTestException("It would take too much time to execute.");}// ...}

The smaller cases (100, 64_001, 127_000) continue to run on all platforms and runtime configurations. The 2 GB case is skipped only when:

  • The process is not 64-bit, or
  • The runtime is not a release runtime (!IsReleaseRuntime, which covers both checked and debug coreclr of any bitness), or
  • The runtime is not .NET Core (i.e. running on .NET Framework)

!PlatformDetection.IsReleaseRuntime is used instead of SlowRuntimeTimeoutModifier != 1 because SlowRuntimeTimeoutModifier returns 1 for checked (non-release, non-debug) builds, meaning it would not correctly skip the 2 GB test case on checked runtimes.

Using [ConditionalTheory] without parameters (instead of plain [Theory]) ensures that SkipTestException is correctly treated as a skip rather than a test failure.

Original prompt

This section details on the original issue you should resolve

<issue_title>System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr</issue_title>
<issue_description>## Build Information
Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Leg Name: Libraries Test Run checked coreclr windows x64 Release

Error Message

{
"ErrorMessage": ["CanReadArrayOfAnySize", "timed out"],
"BuildRetry": false,
"ExcludeConsoleLog": false
}

Description

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out after 14+ minutes on the checked coreclr windows x64 Release leg. The test runs as a long-running test until the Helix work item is killed.

Helix log: https://helix.dot.net/api/2019-06-17/jobs/c6faf158-7c28-4acb-bec6-ddcc415ba5e5/workitems/System.Formats.Nrbf.Tests/console

Previous occurrence: #110285 (closed December 2024)

Pull request where observed: #125961 (codeflow update, unrelated to the failure)

Known issue validation

Build: 🔎https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Error message validated:[CanReadArrayOfAnySize timed out]
Result validation: ✅ Known issue matched with the provided build.
Validation performed at: 3/23/2026 11:03:41 PM UTC

Report

BuildDefinitionTestPull Request
1348938dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125881
1348882dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125174
1348844dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125129
1348773dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125983
1348315dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125961
1348754dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125981
1348752dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125083
1348706dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125439
1348612dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125973
1348585dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125835
1348578dotnet/runtime[System.Formats.Nrbf.Tests.WorkItemExecution](https://dev.azure.co...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…aseRuntime condition
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/185c955f-9d8a-4286-94bf-f0af9fc6b65f
CopilotAI changed the title [WIP] Fix timeout issue in CanReadArrayOfAnySize testFix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesMar 24, 2026
CopilotAI requested a review from danmoseleyMarch 24, 2026 02:19
@danmoseley
danmoseley marked this pull request as ready for review March 24, 2026 02:29
CopilotAI review requested due to automatic review settings March 24, 2026 02:29

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 updates a resource-intensive System.Formats.Nrbf test to avoid timing out when executed on non-Release runtimes (notably Checked CoreCLR), where additional runtime validation makes the existing workload too slow.

Changes:

  • Add PlatformDetection.IsReleaseRuntime to the ConditionalTheory gating EdgeCaseTests.CanReadArrayOfAnySize, skipping it on Checked/Debug runtimes.

@adamsitnikadamsitnik left a comment

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 apply my suggestion, but make sure it builds and tests are passing before you push the commit

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@adamsitnikadamsitnik added the test-enhancement Improvements of test source code label Mar 25, 2026
…f ConditionalTheory
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/127eb19e-8537-496a-aeb8-6a23a106f8cd
auto-merge was automatically disabled March 25, 2026 14:19

Head branch was pushed to by a user without write access

CopilotAI commented Mar 25, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot apply my suggestion, but make sure it builds and tests are passing before you push the commit

Applied in 6a4f571 — build confirmed clean (0 warnings, 0 errors).

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesMar 25, 2026
CopilotAI requested a review from adamsitnikMarch 25, 2026 14:22

@adamsitnikadamsitnik left a comment

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.

LGTM, thank you @danmoseley !

[Theory] does not handle SkipTestException - it treats the throw as a
test failure. [ConditionalTheory] wires up a custom test invoker that
catches SkipTestException and reports the test as skipped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #126006

Holistic Assessment

Motivation: The PR addresses a real, recurring CI timeout — CanReadArrayOfAnySize with the 2 GB Array.MaxLength case takes 14+ minutes on the checked coreclr windows x64 Release leg, killing the Helix work item. The original #if RELEASE && NET compile-time guard was ineffective because it checks the library build configuration (always Release), not the runtime configuration. The fix is well-motivated.

Approach: Replacing the compile-time guard with a runtime skip is the right approach, and using [ConditionalTheory] (no parameters) to enable SkipTestException handling is correct. However, the specific runtime condition has a gap that prevents it from fixing the reported issue.

Summary: ❌ Needs Changes. The skip condition uses PlatformDetection.SlowRuntimeTimeoutModifier != 1, but SlowRuntimeTimeoutModifier returns 1 for Checked runtimes on x64 — the exact configuration that's timing out. The 2 GB test case will still run and timeout on checked coreclr x64.


Detailed Findings

❌ Skip condition does not fire on checked coreclr x64 — the reported CI leg

The condition at line 71:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||PlatformDetection.SlowRuntimeTimeoutModifier!=1||!PlatformDetection.IsNetCore))

On checked coreclr windows x64 (the failing CI leg):

Sub-conditionValueReason
!Is64BitProcessfalseIt's x64
SlowRuntimeTimeoutModifier != 1falseSee trace below
!IsNetCorefalseIt's .NET Core

Result: true && (false \|\| false \|\| false)false — test is NOT skipped, timeout persists.

SlowRuntimeTimeoutModifier trace for Checked runtime on x64:

if(IsReleaseRuntime)// false — it's Checkedreturn1;if(IsRiscV64Process)// false — it's x64returnIsDebugRuntime?10:2;elsereturnIsDebugRuntime?5:1;// IsDebugRuntime is false (Checked ≠ Debug) → returns 1

The three runtime modes (Debug, Checked, Release) are mutually exclusive via AssemblyConfigurationEquals. A Checked runtime is neither Debug nor Release, so SlowRuntimeTimeoutModifier falls through to the else branch and returns 1 — the same as Release.

Note: SlowRuntimeTimeoutModifier's own doc comment says "For use as needed on tests that time out when run on a Debug or Checked runtime", but the implementation doesn't account for Checked on non-RISC-V. That appears to be a pre-existing gap in SlowRuntimeTimeoutModifier itself.

Suggested fix — revert to @adamsitnik's first suggestion which correctly handled this:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore))

Or, if SlowRuntimeTimeoutModifier is preferred, fix it first to return > 1 for Checked runtimes (e.g., return IsDebugRuntime ? 5 : IsCheckedRuntime ? 2 : 1;).

(This finding was independently confirmed by both Claude Opus 4.6 and GPT-5.4.)

[ConditionalTheory] without parameters — Correct

Changing from [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] to bare [ConditionalTheory] is correct:

  • The old attribute skipped all test cases (100, 64K, 127K) on 32-bit, which was unnecessarily restrictive — those small arrays work fine on 32-bit.
  • The 32-bit guard is now applied only to the 2 GB case via the in-method check.
  • [ConditionalTheory] (vs plain [Theory]) is required for SkipTestException to be reported as a skip rather than a failure, as @danmoseley correctly identified.

✅ Smaller test cases now run on all platforms — Good improvement

The smaller InlineData cases (100, 64_001, 127_000) are no longer gated by #if or 64-bit conditions, expanding test coverage.

Note

This review was generated by Copilot (Claude Opus 4.6), with a parallel review from GPT-5.4.

Generated by Code Review for issue #126006 ·

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot address feedback

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

… of any bitness
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/558ab2d2-fd9c-4914-a0aa-7032325fc90c
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
auto-merge was automatically disabled March 26, 2026 15:37

Head branch was pushed to by a user without write access

CopilotAI commented Mar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

Fixed in ff355fb. SlowRuntimeTimeoutModifier returns 1 for checked builds (it only returns >1 for debug), so it didn't catch the timeout case. Switched to !PlatformDetection.IsReleaseRuntime which correctly skips on checked and debug runtimes of any bitness.

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimesMar 26, 2026
@danmoseley

Copy link
Copy Markdown
Contributor

/ba-g one issue opened, the others "cffi_backend" already have issue

@danmoseley
danmoseley merged commit 8204c34 into mainMar 27, 2026
86 of 91 checks passed
@danmoseley
danmoseley deleted the copilot/fix-timeout-edge-case-tests branch March 27, 2026 04:36
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 26, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Formats.Nrbftest-enhancementImprovements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr

4 participants

@danmoseley@adamsitnik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes - #126006

Merged
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests
Mar 27, 2026
Merged

Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimes#126006
danmoseley merged 7 commits into
mainfrom
copilot/fix-timeout-edge-case-tests

Conversation

CopilotAI commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

EdgeCaseTests.CanReadArrayOfAnySize repeatedly times out (~14+ min) on the checked coreclr windows x64 Release CI leg. The 2 GB array case (Array.MaxLength = 2,147,483,591) was previously gated by #if RELEASE && NET, enabling it for Release library builds. However, checked coreclr's extra assertions/validation make processing a 2 GB array prohibitively slow.

Description

Instead of skipping all test cases on non-release runtimes, the fix adds a targeted in-method skip guard for only the 2 GB case. The #if RELEASE && NET preprocessor guard and [ConditionalTheory] with platform conditions are replaced with [ConditionalTheory] (no parameters, to allow SkipTestException to be treated as a skip rather than a failure) and a runtime check inside the method body:

[ConditionalTheory][InlineData(100)][InlineData(64_001)][InlineData(127_000)][InlineData(2147483591)]// Array.MaxLengthpublicvoidCanReadArrayOfAnySize(intlength){if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore)){thrownewSkipTestException("It would take too much time to execute.");}// ...}

The smaller cases (100, 64_001, 127_000) continue to run on all platforms and runtime configurations. The 2 GB case is skipped only when:

  • The process is not 64-bit, or
  • The runtime is not a release runtime (!IsReleaseRuntime, which covers both checked and debug coreclr of any bitness), or
  • The runtime is not .NET Core (i.e. running on .NET Framework)

!PlatformDetection.IsReleaseRuntime is used instead of SlowRuntimeTimeoutModifier != 1 because SlowRuntimeTimeoutModifier returns 1 for checked (non-release, non-debug) builds, meaning it would not correctly skip the 2 GB test case on checked runtimes.

Using [ConditionalTheory] without parameters (instead of plain [Theory]) ensures that SkipTestException is correctly treated as a skip rather than a test failure.

Original prompt

This section details on the original issue you should resolve

<issue_title>System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr</issue_title>
<issue_description>## Build Information
Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Leg Name: Libraries Test Run checked coreclr windows x64 Release

Error Message

{
"ErrorMessage": ["CanReadArrayOfAnySize", "timed out"],
"BuildRetry": false,
"ExcludeConsoleLog": false
}

Description

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out after 14+ minutes on the checked coreclr windows x64 Release leg. The test runs as a long-running test until the Helix work item is killed.

Helix log: https://helix.dot.net/api/2019-06-17/jobs/c6faf158-7c28-4acb-bec6-ddcc415ba5e5/workitems/System.Formats.Nrbf.Tests/console

Previous occurrence: #110285 (closed December 2024)

Pull request where observed: #125961 (codeflow update, unrelated to the failure)

Known issue validation

Build: 🔎https://dev.azure.com/dnceng-public/public/_build/results?buildId=1348315
Error message validated:[CanReadArrayOfAnySize timed out]
Result validation: ✅ Known issue matched with the provided build.
Validation performed at: 3/23/2026 11:03:41 PM UTC

Report

BuildDefinitionTestPull Request
1348938dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125881
1348882dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125174
1348844dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125129
1348773dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125983
1348315dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125961
1348754dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125981
1348752dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125083
1348706dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125439
1348612dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125973
1348585dotnet/runtimeSystem.Formats.Nrbf.Tests.WorkItemExecution#125835
1348578dotnet/runtime[System.Formats.Nrbf.Tests.WorkItemExecution](https://dev.azure.co...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…aseRuntime condition
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/185c955f-9d8a-4286-94bf-f0af9fc6b65f
CopilotAI changed the title [WIP] Fix timeout issue in CanReadArrayOfAnySize testFix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesMar 24, 2026
CopilotAI requested a review from danmoseleyMarch 24, 2026 02:19
@danmoseley
danmoseley marked this pull request as ready for review March 24, 2026 02:29
CopilotAI review requested due to automatic review settings March 24, 2026 02:29

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 updates a resource-intensive System.Formats.Nrbf test to avoid timing out when executed on non-Release runtimes (notably Checked CoreCLR), where additional runtime validation makes the existing workload too slow.

Changes:

  • Add PlatformDetection.IsReleaseRuntime to the ConditionalTheory gating EdgeCaseTests.CanReadArrayOfAnySize, skipping it on Checked/Debug runtimes.

@adamsitnikadamsitnik left a comment

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 apply my suggestion, but make sure it builds and tests are passing before you push the commit

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@adamsitnikadamsitnik added the test-enhancement Improvements of test source code label Mar 25, 2026
…f ConditionalTheory
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/127eb19e-8537-496a-aeb8-6a23a106f8cd
auto-merge was automatically disabled March 25, 2026 14:19

Head branch was pushed to by a user without write access

CopilotAI commented Mar 25, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot apply my suggestion, but make sure it builds and tests are passing before you push the commit

Applied in 6a4f571 — build confirmed clean (0 warnings, 0 errors).

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping on non-release runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesMar 25, 2026
CopilotAI requested a review from adamsitnikMarch 25, 2026 14:22

@adamsitnikadamsitnik left a comment

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.

LGTM, thank you @danmoseley !

[Theory] does not handle SkipTestException - it treats the throw as a
test failure. [ConditionalTheory] wires up a custom test invoker that
catches SkipTestException and reports the test as skipped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs Outdated
@github-actions

This comment has been minimized.

Comment threadsrc/libraries/System.Formats.Nrbf/tests/EdgeCaseTests.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #126006

Holistic Assessment

Motivation: The PR addresses a real, recurring CI timeout — CanReadArrayOfAnySize with the 2 GB Array.MaxLength case takes 14+ minutes on the checked coreclr windows x64 Release leg, killing the Helix work item. The original #if RELEASE && NET compile-time guard was ineffective because it checks the library build configuration (always Release), not the runtime configuration. The fix is well-motivated.

Approach: Replacing the compile-time guard with a runtime skip is the right approach, and using [ConditionalTheory] (no parameters) to enable SkipTestException handling is correct. However, the specific runtime condition has a gap that prevents it from fixing the reported issue.

Summary: ❌ Needs Changes. The skip condition uses PlatformDetection.SlowRuntimeTimeoutModifier != 1, but SlowRuntimeTimeoutModifier returns 1 for Checked runtimes on x64 — the exact configuration that's timing out. The 2 GB test case will still run and timeout on checked coreclr x64.


Detailed Findings

❌ Skip condition does not fire on checked coreclr x64 — the reported CI leg

The condition at line 71:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||PlatformDetection.SlowRuntimeTimeoutModifier!=1||!PlatformDetection.IsNetCore))

On checked coreclr windows x64 (the failing CI leg):

Sub-conditionValueReason
!Is64BitProcessfalseIt's x64
SlowRuntimeTimeoutModifier != 1falseSee trace below
!IsNetCorefalseIt's .NET Core

Result: true && (false \|\| false \|\| false)false — test is NOT skipped, timeout persists.

SlowRuntimeTimeoutModifier trace for Checked runtime on x64:

if(IsReleaseRuntime)// false — it's Checkedreturn1;if(IsRiscV64Process)// false — it's x64returnIsDebugRuntime?10:2;elsereturnIsDebugRuntime?5:1;// IsDebugRuntime is false (Checked ≠ Debug) → returns 1

The three runtime modes (Debug, Checked, Release) are mutually exclusive via AssemblyConfigurationEquals. A Checked runtime is neither Debug nor Release, so SlowRuntimeTimeoutModifier falls through to the else branch and returns 1 — the same as Release.

Note: SlowRuntimeTimeoutModifier's own doc comment says "For use as needed on tests that time out when run on a Debug or Checked runtime", but the implementation doesn't account for Checked on non-RISC-V. That appears to be a pre-existing gap in SlowRuntimeTimeoutModifier itself.

Suggested fix — revert to @adamsitnik's first suggestion which correctly handled this:

if(length==2147483591&&(!PlatformDetection.Is64BitProcess||!PlatformDetection.IsReleaseRuntime||!PlatformDetection.IsNetCore))

Or, if SlowRuntimeTimeoutModifier is preferred, fix it first to return > 1 for Checked runtimes (e.g., return IsDebugRuntime ? 5 : IsCheckedRuntime ? 2 : 1;).

(This finding was independently confirmed by both Claude Opus 4.6 and GPT-5.4.)

[ConditionalTheory] without parameters — Correct

Changing from [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] to bare [ConditionalTheory] is correct:

  • The old attribute skipped all test cases (100, 64K, 127K) on 32-bit, which was unnecessarily restrictive — those small arrays work fine on 32-bit.
  • The 32-bit guard is now applied only to the 2 GB case via the in-method check.
  • [ConditionalTheory] (vs plain [Theory]) is required for SkipTestException to be reported as a skip rather than a failure, as @danmoseley correctly identified.

✅ Smaller test cases now run on all platforms — Good improvement

The smaller InlineData cases (100, 64_001, 127_000) are no longer gated by #if or 64-bit conditions, expanding test coverage.

Note

This review was generated by Copilot (Claude Opus 4.6), with a parallel review from GPT-5.4.

Generated by Code Review for issue #126006 ·

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot address feedback

@danmoseley

Copy link
Copy Markdown
Contributor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

… of any bitness
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/558ab2d2-fd9c-4914-a0aa-7032325fc90c
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
auto-merge was automatically disabled March 26, 2026 15:37

Head branch was pushed to by a user without write access

CopilotAI commented Mar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot it needs to skip on all checked runtimes whatever bitness. See text above

Fixed in ff355fb. SlowRuntimeTimeoutModifier returns 1 for checked builds (it only returns >1 for debug), so it didn't catch the timeout case. Switched to !PlatformDetection.IsReleaseRuntime which correctly skips on checked and debug runtimes of any bitness.

CopilotAI changed the title Fix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on slow runtimesFix CanReadArrayOfAnySize timeout on checked coreclr by skipping the 2 GB case on non-release runtimesMar 26, 2026
@danmoseley

Copy link
Copy Markdown
Contributor

/ba-g one issue opened, the others "cffi_backend" already have issue

@danmoseley
danmoseley merged commit 8204c34 into mainMar 27, 2026
86 of 91 checks passed
@danmoseley
danmoseley deleted the copilot/fix-timeout-edge-case-tests branch March 27, 2026 04:36
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 26, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Formats.Nrbftest-enhancementImprovements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Formats.Nrbf.Tests.EdgeCaseTests.CanReadArrayOfAnySize times out on checked coreclr

4 participants

@danmoseley@adamsitnik