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

Replace MauiTestUtils with DeviceRunners nuget package #3628

Merged
merged 20 commits into from
Oct 6, 2024
Merged

Conversation

jamescrosswell
Copy link
Collaborator

@jamescrosswell jamescrosswell commented Sep 25, 2024

The device tests use a device runner Test App, which is a very simple MAUI application containing project references to all of our test projects. XHarness can be used to install and launch the test app on iOS and Android physical devices or simulators, run any xUnit tests referenced and then capture both test results and application logs as artefacts.

The mechanism used to discover and run xUnit tests within the test app is called Device Runners. Originally Microsoft made the Device Runners available as some sample code in thedotnet/maui repository, which we vendored into (i.e. copied into) the test/MauiTestUtils folder of the sentry/sentry-dotnet repo.

Since then, that code appears to have been turned into a semi-official Device Runners NuGet package, which is being actively maintained.

This PR removes our vendored in code - using a package reference to the most recent version of the NuGet package instead.

Hopefully this will make it easier for us to upgrade the device tests to net8.0 in a separate PR.

Notes to Reviewers

It looks like a lot to review but mostly it's deleting code:

  • If you collapse the tests/MauiTestUtils directory, all of which is being deleted, it starts to look less overwhelming
  • The test/Sentry.Maui.Device.TestApp/Platforms/Android/SentryInstrumentation.cs class is worth calling out... it's a custom instrumentation class that enables us to intercept launching of the device tests on Android (so we can inject environment variables that have been set on CI - mainly so we can skip certain flaky tests in CI)
  • Most of the other changes are addressing flaky tests

Notes on Flaky Tests

There are a few tests that I had to disable on iOS and some others I had to disable on Android. On Android, it was only when targeting API 31 or 33 (the only two I tried other than 27). When this happens, the error message from Xharness is a bit misleading - it indicates the tests are timing out after 900 seconds.

This may be related to Restrictions on non-SDK interfaces but, if so, I don't understand how.

You can download a CSV describing ~656k API interfaces and the availability of each... so it's possible some of the APIs we're using in our device tests are not supported on API 33. How to know whether we're using any of the blocked/obsolete APIs though??? Maybe that information is in the adb logs if you know how to look for it but our adb logs are about 33k lines so that's a bit like trying to finding a needle in a haystack.

If we could work out what was going on there, we might be able to re-enable the Android tests. I don't have any leads for the iOS tests.

#skip-changelog

@jamescrosswell jamescrosswell changed the title Replaced MauiTestUtils with DeviceRunners nuget package Replace MauiTestUtils with DeviceRunners nuget package Sep 30, 2024
@jamescrosswell jamescrosswell marked this pull request as ready for review October 3, 2024 22:39
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

a bit concerned with the Skip calls. otherwise LGTM

@bruno-garcia
Copy link
Member

Feel free to merge once u're comfortable with maintaining this, from my PoV looks good

@jamescrosswell jamescrosswell merged commit d513b23 into main Oct 6, 2024
23 checks passed
@jamescrosswell jamescrosswell deleted the mattleibow branch October 6, 2024 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants