-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[Unix|Windows|AllArch] Unify exclusion list and enable runtest.py #19213
Conversation
/cc @dotnet/jit-contrib @RussKeldorph please feel free to include any others. |
@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test |
Based on suggestions, I will change runtest.sh to use runtest.py if on x64. Will work on that change later tonight. |
Please also update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
tests/runtest.py
Outdated
print "Using default location for test_native_bin_location." | ||
test_native_bin_location = os.path.join(os.path.join(coreclr_repo_location, "bin", "obj", "%s.%s.%s" % (host_os, arch, build_type), "tests")) | ||
print "Native bin location: %s" % test_native_bin_location | ||
|
||
if host_os != "Windows_NT": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can merge it with the previous block.
# come from issues.targets. If there is a jit stress or gc stress exclude, | ||
# please add GCStressIncompatible or JitOptimizationSensitive to the test's | ||
# ilproj or csproj. | ||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GCStressIncompatible and JitOptimizationSensitive excludes should probably be called out in docs too since this isn't very discoverable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed
tests/runtest.py
Outdated
# The xunit runner currently relies on tests being built on the same host as the | ||
# target platform. This requires all tests run on linux x64 to be built by the | ||
# same platform and arch. If this is not done, the tests will run correctly; | ||
# however, excpect failures due to incorrect exclusions in the xunit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excpect -> expect
# | ||
# The xunit runner currently relies on tests being built on the same host as the | ||
# target platform. This requires all tests run on linux x64 to be built by the | ||
# same platform and arch. If this is not done, the tests will run correctly; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this cause the official build test run to fail? If I recall, these would build wrappers on windows and run them on linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not necessarily true for helix, which currently gets around this issue by setting an env variable BuiltAgainstPackages.
tests/runtest.py
Outdated
# however, excpect failures due to incorrect exclusions in the xunit | ||
# wrappers setup at build time. | ||
# | ||
# Note that for linux targets the native componants to the tests are still built |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
componants -> components
tests/runtest.py
Outdated
# | ||
# Note that for linux targets the native componants to the tests are still built | ||
# by the product build. This requires all native components to be either copied | ||
# into the Core_Root directory or the test's managed directory. The later is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
later -> latter
tests/runtest.py
Outdated
If you are running tests on a different target than the host that built, the | ||
native tests components must be copied from: | ||
bin/obj/<Host>.<Arch>.<BuildType/tests to the target. If the location is not | ||
standard please pass the -test_native_bin_location flag to the script.""") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comments as above apply to this description.
REM with the appropriate environment set and the correct arch and build_type | ||
REM passed. | ||
REM | ||
REM ============================================================================ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks promising! It'll be great if this makes it easy to repro failures. Could you add some documentation on how this would be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will address this as a separate PR
# with the appropriate environment set and the correct arch and build_type | ||
# passed. | ||
# | ||
# ============================================================================ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you maybe avoid duplicating all of this text? Also, this one should say create_bash_wrapper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did not want to spend too much time on this, and could not think of a super simple way to avoid this. Worth living with it for now.
@@ -159,7 +477,7 @@ def call_msbuild(coreclr_repo_location, | |||
|
|||
Args: | |||
coreclr_repo_location(str) : path to coreclr repo | |||
msbuild_location(str) : path to msbuild | |||
dotnetcli_location(str) : path to the dotnet cli in the tools dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this change!
@@ -65,7 +65,7 @@ | |||
<XunitCommandLine>$(CorerunExecutable) $(XunitConsoleRunner) @(TestAssemblies->'%(Identity)', ' ') $(XunitArgs)</XunitCommandLine> | |||
</PropertyGroup> | |||
|
|||
<Error Condition="$(XunitCommandLine.Length) > 8191" Text="Xunit command line is too long." /> | |||
<Error Condition="$(XunitCommandLine.Length) > 8191 and '$(RunningOnUnix)' != 'true'" Text="Xunit command line is too long." /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this!
Adding no merge for now. With more recent changes to use runtest.py by default in runtest.sh/cmd I will be doing much more in depth testing before merging. Thanks for the reviews! |
Please don't forget to update documentation. |
d9aa922
to
8c89e2b
Compare
Many more changes have been posted. With these changes pri1 tests and gc stress (the longest tests I could think of) now run cleanly on all targets using runtest.py. Runtest.sh has been gutted, it is now a simple wrapper around runtest.py. Runtest.cmd will keep a large amount of existing logic, to be addressed in the future. I have added a little documentation, but there still needs to be more. I have listed the work to still be done below:
|
@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test |
1 similar comment
@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test |
cf111c6
to
a71d711
Compare
Rebased |
@dotnet-bot test this please |
c31bb13
to
f8f97d6
Compare
Rebased |
f8f97d6
to
b39debe
Compare
Squashed and rebased. |
@dotnet-bot test Ubuntu arm Cross Checked r2r Build and Test |
@BruceForstall ptal |
fb55fdd
to
c8897b7
Compare
@dotnet-bot test Ubuntu arm Cross Checked r2r Build and Test |
c8897b7
to
9ec1188
Compare
@dotnet-bot test Ubuntu arm Cross Checked r2r Build and Test |
9ec1188
to
25cdfe9
Compare
@dotnet-bot test Ubuntu arm Cross Checked r2r Build and Test |
25cdfe9
to
0fac8f1
Compare
@dotnet-bot test Ubuntu arm Cross Checked r2r Build and Test |
0fac8f1
to
b317c71
Compare
@dotnet-bot test Ubuntu arm Cross Checked r2r Build and Test |
1 similar comment
@dotnet-bot test Ubuntu arm Cross Checked r2r Build and Test |
Change build-test.sh to always build the xunit wrappers. Before it would drop a token and check the existence of the token. Unify x64 linux/OSX/Windows excludes into one file, issues.targets. I have added different locations in the file which show where to put excludes. Remove all target specific aspects of issues.targets, all tests are excluded now via wildcard, this allows expanding to .cmd and .sh based on the build platform Unify path separators to forward slash(/) in issues.targets to support both platforms Clean up issues.targets by removing long standing exclude tests, specifically tests that have been excluded due to missing features like rva_statics Add DisableProjectBuild to tests which have been removed from issues.targets Conditionally add DisableProjectBuild to tests which have been marked as unsupported on unix. This is mostly a port of the unsupportedOnUnix.txt list. Instead of excluding the tests, unix will simply not build them. Note that this refcount dependency on running tests on the same platform as where they were built All exclusions ported to issues.targets for linux targets. Expand runtest.py, this includes simple issues that made it past the original CR. In addition it adds more optional features to help with inner loop dev work such as: creating a repro folder under bin/repro/.. which sets up the env and calls the failing test. In addition a launch.json will now be created under bin/repro/.. which can be used to easily debug using vscode. More logging, such as printing failures, longest running tests ect. Initial excludes ported for arm64 windows Arm64 linux, armhf unix excludes and enables running runtest.sh for these targets. arm64 windows and arm32 windows excludes and enables running runtest.cmd on arm64 targets init-tools.sh changes to pull armhf and aarch64 dotnetcli init-tools.cmd changes to pull x86 packages for dotnetcli for arm64 windows runtest.cmd for almost all scenarios will call runtest.py runtest.sh for almsot all scenarios will call runtest.py Removes all logic for running tests using runtest.sh
b317c71
to
3fe2153
Compare
At this point outerloops jobs have been confirmed to work as expected. Running the innerloop jobs once more with a comment change. Then I think it is good to merge. Will check over the outerloop jobs this weekend to see if anything has gone wrong. |
@dotnet-bot test Windows_NT x64 Release CoreFX Tests |
Yay merged! |
…lix queue Port (partially) the following changes: * Build xunit wrappers the same way on windows and unix (#18695) * Update vc-runtime package to support ARM and ARM64 with current builds (#19265) * build-test - fix TestWrapper CS warnings (#19180) * Clean up build.cmd/build-test.cmd/runtest.cmd (#19291) * Use runtest.py to run tests for all platforms (#19213) * Updating runtest.py so that it works with Python 3 (#19768) * Fix build-test.sh wrapper build (#19779) * Move ARM64 Windows boxen to be Helix-provisioned (#20204) * Runtest.py on Windows Arm(64) (#20227) * Use runtest.cmd for arm(64) windows testing (#20301) * Do not restore or initialize buildtools for x86/arm64 (#20370) * Add hack for arm64/x86 to skip build tools restore (#20390) Update Xunit to 2.4.1 prerelease version Use the latest version (dbf0bf1) of tests/runtest.py
…lix queue Port (partially) the following changes: * Build xunit wrappers the same way on windows and unix (#18695) * Work around cmd command length limit in xunit Exec task (#19095) * Update vc-runtime package to support ARM and ARM64 with current builds (#19265) * build-test - fix TestWrapper CS warnings (#19180) * Clean up build.cmd/build-test.cmd/runtest.cmd (#19291) * Use runtest.py to run tests for all platforms (#19213) * Updating runtest.py so that it works with Python 3 (#19768) * Fix build-test.sh wrapper build (#19779) * Move ARM64 Windows boxen to be Helix-provisioned (#20204) * Runtest.py on Windows Arm(64) (#20227) * Use runtest.cmd for arm(64) windows testing (#20301) * Do not restore or initialize buildtools for x86/arm64 (#20370) * Add hack for arm64/x86 to skip build tools restore (#20390) Update Xunit to 2.4.1 prerelease version Use the latest version (dbf0bf1) of tests/runtest.py
…lix queue Port (partially) the following changes: * Build xunit wrappers the same way on windows and unix (#18695) * Work around cmd command length limit in xunit Exec task (#19095) * Update vc-runtime package to support ARM and ARM64 with current builds (#19265) * build-test - fix TestWrapper CS warnings (#19180) * Clean up build.cmd/build-test.cmd/runtest.cmd (#19291) * Use runtest.py to run tests for all platforms (#19213) * Updating runtest.py so that it works with Python 3 (#19768) * Fix build-test.sh wrapper build (#19779) * Move ARM64 Windows boxen to be Helix-provisioned (#20204) * Runtest.py on Windows Arm(64) (#20227) * Use runtest.cmd for arm(64) windows testing (#20301) * Do not restore or initialize buildtools for x86/arm64 (#20370) * Add hack for arm64/x86 to skip build tools restore (#20390) Update Xunit to 2.4.1 prerelease version Use the latest version (dbf0bf1) of tests/runtest.py
…lix queue Port (partially) the following changes: * Build xunit wrappers the same way on windows and unix (#18695) * Work around cmd command length limit in xunit Exec task (#19095) * Update vc-runtime package to support ARM and ARM64 with current builds (#19265) * build-test - fix TestWrapper CS warnings (#19180) * Clean up build.cmd/build-test.cmd/runtest.cmd (#19291) * Use runtest.py to run tests for all platforms (#19213) * Updating runtest.py so that it works with Python 3 (#19768) * Fix build-test.sh wrapper build (#19779) * Move ARM64 Windows boxen to be Helix-provisioned (#20204) * Runtest.py on Windows Arm(64) (#20227) * Use runtest.cmd for arm(64) windows testing (#20301) * Do not restore or initialize buildtools for x86/arm64 (#20370) * Add hack for arm64/x86 to skip build tools restore (#20390) Update Xunit to 2.4.1 prerelease version Use the latest version (dbf0bf1) of tests/runtest.py
The change has a few different pieces. I have kept the commit history in an effort to potentially help review the change. The change includes, but is not limited to:
Note that this refcount dependency on running tests on the same platform as where they were builtWhat this PR does not include:
Arm and arm64, both windows and unix, support for runtest.py. This is currently blocked on tools acquisition using init-tools.cmd/init-tools.sh, these scripts will currently pull down the x64 tools and we are unable to run msbuild on those platforms.Notes
In order to support different return codes used by runtest.sh and the xunit console runner. I have conditionally included different return codes for the test bash wrappers. This means that for our ci jobs which build bash targets on windows, unix testing will continue to succeed using runtest.sh. However, this will break all tests run using runtest.sh using a test build that is built on unix. Basically runtest.sh will be deprecated with this change, and runtest.py will be the only supported use case.With this change, only the msbuild xunit console runner will be supported to run tests. Runtest.sh will now call runtest.py, which is a wrapper for runtest.proj, which is a wrapper (keeps going I know...) for calling the xunit console runner.