-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
Move many interop tests to run in-proc and move many to built in the merged runner itself #94109
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ome "process isolation" entries that were added for transition but are not required.
…ies and allow the interop tests that were only process-isolated due to that restriction to run in proc. Also update exclusions for some projects to use attributes instead of project properties to enable more of them to be run in-proc.
…mblies we build. Also convert more of the tests to be more xunit-like.
jkoritzinsky
added
test-enhancement
Improvements of test source code
area-Interop-coreclr
labels
Oct 27, 2023
ghost
assigned jkoritzinsky
Oct 27, 2023
…dows in the merged runner and don't build the native assets on non-Windows.
trylek
approved these changes
Oct 27, 2023
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.
Awesome cleanup and improvement, thanks Jeremy!
AaronRobinsonMSFT
approved these changes
Oct 27, 2023
…ests running so we see where we actually have failures.
…erge # Conflicts: # src/tests/Interop/PInvoke/IEnumerator/IEnumeratorNative.cpp
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
… into interop-tests-merge
…urrently build native assets on.
Failures are all #95712 |
AaronRobinsonMSFT
approved these changes
Dec 7, 2023
trylek
approved these changes
Dec 7, 2023
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.
Looks great to me, thanks for pulling this off!
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference the native assets from the merged runner assembly itself so we can run more tests in-proc. Move many of the in-proc tests to build in-assembly (any test that isn't testing an assembly-wide flag is eligible for this treatment).
In the process of this. move more tests to use the XUnit attributes on the test methods directly, and fix some Xunit analyzer warnings in the GenericsTest test suite.
I've left the files that are in the merged runner in their existing places in the file tree and left the native builds as-is to limit diffs. In the future, we can follow-up this work by moving the sources around (and possibly combining the native libraries) to a way that we can better understand in this new model.