Skip to content

[ci] Automatically retry failed MSBuild/emulator tests. #7997

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 3, 2023
+44 −12
Merged

[ci] Automatically retry failed MSBuild/emulator tests. #7997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build-tools/automation/yaml-templates/install-dotnet-test-slicer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
parameters:
version: '0.1.0-alpha2'
version: '0.1.0-alpha5'
condition: succeeded()
continueOnError: true

Expand Down
2 changes: 1 addition & 1 deletion build-tools/automation/yaml-templates/run-msbuild-tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ jobs:
testAssembly: $(System.DefaultWorkingDirectory)/bin/Test$(XA.Build.Configuration)/$(DotNetStableTargetFramework)/Xamarin.Android.Build.Tests.dll
testFilter: ${{ parameters.testFilter }} $(ExcludedNUnitCategories)
testRunTitle: Xamarin.Android.Build.Tests - ${{ parameters.testOS }}
testResultsTitle: TestResult-MSBuildTests-${{ parameters.jobName }}
retryFailedTests: false

- template: upload-results.yaml
parameters:
Expand Down
49 changes: 41 additions & 8 deletions build-tools/automation/yaml-templates/run-sliced-nunit-tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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 •
edited
Loading

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. 🤔


steps:
- pwsh: |
dotnet-test-slicer `
slice `
--test-assembly="${{ parameters.testAssembly }}" `
--test-filter="${{ parameters.testFilter }}" `
--slice-number=$(System.JobPositionInPhase) `
--total-slices=$(System.TotalJobsInPhase) `
--outfile="${{ parameters.testAssembly }}.runsettings"
displayName: Slice unit tests

- template: run-nunit-tests.yaml
parameters:
useDotNet: true
testRunTitle: ${{ parameters.testRunTitle }}-$(System.JobPositionInPhase)
testAssembly: ${{ parameters.testAssembly }}
dotNetTestExtraArgs: --settings "${{ parameters.testAssembly }}.runsettings"
testResultsFile: ${{ parameters.testResultsTitle }}-$(System.JobPositionInPhase)-$(XA.Build.Configuration).xml
- ${{ if eq(parameters.retryFailedTests, 'false') }}:
# If we aren't using auto-retry logic, then this is just a simple template call
- template: run-nunit-tests.yaml
parameters:
testRunTitle: ${{ parameters.testRunTitle }}-$(System.JobPositionInPhase)
testAssembly: ${{ parameters.testAssembly }}
dotNetTestExtraArgs: --settings "${{ parameters.testAssembly }}.runsettings"

- ${{ if eq(parameters.retryFailedTests, 'true') }}:
# We need a custom dotnet test invocation here that does not trigger a task failure on failed tests
- pwsh: |
dotnet `
test `
${{ parameters.testAssembly }} `
--settings "${{ parameters.testAssembly }}.runsettings" `
--logger trx --results-directory $(Agent.TempDirectory) `
-- NUnit.NumberOfTestWorkers=$(NUnit.NumberOfTestWorkers)
displayName: Run tests
ignoreLASTEXITCODE: true

- pwsh: |
dotnet-test-slicer `
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.

https://github.com/jpobst/dotnet-test-slicer/blob/main/src/dotnet-test-slicer/Program.cs#L122-L164


# dotnet-test-slicer removed the failed tests from our results file, so it's safe to publish it now
- task: PublishTestResults@2
inputs:
testResultsFormat: VSTest
testResultsFiles: $(Agent.TempDirectory)/*.trx
testRunTitle: ${{ parameters.testRunTitle }}-$(System.JobPositionInPhase)

- template: run-nunit-tests.yaml
parameters:
testRunTitle: ${{ parameters.testRunTitle }}-$(System.JobPositionInPhase) (Auto-Retry)
testAssembly: ${{ parameters.testAssembly }}
dotNetTestExtraArgs: --settings "${{ parameters.testAssembly }}.runsettings"
1 change: 0 additions & 1 deletion build-tools/automation/yaml-templates/stage-msbuild-emulator-tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ stages:
testAssembly: $(System.DefaultWorkingDirectory)/bin/Test$(XA.Build.Configuration)/MSBuildDeviceIntegration/$(DotNetStableTargetFramework)/MSBuildDeviceIntegration.dll
testFilter: cat != TimeZoneInfo & cat != Localization $(ExcludedNUnitCategories)
testRunTitle: MSBuildDeviceIntegration On Device - macOS
testResultsTitle: TestResult-MSBuildDeviceIntegration-${{ parameters.job_name }}

- template: start-stop-emulator.yaml
parameters:
Expand Down
2 changes: 1 addition & 1 deletion build-tools/automation/yaml-templates/start-stop-emulator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ parameters:
avdApiLevel: # Device API level, like '30', required if 'specificImage' is 'true'
avdAbi: # Device ABI, like 'x86', required if 'specificImage' is 'true'
avdType: # Device AVD, like 'android-wear', required if 'specificImage' is 'true'
launchTimeoutMin: 15 # Minutes to wait for the emulator to start
launchTimeoutMin: 20 # Minutes to wait for the emulator to start
xaSourcePath: $(System.DefaultWorkingDirectory) # working directory

steps:
Expand Down