-
-
Notifications
You must be signed in to change notification settings - Fork 226
Device/integration tests for mobile on .net10 #4750
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4750 +/- ##
==========================================
- Coverage 73.85% 73.85% -0.01%
==========================================
Files 485 485
Lines 17689 17689
Branches 3497 3497
==========================================
- Hits 13065 13064 -1
Misses 3762 3762
- Partials 862 863 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Tricky... locally everything passes for me: However in CI we've got 2 tests failing in Debug and 2 tests failing in Release: Looking at the 2 envelopes received for the Java crashes (java_received.json) the only difference is the timestamp: ... so it looks like we're getting duplicate envelopes being sent for some reason |
|
@supervacuus wondering if you might have some insights. When bumping to .NET 10, our Android integration tests fail when creating a native crash...
Basically we have a single crash, so we're expecting a single envelope... but instead we get loads of envelopes (from sentry.native). We don't see the same behaviour in .net 9 (those tests pass fine)... so maybe a change in behaviour in terms of how .net 10 wraps signals 🤷🏻 although, if so, not one that's documented in the release notes. It's not always 16 envelopes - in previous runs it's been 26 or 28 etc. so basically just "way more than expected". I can't reproduce this locally - it only happens to us in CI (which makes it pretty tricky to analyse). It looks like maybe there are some tests that are already relevant in the native repo, so I created a PR to bump these to .NET 10 to see if we see the same issue there... although those are run on Linux Alpine (not Android) so not necessarily expecting the same results: |
|
Hi @jamescrosswell 👋
From what I have seen over the last year in the .NET runtime's "documentation" on signal usage, I would be surprised if a change in behavior in that area made its way into the release notes. I don't have an immediate idea and it is rather hard to follow for me, because i don't know what your test does and which of those outputs are relevant. However, intuitively, a variable number of envelopes rather than one looks like a signal loop (typically caused by an But it's really hard to say for me from what I know about your tests. It would be interesting to see all envelopes rather than just their count.
But does it repro on CI stably? Would it be possible to
Yes, the .NET tests fail with that bump, but likely due to .NET 10 being an unsupported target (see my response). I would also ping @jpnurmi on this topic. |
This reverts commit 344c80a.
Yes normally. He's unfortunately unavailable until next year 😢
That's fair... JP actually wrote the tests. They're supposed to be high level simple integration tests in pester/powershell that compile a .net app and run it, passing in an argument that determines what the app should do. For example, in the test that's failing here, the argument is
Hm, that gives me some clues. After each test, the app gets shut down with this. That should shut down Sentry before calling I'll dig into that and report back - thanks @supervacuus 🙏🏻 |
|
So I tried cascading the SDK shutdown to the Java SDK: sentry-dotnet/src/Sentry/Internal/Hub.cs Line 874 in 278c04c
Unfortunately the Java SDK also doesn't cascade this to the instance of the native SDK 😢 I guess it would be possible to rectify that in the Java SDK but that would require a series of new releases etc. I'll see if I can find an alternative workaround. |
|
@Flash0ver in the latest test runs we're seeing:
However when I run the same code in a sample application, I don't see the same behaviour, so I'm pretty sure this is as a result of the difficulty in finding a way to shut the app down in a way that does't create a signal loop and lots of other unrelated exceptions that get captured by the native client. One possible "proper fix" would be to cascades the SDK shutdown logic, but I don't want to delay the version 6 release in order to build that so I think we park this PR for the time being and circle back post v6 release (perhaps in the new year when JP is back on deck). |


Resolves #4744
#skip-changelog