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
Hubs/Scopes Merge 42d - Close previous scopes before binding a new global client #3404
Conversation
…pes-merge-2-add-scopes
…ainScopes to rootScopes
import kotlin.test.Test | ||
|
||
class MetricsIntegrationTest { | ||
|
||
@BeforeTest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test started becoming flaky. Not sure why exactly.
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- Hubs/Scopes Merge 42d - Close previous scopes before binding a new global client ([#3404](https://github.com/getsentry/sentry-java/pull/3404)) If none of the above apply, you can opt out of this check by adding |
Performance metrics 🚀
|
it.dsn = dsn | ||
} | ||
verify(scopes).close(eq(true)) | ||
assertTrue(Sentry.getGlobalScope().client.isEnabled) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also check that Sentry.getCurrentScopes()
has an enabled client?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We basically already are. It'd be more of a duplicate of tests already present in ScopesTest / CombinedScopeViewTest. I think it's fine as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
One question on test
📜 Description
💡 Motivation and Context
When calling
close
on the previous scopes after a new client has already been bound to the global scope, the new client is closed and Sentry does not capture events anymore.💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps