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

test: Fix flaky session tracker tests #2198

Merged
merged 5 commits into from
Sep 21, 2022

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Sep 20, 2022

📜 Description

Testing with NSNotificationCenter in CI leads to flaky tests.
Therefore, we use a wrapper around NSNotificationCenter to not
depend on it. Instead, we call the methods NSNotificationCenter
would call with Dynamic and ensure that sut properly subscribes
to NSNotificationCenter.

#skip-changelog

💡 Motivation and Context

Fixes #2062

💚 How did you test it?

Unit tests.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

@philipphofmann philipphofmann changed the title Test/fix flaky session tracker tests test: Fix flaky session tracker tests Sep 20, 2022
@philipphofmann philipphofmann marked this pull request as ready for review September 20, 2022 15:25
Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

LGTM

@philipphofmann philipphofmann merged commit 9fef3ff into master Sep 21, 2022
@philipphofmann philipphofmann deleted the test/fix-flaky-session-tracker-tests branch September 21, 2022 12:30
@armcknight
Copy link
Member

Looking back at my attempt to use expectations to wait for the async delivery of these notifications (edd4706), and I just realized I never actually fulfilled them in those test cases 😕 Gotta use expectations (and correctly haha) for async api like this

@philipphofmann
Copy link
Member Author

I also tried the expectations, but it didn't work. I have no clue what the issue is, but not it works.

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

Successfully merging this pull request may close these issues.

Fix SentryTests.SentrySessionTrackerTests Flaky tests
3 participants