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.

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] Automatically retry failed MSBuild/emulator tests. #7997

Merged
merged 1 commit into from
May 3, 2023

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Apr 27, 2023

Previously, we have used retryCountOnTaskFailure to retry some of our flaky test suites. However this is not a good solution for our MSBuild and emulator tests for 2 reasons:

  • Running each suite takes 30-60 minutes, so it would take a long time to retry the entire suite.
  • The suites are very flaky, and rerunning the entire suite would likely result in test(s) that were successful on the first run to fail on the second.

We need a solution that only retries the specific test(s) that failed. Unfortunately nothing like that exists, so we'll once again have to roll our own solution, and we can reuse dotnet-test-slicer for this.

This involves some trickery to make everything show up correctly in the AzDO UI:

  • Run the initial test suite
    • Run in a Powershell so we can ignore any test failures that would move the pipeline into SucceededWithIssues state
    • Do not automatically publish the test results, as they would contain test failures
  • Run dotnet-test-slicer retry
    • This creates a new .runsettings file that only contains the failed tests so dotnet test can run only them
    • It also rewrites the test results file of the initial run and removes any failed tests from so it can be published to AzDO
  • Publish the successful tests from the first run to AzDO
  • Run the retried tests
    • This time we can use normal dotnet, and any test failures can be automatically reported to AzDO

Affected Test Suites:

  • MSBuild Emulator Tests

@jpobst jpobst force-pushed the retry-failed-tests branch 5 times, most recently from 55aef28 to 0d8b820 Compare April 28, 2023 14:35
@jpobst jpobst marked this pull request as ready for review May 1, 2023 18:14
@@ -2,22 +2,55 @@ parameters:
testAssembly: # NUnit test assembly to run
testFilter: # Filter used to select tests (NUnit test selection language, not dotnet test filter language)
testRunTitle: # Title of the test run
testResultsTitle: # Title used to construct test results file name
retryFailedTests: true # Retry failed tests once
Copy link
Member

@pjcollins pjcollins May 1, 2023

Choose a reason for hiding this comment

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

General thought but might it be better to add the [Retry] attribute to tests that we see failing somewhat regularly? NUnit does provide some built in support for this that we are already using in a handful of places. This would also give us a better understanding of the tests that do fail regularly so that we could try to improve them at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I agree we should be trying to improve our test reliability, it does not seem like something we have been dedicating time to or are likely to start dedicating time to.

We have this data today: https://devdiv.visualstudio.com/DevDiv/_test/analytics?definitionId=11410&contextType=build, but it doesn't appear to be localized to a few tests. (550 unique tests failed over the past 14 days.) However, this may point to an issue with the suite(s) themselves that could be investigated.

It is good to point out that this data will not be available after this PR (unless the test fails on retry), as we will no longer be reporting initial failures to AzDO so that we have a green build. If we would like to continue having this data, a good compromise might be to perform automatic retries on PR builds but not for main builds.

I feel like the large number of unique test failures makes [Retry] a poor option. I looked at applying it at the class or assembly level, but that isn't supported. It can only be placed on individual tests.

Grendel and I also discovered that [Retry] is not available in NUnitLite which is used by the on-device tests.

Copy link
Member

@pjcollins pjcollins May 2, 2023

Choose a reason for hiding this comment

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

Breaking this down a bit further you can sort by test file, and you can see the vast majority are coming from Designer tests (though I'm not sure what branch).

Other than that, we clearly have a few core culprits under a ~90% pass rate:

image

image

Maybe it would be best to only enable this for the emulator test jobs for now, as that is where most issues seem to be? This seems like a good interim solution, at least until we can find a machine pool that will provide a more stable emulator/device for us.

I believe most of our test related CI headaches stem from using an emulator in nested virtualization, and could be resolved by investigating different test execution environments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Set retryFailedTests: false for the regular MSBuild job, so it will only run on emulator tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe most of our test related CI headaches stem from using an emulator in nested virtualization, and could be resolved by investigating different test execution environments.

I also wonder if we've ever considered we could have a bug in our code but we just blame the emulator rather than investigating. 🤔

retry `
--trx="$(Agent.TempDirectory)" `
--outfile="${{ parameters.testAssembly }}.runsettings"
displayName: Look for failed tests
Copy link
Member

Choose a reason for hiding this comment

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

Do you have a link to the logic for this retry command for reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jpobst jpobst merged commit d00b779 into main May 3, 2023
@jpobst jpobst deleted the retry-failed-tests branch May 3, 2023 16:57
grendello added a commit to grendello/xamarin-android that referenced this pull request May 4, 2023
* main:
  Bump to dotnet/installer@0ce891843a 8.0.100-preview.5.23228.7 (dotnet#7994)
  [ci] Automatically retry failed emulator tests. (dotnet#7997)
  [xaprepare] Combine 'AndroidTestDependencies' and 'EmulatorTestDependencies' scenarios. (dotnet#8006)
  [xaprepare] Provision 'platform-33_r02' (dotnet#8004)
  [Xamarin.Android.Build.Tasks] Add AndroidEnableRestrictToAttributes (dotnet#7990)
  [ci] Remove plots-to-appinsights. (dotnet#8002)
  [Xamarin.Android.Build.Tasks] remove NuGet dependencies (dotnet#8000)
  Bump to 33.0.56 $(AndroidNet7Version) (dotnet#7998)
  $(AndroidPackVersionSuffix)=preview.5; net8 is 34.0.0-preview.5 (dotnet#7996)
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants