Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unify corerun test runner #49529

Merged
merged 9 commits into from
Mar 17, 2021
Merged

Unify corerun test runner #49529

merged 9 commits into from
Mar 17, 2021

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Mar 12, 2021 •
edited
Loading

Rewrite corerun so that the same source works for all platforms instead of having a Windows and non-Windows implementation.

Fixes #35814
Fixes #7509
Fixes #5116 - 'wait for debugger' attach is supported on macOS and systems with procfs.

The -v verbose flag has been removed since it didn't offer a lot of insight. New logging has been added instead. corerun now dumps out all the details passed to CoreCLR when it fails to initialize or if the execute call fails. See below for an example of output.

BEGIN: coreclr_execute_assembly failed - Error: 0xffffffff
Exe path: D:\runtime\artifacts\tests\coreclr\windows.x64.Debug\Tests\Core_Root\corerun.exe
Properties:
    TRUSTED_PLATFORM_ASSEMBLIES = <The computed TPA is displayed. It has been removed here for brevity.>
    APP_PATHS = D:\runtime\artifacts\tests\coreclr\windows.x64.Debug\GC\API\WeakReference\multipleWRs_1\
    APP_NI_PATHS = D:\runtime\artifacts\tests\coreclr\windows.x64.Debug\GC\API\WeakReference\multipleWRs_1\NI\;D:\runtime\artifacts\tests\coreclr\windows.x64.Debug\GC\API\WeakReference\multipleWRs_1\
    NATIVE_DLL_SEARCH_DIRECTORIES = D:\runtime\artifacts\tests\coreclr\windows.x64.Debug\GC\API\WeakReference\multipleWRs_1\;D:\runtime\artifacts\tests\coreclr\windows.x64.Debug\Tests\Core_Root\;
    System.GC.Server = false
    System.GC.Concurrent = true
    System.Globalization.Invariant = false
Managed assembly: D:\runtime\artifacts\tests\coreclr\windows.x64.Debug\GC\API\WeakReference\multipleWRs_1\multipleWRs_1.dll
Arguments (2): 10000 track
END: coreclr_execute_assembly failed - Error: 0xffffffff

@danmoseley
Copy link
Member

It doesn't already?

@AaronRobinsonMSFT
Copy link
Member Author

It doesn't already?

@danmoseley It does, but there are two implementations - corerun and unixcorerun. This PR combines them.

This was referenced Mar 12, 2021
Closed
Closed
Closed
Closed
@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review March 13, 2021 23:46
@AaronRobinsonMSFT
Copy link
Member Author

/cc @vitek-karas @jkoritzinsky @elinor-fung @janvorli

@runfoapp runfoapp bot mentioned this pull request Mar 15, 2021
Closed
vitek-karas
vitek-karas approved these changes Mar 15, 2021
View reviewed changes
Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

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

LGTM, but somebody with build system experience should probably take a second look

@AaronRobinsonMSFT
Remove unused files.
33369e8
@AaronRobinsonMSFT
Rewrite corerun so that it runs on all platforms.
d9b46c2 
Remove the unixcorerun implementation but fold in
those its features.

Attach debugger is supported on Windows and macOS.
CORE_ROOT can now be set the same way on all platforms:
  --clr-path, CORE_ROOT, corerun dir.
@AaronRobinsonMSFT
Missed forward declare of overload.
6f6a9da
@AaronRobinsonMSFT
Dump the computed configuration for runtime if a failure to
05faa28 
  initialize or execute the entry assembly occurs.
Remove -v flag.
@AaronRobinsonMSFT
Add debugger attach support for systems with procfs.
cb759b2
@AaronRobinsonMSFT
Simplfy CMake for including the coreclr/hosts project.
2521534
@AaronRobinsonMSFT
Revert "Simplfy CMake for including the coreclr/hosts project."
053cf2b 
This reverts commit 51df3bf8e962624f4fb26f0e46cf412d4543a6da.
@AaronRobinsonMSFT
Add comments to the self-testing method.
de2c659
@AaronRobinsonMSFT
Copy link
Member Author

Anyone else have feedback here?

jkotas
jkotas reviewed Mar 16, 2021
View reviewed changes
src/coreclr/hosts/corerun/corerun.cpp
//
const char* property_keys[] = {
const char* propertyKeys[] =
{
"TRUSTED_PLATFORM_ASSEMBLIES",
"APP_PATHS",
"APP_NI_PATHS",
Copy link
Member

Choose a reason for hiding this comment

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

I think APP_NI_PATHS can be deleted. Nothing should depend on it anymore.

Copy link
Member Author

@AaronRobinsonMSFT AaronRobinsonMSFT Mar 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

You would think that, but unfortunately we do have tests that rely on it. That is something I was going to cleanup after this is in.

AaronRobinsonMSFT added a commit to dotnet/docs that referenced this pull request Mar 16, 2021
@AaronRobinsonMSFT
Update netcore-hosting.md
0362314 
Updates based on dotnet/runtime#49529.
@AaronRobinsonMSFT AaronRobinsonMSFT mentioned this pull request Mar 16, 2021
Merged
@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 7b500a4 into dotnet:main Mar 17, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the unify_corerun branch March 17, 2021 05:56
@AaronRobinsonMSFT AaronRobinsonMSFT mentioned this pull request Mar 21, 2021
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Apr 20, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jkotas jkotas jkotas approved these changes

@vitek-karas vitek-karas vitek-karas approved these changes

Assignees
No one assigned
Labels
area-VM-coreclr
Projects
None yet
Milestone
6.0.0
5 participants
@AaronRobinsonMSFT @danmoseley @jkotas @vitek-karas @karelz