Skip to content

Conversation

@markples
Copy link
Contributor

@markples markples commented Dec 15, 2022

This is another collection of changes that can be merged independently of actual test merging. It may be easier to look at each commit separately. I have them collected here in order to just do one PR process, but I can split them if needed. Automated changes were done with the ILTransform tool (started by @trylek, my version at https://github.com/markples/utils/tree/for-PR-dotnet-runtime-79697, commit 6324a32f).

Remove test JIT/Regression/VS-ia64-JIT/V1.2-Beta1/b102887 - identical…

  • Tests JIT/Regression/VS-ia64-JIT/V1.2-Beta1/b102887 and JIT/Regression/VS-ia64-JIT/V1.2-M02/b28077 were identical

Dedup class names in GitHub_23199

  • JIT/Regression/JitBlue/GitHub_23199 used #ifdefs to create 32 and 64 bit versions of the test
  • Both versions can run on any host
  • Duplicate the test with different names and no ifdefs
  • Run both versions on all hosts

Merge b399444 into one test that handles both cases

  • JIT/Regression/CLR-x86-JIT/V2.0-Beta2/b399444 used #ifdefs to create two tests
  • Combine both into one test that checks both without #ifdefs
  • Delete unused .xml files

Add RequiresProcessIsolation

  • Automated via ILTransform -prociso
  • Add <RequiresProcessIsolation>true</RequiresProcessIsolation> for any test with CLRTestTargetUnsupported, GCStressIncompatible, UnloadabilityIncompatible, JitOptimizationSensitive, TieringTestIncompatible, HeapVerifyIncompatible, IlasmRoundTripIncompatible, SynthesizedPgoIncompatible, or CrossGenTest properties.
  • Add <RequiresProcessIsolation>true</RequiresProcessIsolation> for any test with CLRTestBashEnvironmentVariable, CLRTestBatchEnvironmentVariable, CLRTestEnvironmentVariable, Content, CMakeProjectReference (not sure about the last two, but they only impact 3 tests and can be examined later)
  • Add <RequiresProcessIsolation>true</RequiresProcessIsolation> for any test that calls Environment.Exit
  • Add comment explaining why RequiresProcessIsolation is set to help with removing them later

ILTransform -public

  • Automated via ILTransform -public
  • Update entry point method (Main for C#, whatever is marked .entrypoint for IL) to be public
  • Update entry point's enclosing type to be public if it exists
  • Wrap IL entry points in a new class if one doesn't exist (e.g., CLR-x86-JIT/V1-M09.5-PDC/b10940/b10940a.il)

Manual fixes for public changes

  • These were all C# cases where Main was previously in a non-public type with other public methods that contained non-public types
  • Fix in all cases is to just make the other method not public

@ghost
Copy link

ghost commented Dec 15, 2022

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: markples
Assignees: markples
Labels:

area-System.Reflection.Metadata

Milestone: -

@markples
Copy link
Contributor Author

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@markples markples added test-enhancement Improvements of test source code area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-System.Reflection.Metadata labels Dec 15, 2022
@markples markples added this to the 8.0.0 milestone Dec 15, 2022
@ghost
Copy link

ghost commented Dec 15, 2022

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

Issue Details

null

Author: markples
Assignees: markples
Labels:

test-enhancement, area-CodeGen-coreclr

Milestone: -

@markples
Copy link
Contributor Author

"Build Installer Build and Test coreclr OSX_x64 Release" failure is a timeout. The entire leg ran incredibly slowly from the beginning (project restore, configuring), so I don't think it is relevant here.

@markples markples marked this pull request as ready for review December 15, 2022 18:49
@markples
Copy link
Contributor Author

Note: I force pushed back to an earlier version of this change without class deduplication. The tests pass with and without, but in doing the next steps I discovered that it appears that all classes, not just the entry point ones, need to be unique. Therefore, I will be reimplementing the dedup logic. The changes here can be merged without dedup - I had simply added those since they build on this PR and (I thought) were ready.

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@markples
Copy link
Contributor Author

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@markples
Copy link
Contributor Author

Failures are timeouts that, as best as I can tell, are intermittent but preexisting.

@markples markples merged commit 1818b9d into dotnet:main Dec 22, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI test-enhancement Improvements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants