-
Notifications
You must be signed in to change notification settings - Fork 538
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
[ci] Use AZDO built-in parallelization strategy. #7804
Conversation
99d3866
to
55f4f8e
Compare
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 was comparing two PR builds:
- This PR: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=7363883&view=ms.vss-test-web.build-test-results-tab
- Random green PR: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=7367450&view=ms.vss-test-web.build-test-results-tab
This PR ran 182,624 tests and the other one has 182,627 tests.
Is it worth checking if we somehow lost 3 tests? How can we tell?
The 3 test difference appears to be in the emulator tests, though I haven't looked through all of them yet: The additional jobs ultimately use more machine time as well, since we still have a somewhat expensive "provisioning" process for each test job that could use some optimization. Not having to tag every test/fixture with a node is definitely a plus though. |
One of those tests ( The other two are the permutations of
Yep, this is something I am planning on looking at next. |
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 feel like we could try this in main for a while and see how it goes? Anyone else have concerns?
a109ce8
to
d98dbd0
Compare
* main: [ci] Use AZDO built-in parallelization strategy. (dotnet#7804) Bump to dotnet/installer@e3ab0b5 8.0.100-preview.2.23123.10 (dotnet#7813) [ci] Run nunit tests with stable .NET version (dotnet#7826) [monodroid] Update p/invoke symbol names from net8 alpha1 (dotnet#7829) Localized file check-in by OneLocBuild (dotnet#7827)
* main: [ci] Use AZDO built-in parallelization strategy. (dotnet#7804) Bump to dotnet/installer@e3ab0b5 8.0.100-preview.2.23123.10 (dotnet#7813) [ci] Run nunit tests with stable .NET version (dotnet#7826) [monodroid] Update p/invoke symbol names from net8 alpha1 (dotnet#7829) Localized file check-in by OneLocBuild (dotnet#7827)
* main: [ci] Use AZDO built-in parallelization strategy. (dotnet#7804) Bump to dotnet/installer@e3ab0b5 8.0.100-preview.2.23123.10 (dotnet#7813) [ci] Run nunit tests with stable .NET version (dotnet#7826) [monodroid] Update p/invoke symbol names from net8 alpha1 (dotnet#7829) Localized file check-in by OneLocBuild (dotnet#7827)
* main: [ci] Use AZDO built-in parallelization strategy. (dotnet#7804) Bump to dotnet/installer@e3ab0b5 8.0.100-preview.2.23123.10 (dotnet#7813) [ci] Run nunit tests with stable .NET version (dotnet#7826) [monodroid] Update p/invoke symbol names from net8 alpha1 (dotnet#7829)
* main: (22 commits) Bump to dotnet/installer@632ddca 8.0.100-preview.3.23128.1 (dotnet#7836) LEGO: Merge pull request 7852 [ci] Reduce overhead for MSBuildIntegration unit test jobs. (dotnet#7832) [ci] Allow dynamic`$(NuGetArtifactName)` values (dotnet#7848) [Xamarin.Android.Build.Tasks] guard `AutoImport.props` against empty values (dotnet#7837) [Mono.Android] Print type & member remapping info (dotnet#7844) [Mono.Android] Tweak AndroidMessageHandler behavior for WCF support (dotnet#7785) LEGO: Merge pull request 7845 Localized file check-in by OneLocBuild Task (dotnet#7842) [ci] Use compliance stage template (dotnet#7818) [build] pass `--skip-sign-check` to `dotnet workload` (dotnet#7840) Replace K4os.Hash.xxHash with System.IO.Hashing (dotnet#7831) $(AndroidPackVersionSuffix)=preview.3; net8 is 34.0.0-preview.3 (dotnet#7839) [Xamarin.Android.Build.Tasks] Remove support for mkbundle (dotnet#7772) [Xamarin.Android.Build.Tasks] `unable to open file as zip archive`? (dotnet#7759) [monodroid] Properly process satellite assemblies (dotnet#7823) Bump to xamarin/java.interop/main@77800dda (dotnet#7824) [ci] Use AZDO built-in parallelization strategy. (dotnet#7804) Bump to dotnet/installer@e3ab0b5 8.0.100-preview.2.23123.10 (dotnet#7813) [ci] Run nunit tests with stable .NET version (dotnet#7826) ...
Bring the AzDO parallelization from #7804 and environment setup improvements from #7832 to `Xamarin.Android.Build.Tests.csproj` based test suites. This includes both the main `MSBuild` stage and the `Smoke Tests` jobs. Increases parallelization of all jobs as many were approaching ~90 minutes. As there is no longer a place in the `MSBuild` stage to run `Xamarin.Android.Tools.Aidl-Tests` tests, it was moved to the `macOS > Tests > APKs .NET` stage. This suite should be fine to only run on Mac and not Windows. (Note this test assembly was also updated to .NET 7. This required moving it from `Xamarin.Android-Tests.sln` which is currently built with Mono which cannot build .NET 7+ projects. It now is built via `Xamarin.Android.sln`.)
Context: #7850 (comment) With the re-parallelization of our test suites (#7804, #7850), combined with the fact that most PR's end up running the full test suite anyways, the separate "Smoke Tests" jobs aren't really saving us much, if any, CI time. We feel we would be better off using those additional agents to run the full test suite faster. This PR removes the separate "Smoke Tests" jobs and runs all tests in the same jobs. Additionally, as the `[Category ("SmokeTests")]` NUnit attribute is now only used by the Windows build stage to run in-tree tests, it has been cut back to the absolute bare minimum, saving ~45 minutes. Note that all the tests will continue to be run against the Windows installer, this only runs fewer tests for the Windows in-tree tests.
Previously, we split our
MSBuildIntegration
unit tests to run across multiple CI test agents. While a huge improvement, there are a few downsides to the approach we went with at the time:[Category ("Node-X")]
in code which is cumbersome and hard to keep updated.As we are at the point where our tests have expanded well past the 1 hour target we are faced with needing to increase our parallelization.
This time we're going to invest in a better fix for the above issues.
First, we can remove the YAML duplication by using AZDO's built-in
parallel
strategy, allowing us to control the number of agents to use with a single number:This leaves us the issue of automatically splitting our tests among an unknown number of test agents. A clever solution is provided in the AZDO docs:
dotnet test --list-tests
to find all the tests$(System.JobPositionInPhase)
and$(System.TotalJobsInPhase)
dotnet test
as a filterUnfortunately there are issues with the provided approach:
dotnet test --list-tests
is not compatible with--filter
so you have to run all tests in the test assembly which is not desirable for us. (Option --list-tests should take into account --filter option sdk#8643)So the approach is good, but we have to do all the work ourselves. Enter dotnet-test-slicer.
This dotnet global tool:
NUnit.Engine
NuGet package to find all tests in a test assembly using the desired filter query..runsettings
file that can be passed todotnet test --settings foo.runsettings
, bypassing command line arguments.Voila! Now we can automatically scale our test agents up or down as needed by changing a single variable in our YAML file.
Limitation: The tests are sliced in a round robin fashion. This can produce uneven results if test durations are uneven. A future optimization would be to store approximate test durations so tests can be load balanced more intelligently.