-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Refine test discovery and partition during test build with new target #17331
Conversation
test Windows_NT x64 Checked jitincompletehwintrinsic test Windows_NT x86 Checked jitincompletehwintrinsic test Ubuntu x64 Checked jitincompletehwintrinsic |
Changes from this PR allow to slightly increase test build performance (not visible in some of the runs as all jobs including unchanged coreclr runtime builds were running substantially slower than in previous #17161 PR) and enables to automatically generate
My plan for further improvements is to work on:
|
Unfinished OSX and Ubuntu arm CI jobs seems to be hung - they run for more than 14 hours at the moment I am writing this post. |
I think the whole thing needs to be more or less automatic, based on declarative properties in the test projects and/or files in various points in the directory hierarchy. That is, we should not require that people remember to run a script or do anything special when adding a new test (or removing or renaming a test, etc). Because people will forget these extra steps and then tests will not get built and so also won't get run -- and it is very easy to overlook this as tests that don't get run also don't fail. Also these kinds of generated scripts tend to be a frequent source of merge conflicts, unless you are very careful to design the file format with ease of auto-merging in mind. So any sort of generative test project metadata needs to be built and consumed as part of the test build, not as a checked-in input to the test build. As part of this there needs to be some automatic default that kicks in even if the test project or directory is missing whatever new property we use to partition, so that all tests are automatically put into some category and none are left out. It is ok if because of this the execution is not optimally partitioned or can get out of balance over time as new tests are added. That we tolerate and fix when needed. Ironically we caught a problem in the first version of this test partitioning because on arm we do keep a side test list, and so we noticed some tests weren't getting built. This side list getting out of sync is a frequent source of pain, though it is so rarely updated that merge conflicts are not a problem, instead we just miss running tests on arm until somebody notices and regenerates the thing. |
Sure, will implement this action as a separate build target run during every test build as @weshaggard suggested before.
This is implemented already in this version - all existing tests even from new directories will always be included into build. Partitioning is based on number of projects which will be built.
I will try to improve on that, however, there is a trade off between how much we process before build is being run to how long build runs |
|
||
task.Exclude = new ITaskItem[] | ||
{ | ||
new TaskItem(@"..\src\TestWrappers*\**\*.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.
I don't like duplicating these sets of excluded test projects. We should figure out a way to share.
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 am working on more comprehensive fix which will address @AndyAyersMS feedback and will include fix to the above duplication of excluded test projects
Closing stale PRs |
Addresses code review feedback from #17161 fixes partially #17282
FYI @BruceForstall This is general fix for getting all tests included in the build plus some improvements.