Skip to content

Conversation

@mydea
Copy link
Member

@mydea mydea commented Nov 9, 2022

We already had some logic to make test less brittle, but the runloop is a bit unstable, and thus hard to test. So it is easier to just don't test the runloop execution in detail, especially since these are the least interesting spans, and focus on testing the important parts (that are also more stable).

@mydea mydea added Package: ember Issues related to the Sentry Ember SDK Dev: CI labels Nov 9, 2022
@mydea mydea requested review from Lms24 and k-fish November 9, 2022 08:56
@mydea mydea self-assigned this Nov 9, 2022
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

🚀

Comment on lines -92 to -96
'ui.ember.runloop.actions | undefined',
'ui.ember.runloop.routerTransitions | undefined',
'ui.ember.runloop.render | undefined',
'ui.ember.runloop.afterRender | undefined',
'ui.ember.runloop.destroy | undefined',
Copy link
Member

Choose a reason for hiding this comment

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

l: Since you said that the order of these spans is the thing that makes the tests brittle, is there a way we can check that they are still present regardless of the order? Maybe some sort of "just make sure that some runloop spans are recorded" sort of thing? But you're obviously the Ember expert here, so if you say that there's no value in doing that, that's totally fine with me.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could do that, but that also goes into rather brittle territory - e.g. these are all auto-captured, and could even change between versions etc (theoretically).

I guess I can add a check that at least one runloop span has been added, that should be safe to do, and at least ensures that the overall runloop capturing happens!

@mydea mydea force-pushed the fn/ember-flaky-test branch 2 times, most recently from 9a02920 to 5ffdbf7 Compare November 9, 2022 10:10
We already had some logic to make test less brittle, but the runloop is a bit unstable, and thus hard to test. So it is easier to just don't test the runloop execution in detail, especially since these are the least interesting spans, and focus on testing the important parts (that are also more stable).
@mydea mydea force-pushed the fn/ember-flaky-test branch from 5ffdbf7 to b2250d3 Compare November 9, 2022 12:56
@mydea mydea merged commit adbe784 into master Nov 9, 2022
@mydea mydea deleted the fn/ember-flaky-test branch November 9, 2022 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Dev: CI Package: ember Issues related to the Sentry Ember SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants