Disable runtimeasyncapis in ReadyToRun tests - #133261
Disable runtimeasyncapis in ReadyToRun tests#133261jtschuster with Copilot wants to merge 3 commits into
Conversation
|
Azure Pipelines: 16 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
github-actions
Bot
added
the
area-Infrastructure-coreclr
Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
|
/azp run runtime-coreclr crossgen2 |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟢 Approval recommended
The change is a minimal, targeted test-project setting that uses established harness behavior to skip only R2R runs without affecting normal execution.
Pull request overview
Disables the runtimeasyncapis profiler test when running under ReadyToRun (Crossgen2) by marking the test project as R2R-incompatible, while keeping the test runnable in non-R2R configurations.
Changes:
- Set
<R2RIncompatible>true</R2RIncompatible>forsrc/tests/profiler/runtimeasyncapis/runtimeasyncapis.csprojso R2R runs skip execution via the existing CrossGen test harness logic. - Preserve existing non-R2R behavior (no change to normal execution path).
File summaries
| File | Description |
|---|---|
| src/tests/profiler/runtimeasyncapis/runtimeasyncapis.csproj | Marks the test as R2R-incompatible so ReadyToRun configurations skip executing it, while leaving non-R2R runs intact. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
jtschuster
added
area-Diagnostics-coreclr
area-ReadyToRun
and removed
area-Infrastructure-coreclr
GitHub Actions
Inactive
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
|
Tagging subscribers to this area: @dotnet/crossgen-contrib |
GitHub Actions
Inactive
| <OutputType>exe</OutputType> | ||
| <Optimize>true</Optimize> | ||
| <Features>$(Features);runtime-async=on</Features> | ||
| <R2RIncompatible>true</R2RIncompatible> |
There was a problem hiding this comment.
Is this by-design or is this just disabling the test for now since it is failing frequently?
If this is by-design, there should be a comment with explanation.
If this is a workaround, there should be a link to ActiveIssue that tracks fixing the bug.
| <OutputType>exe</OutputType> | ||
| <Optimize>true</Optimize> | ||
| <Features>$(Features);runtime-async=on</Features> | ||
| <R2RIncompatible>true</R2RIncompatible> |
There was a problem hiding this comment.
#132810 has hits in runtime-coreclr outerloop and runtime-interpreter too. I do not think this is R2R-speciifc issue. Disabling the test for R2R will reduce the failure rate, but I expect the test is still going to fail frequently.
There was a problem hiding this comment.
This test checks for needs JIT hooks to run in order to pass, so it makes sense this would fail in interpreter and other outerloop tests too. The disablement should be broader.
There was a problem hiding this comment.
Do you mean JITCompilationStarted/JITCompilationFinished callbacks?
The profiler test should be able to use JITCachedFunctionSearchStarted/JITCachedFunctionSearchFinished to intercept the code that is originating from R2R image. It would be preferable to fix the test to maintain coverage across different configurations rather than disabling the test.
|
@copilot this test expects JIT hooks to run in order to pass. This should be disabled for all platforms that don't have fully JITed code at runtime. Modify the skip logic to skip platforms/form factors that aren't fully jitted and add a comment describing why this test is disabled in those cases. |
Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Updated the skip logic to cover both interpreter and ReadyToRun execution modes, with an explanatory ActiveIssue reference for #132810. Normal execution remains unchanged in fully JITed runs. d072f69 |
There was a problem hiding this comment.
🟡 Changes recommended
The PR metadata should be aligned with the fact that the change also skips interpreter runs, and the new explanatory comment should be clarified to avoid confusion.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
| <!-- | ||
| This profiler test relies on JIT hooks firing, so it does not validate the same behavior when | ||
| the runtime is executed in interpreter or ReadyToRun modes. Skip those non-fully-JITed paths | ||
| until ActiveIssue https://github.com/dotnet/runtime/issues/132810 is fixed. | ||
| --> |
| <InterpreterIncompatible>true</InterpreterIncompatible> | ||
| <R2RIncompatible>true</R2RIncompatible> |
The
runtimeasyncapisprofiler test repeatedly fails across ReadyToRun configurations because the expected profiler success marker is missing.R2RIncompatible.