Skip to content

Conversation

@kamilogorek
Copy link
Contributor

@kamilogorek kamilogorek commented Apr 29, 2021

@kamilogorek kamilogorek requested review from a team, ahmedetefy and iker-barriocanal and removed request for a team April 29, 2021 12:42
@github-actions
Copy link
Contributor

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 20.63 KB (-0.02% 🔽)
@sentry/browser - Webpack 21.5 KB (0%)
@sentry/react - Webpack 21.53 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 27.92 KB (-0.01% 🔽)

Copy link
Contributor

@rhcarvalho rhcarvalho left a comment

Choose a reason for hiding this comment

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

Do we have examples of the flakiness in CI (to document in the PR description)?

import { OnUnhandledRejection } from '../src/integrations/onunhandledrejection';

jest.mock('@sentry/hub', () => {
// we just want to short-circuit it, so dont worry about types
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the rest of the change, don't fully understand this.

Why do we want to short-circuit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because setupOnce is checking whether an integration is installed, which in the case of these tests should always be the case. I think this was the reason for flakyness, because hub is attached to the global object.

Comment on lines +1 to +2
import { Scope } from '@sentry/core';
import { Hub } from '@sentry/hub';
Copy link
Contributor

Choose a reason for hiding this comment

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

For my learning -- Does changing this import affect the test, or is it just cosmetic/preference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cosmetic preference

@kamilogorek
Copy link
Contributor Author

Linked an example

@kamilogorek kamilogorek requested a review from rhcarvalho April 29, 2021 13:10
@HazAT HazAT merged commit 35f9eb1 into master Apr 30, 2021
@HazAT HazAT deleted the fix-flaky-test branch April 30, 2021 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants