-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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(ember): Update ember tests to use E2E structure #11827
base: develop
Are you sure you want to change the base?
Conversation
fb0bc07
to
28e38c5
Compare
packages/ember/addon/index.ts
Outdated
@@ -75,9 +75,11 @@ export const instrumentRoutePerformance = <T extends RouteConstructor>(BaseRoute | |||
{ | |||
attributes: { | |||
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, | |||
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember', |
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.
this was actually missing, oops.
@@ -145,6 +145,12 @@ export function _instrumentEmberRouter( | |||
|
|||
routerService.on('routeWillChange', (transition: Transition) => { | |||
const { fromRoute, toRoute } = getTransitionInformation(transition, routerService); | |||
|
|||
// We want to ignore loading && error routes | |||
if (transitionIsIntermediate(transition)) { |
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.
Ember has a feature where you can easily add loading
and error
routes that are automatically picked up. However, if you do that it will also trigger transitions for them, which we don't really want here. So we need to filter them out here to avoid that we finish the "actual" routing spans too early.
178c8f9
to
7b22096
Compare
Extracted out from #11827. I added tests for loading & error states as well, to ensure this does not regress.
ef0e1dc
to
f7c88f9
Compare
a620ce7
to
a774dd1
Compare
This gets rid of the Ember canary tests that are always failing, probably due to ember-try (which we use there) not playing nicely with the monorepo etc.
Instead, this now uses the proper E2E setup. I added two tests, for classic ember and modern embroider-based ember. While doing this I also noticed two bugs I fixed along the way :O