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

Integration testing for the sentry-native new unwinder #1359

Closed
bruno-garcia opened this issue Mar 29, 2021 · 12 comments
Closed

Integration testing for the sentry-native new unwinder #1359

bruno-garcia opened this issue Mar 29, 2021 · 12 comments

Comments

@bruno-garcia
Copy link
Member

bruno-garcia commented Mar 29, 2021

We've updated sentry-native with a new stack unwinder. Initial testing was successful but we need more real device testing.

This is a good candidate for a proper e2e test on a device farm.

We need to test the latest version (with the new stackwalker):
https://github.com/getsentry/sentry-java/releases/tag/4.4.0-alpha.1

vs and older version:
https://github.com/getsentry/sentry-java/releases/tag/4.3.0

The idea is to load the sample project and hit the native crash button. Then copy the generated envelope and send it somewhere for analysis.

This somewhere could be a Sentry event. The issue could be the Android SDK version and the device name/version could be a tag. The envelope can be an attachments. This way we can compare the attachments of both versions.
Or perhaps more easily is to push this to a GCP bucket so we can script out a side-by-side comparisson between the two version.

Make sure to keep around the debug symbols in case we need to ingest these events (or if we can restart the app the event will actually be sent already and properly symbolicated). This way best to use a new sentry project in sentry-sdks so we can look at the data there.

This code doesn't need to be merged here so hack away.

@Swatinem do we need anything specific in terms of stacktrace? Do we need some number of frames like 10 or whatever we have on the sample app today is fine?

The current sample native crash actually has a LOT of frames:
https://sentry.io/organizations/sentry-sdks/issues/2260073983/?project=5428559&query=is%3Aunresolved

@Swatinem
Copy link
Member

Not really, things should be fine.

Some customers reported completely empty traces before, some related to C++ code, etc. Not sure we can hit those conditions with simplified testcases here. But it would be good enough to ensure we don’t regress for these simpler cases.

@marandaneto
Copy link
Contributor

to be honest my biggest concern here is not even the frames content but actually if it crashes the user's Apps.
so even before analyzing the content of it, we need to guarantee that the number of hard crashes == number of envelopes written to the disk, so we know that the App actually didn't crash because of us (segfault in the SDK) but actually because of forced a hard crash and the file was written to the disk properly, this is the main and most important goal of the test, obviously, a next step would be analyzing its contents.

@denrase
Copy link
Collaborator

denrase commented Mar 30, 2021

Short update:

  • I have created two UI tests. The first one inits the sentry sdk and triggers the native crash. The second one checks for the file in cache/sentry/outbox.

  • The tests are executed with espresso.

  • The problem here, is that the first test takes the whole test process down.

  • Setup Microsoft AppCenter and uploaded signed APKs, same result there, obviously.

  • A promising solution was to use Android Test Orchestrator, so individual tests are run in isolation and crashes don't take down the whole process.

  • This works, the first test always fails as expected and the second one passes when the file is there and we are in FlightMode.

  • The problem is, I only managed to get this working locally, the version with Orchestrator does not work on AppCenter. Neither release or debug builds of the APKs.

  • Next step is to try another testing framework (Appium) and see if we can get around the process crashing limitation there.

@denrase
Copy link
Collaborator

denrase commented Mar 30, 2021

We seem to be running into this issue: android/android-test#743

@marandaneto
Copy link
Contributor

We seem to be running into this issue: android/android-test#743

does it work on Android < 11? we could focus on that for now if its the case

@denrase
Copy link
Collaborator

denrase commented Mar 31, 2021

@marandaneto It did not work on the AppCenter Android 10 devices either.

@denrase
Copy link
Collaborator

denrase commented Mar 31, 2021

Also tried another approach, triggering the Crash from a Service that runs in its own process, circumventing the issue that the test process is also taken down and orchestrator not working on devices. Unfortunately, this also did not work, as there was no envelope file written to outbox during my testing.

So with the limitations above, I'm not sure we can do this on AppCenter, only thing left would be to try the Appium test framework, but we might run into the same problems as with Espresso.

I'm open for more ideas/suggestions on how to do this.

@denrase
Copy link
Collaborator

denrase commented Apr 26, 2021

Ok, so basically we run two UI tests in #1371 with the following goals:

  • Start a Service where we init the Sentry SDK and provoke a crash.
  • Check if an envelope file was written to the expected file path.

In the second test, the Sentry SDK is not initialized, so the file is not ingested and deleted.

The biggest issues is due to the fact that the Android OS displays a dialog which is warning users when an app keeps crashing. We try to dismiss this dialog, but it seems that this does not always succeed. Another source of failure is other system dailogs, for example the Phone app crashing or some information about USB connection regarding debugging, that get into the way of our UI tests.

After running initially, we would run again on the failed devices. If there are still devices failing, we'd check the logs manually to see if the native SDK handled the envelope during the first test.

It is rather non-deterministic which devices will fail, so i'm not sure we can automate this with the confidence that we would need for CI. Also, tests runs could be blocked by other tests running on AppCenter, which was often the case during manual test runs. Lastly, we would need to read the result of the test through the AppCenter API.

Do you have anything to add to this @marandaneto and @bruno-garcia?

@bruno-garcia
Copy link
Member Author

Sounds like it'll be hard to maintain this. Given the false positives. Trying to think if we could rely on the screenshot of the failed test to quickly dismiss as false positive for example. And only really trigger these tests on a weekly basis for example.

It sounds like we'll end up using this manually as needed, in cases such as this. When we make a significant change to it.
That said I believe it could be worth investing a bit more time to try to get something automated. At least for some common cases which are hard to test in other ways: Hard crash in Java and native. I wonder what @marandaneto thinks though

@marandaneto
Copy link
Contributor

instead of using AppCenter and AppenterAPI to read logs, we could run on a set of emulators on CI, with different arch. and OS versions, which is good enough for CI, obviously we'd run on AppCenter too when there's the chance to crash users.
I don't think a SS will give us the idea if its a false positive or not, usually is just the App crash OS dialog, which means, the app crashed, did it crash because of us (forcing it) or did the SDK segfault? we wont know if we don't see the logs.

@marandaneto
Copy link
Contributor

I think we can have a 2nd Android example that mimics the behavior of this test and we compile/run manually on AppCenter when necessary, it'd be the least flaky I guess.

@bruno-garcia
Copy link
Member Author

I assume we can close this since we've done the 400-device test already?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants