search

getsentry / sentry-cocoa

The official Sentry SDK for iOS, tvOS, macOS, watchOS.
https://sentry.io/for/cocoa/
MIT License
812 stars 325 forks source link

Fix flaky testProfilerMutationDuringSlicing #3910

Open philipphofmann opened 6 months ago

philipphofmann commented 6 months ago

GitHub action Run Link

https://github.com/getsentry/sentry-cocoa/actions/runs/8844626847/job/24286898382

Disabling PR

https://github.com/getsentry/sentry-cocoa/pull/3909

Description

No response

armcknight commented 6 months ago

This test is highly questionable IMO, we're just jamming a bunch of concurrent slice and mutate operations together and hoping nothing bad happens, but it's not deterministic.

In order to deterministically test that, we'd need to add a hook into the slice operation that allows us to mutate the data structures.

I know there's not much appetite for conditionally compiled test logic, but that seems like that's what's going to have to happen if we want to test this deterministically.

Ofc this is not why the test flaked in this instance; the 50 concurrent operations were not able to complete in 1 second so the expectation timed out. I'm not very inclined to just bump it to 2 and call it a day, though. It doesn't make much sense that it couldn't mutate a data structure in ~20ms (the mutations are on their own serial queue–1000ms/50ms=20ms) or compute 50 slices (those ops are on a concurrent queue, no less).

philipphofmann commented 6 months ago

@armcknight, could it be that this test will become obsolete because the logic it tests will become obsolete with continuous profiling? If yes, I wouldn't fix it but delete it at some point.