Skip to content

Comments

Remove RequiresProcessIsolation from HFA tests#111406

Merged
MichalStrehovsky merged 1 commit intodotnet:mainfrom
MichalStrehovsky:hfatests
Jan 14, 2025
Merged

Remove RequiresProcessIsolation from HFA tests#111406
MichalStrehovsky merged 1 commit intodotnet:mainfrom
MichalStrehovsky:hfatests

Conversation

@MichalStrehovsky
Copy link
Member

@MichalStrehovsky MichalStrehovsky commented Jan 14, 2025

Tests using CMakeProjectReference seem to work just fine with the merged runner.

Before (CoreCLR, checked, Ryzen 9700X):

Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
     47.303 |   112 |    112 |      0 |       0 | JIT.jit64.jit64_1

After:

Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
      2.694 |   112 |    112 |      0 |       0 | JIT.jit64.jit64_1

Cc @dotnet/jit-contrib @jkoritzinsky

@MichalStrehovsky
Remove RequiresProcessIsolation from HFA tests
45094ab 
Tests using `CMakeProjectReference` seem to work just fine with the merged runner.

Before (CoreCLR, checked, Ryzen 9700X):

```
Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
     47.303 |   112 |    112 |      0 |       0 | JIT.jit64.jit64_1
```

After:

```
Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
      2.694 |   112 |    112 |      0 |       0 | JIT.jit64.jit64_1
```
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 14, 2025
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jan 14, 2025

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

EgorBo
EgorBo approved these changes Jan 14, 2025
View reviewed changes
Copy link
Member

@EgorBo EgorBo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I assume none of the projects define custom ennvars in csproj? I remember those required process isolation too (the tests still pass without the envvars, but may just stop being useful)

This was referenced Jan 14, 2025
Open
Open
@MichalStrehovsky
Copy link
Member Author

MichalStrehovsky commented Jan 14, 2025

I assume none of the projects define custom ennvars in csproj? I remember those required process isolation too (the tests still pass without the envvars, but may just stop being useful)

Nope, these all look the same (a couple project references and settings related to opt/debug info and that's it).

@MichalStrehovsky MichalStrehovsky merged commit cea112c into dotnet:main Jan 14, 2025
68 of 70 checks passed
@MichalStrehovsky MichalStrehovsky deleted the hfatests branch January 14, 2025 22:45
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this pull request Jan 28, 2025
@MichalStrehovsky
Drop RequiresProcessIsolation on mcc tests
12c3988 
As per dotnet#111406, `RequiresProcessIsolation` is not needed just because of `CMakeProjectReference`. The tests also have `CLRTestTargetUnsupported`, but all of them do, so I just moved it to the merged runner.

(Note, mcc_i00 didn't have CLRTestTargetUnsupported to make it Windows only, but instead we had it in issues.targets as `needs triage`, so deleting that.)

Before (CoreCLR checked, Microsoft DevBox):

```
Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
     74.311 |    56 |     56 |      0 |       0 | JIT.jit64.jit64_2
```

After:

```
Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
      2.088 |    56 |     56 |      0 |       0 | JIT.jit64.jit64_2
```
@MichalStrehovsky MichalStrehovsky mentioned this pull request Jan 28, 2025
Merged
MichalStrehovsky added a commit that referenced this pull request Jan 28, 2025
@MichalStrehovsky
Drop RequiresProcessIsolation on mcc tests (#111887)
54f41ac 
As per #111406, `RequiresProcessIsolation` is not needed just because of `CMakeProjectReference`. The tests also have `CLRTestTargetUnsupported`, but all of them do, so I just moved it to the merged runner.

(Note, mcc_i00 didn't have CLRTestTargetUnsupported to make it Windows only, but instead we had it in issues.targets as `needs triage`, so deleting that.)

Before (CoreCLR checked, Microsoft DevBox):

```
Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
     74.311 |    56 |     56 |      0 |       0 | JIT.jit64.jit64_2
```

After:

```
Time [secs] | Total | Passed | Failed | Skipped | Assembly Execution Summary
============================================================================
      2.088 |    56 |     56 |      0 |       0 | JIT.jit64.jit64_2
```
@MichalStrehovsky MichalStrehovsky mentioned this pull request Feb 3, 2025
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Feb 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@EgorBo EgorBo EgorBo approved these changes

@jkoritzinsky jkoritzinsky jkoritzinsky approved these changes

Assignees

@MichalStrehovsky MichalStrehovsky

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MichalStrehovsky @EgorBo @jkoritzinsky