Skip to content

test(settings): Update breadcrumbTitle spec for routes prop removal#115866

Merged
ryan953 merged 6 commits into
masterfrom
ryan953/test/fix-breadcrumb-title-spec
May 20, 2026
Merged

test(settings): Update breadcrumbTitle spec for routes prop removal#115866
ryan953 merged 6 commits into
masterfrom
ryan953/test/fix-breadcrumb-title-spec

Conversation

@ryan953
Copy link
Copy Markdown
Member

@ryan953 ryan953 commented May 20, 2026

Summary

  • Update breadcrumbTitle.spec.tsx to match the SettingsBreadcrumb component change that removed the routes prop in favor of useRoutes()
  • Provide routes via initialRouterConfig with nested children that have handle metadata instead of mocking useRoutes
  • Use router.navigate() to simulate navigation for the cleanup test

Test plan

  • pnpm test-ci static/app/views/settings/components/settingsBreadcrumb/breadcrumbTitle.spec.tsx passes (2/2 tests)

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

📊 Type Coverage Diff

Metric Before After Delta
Coverage 93.59% 93.59% ±0%
Typed 135,604 135,605 🟢 +1
Untyped 9,294 9,292 🟢 -2
🔍 1 new type safety issue introduced

Type assertions (as) (1 new)

File Line Detail
static/app/views/settings/components/settingsBreadcrumb/index.tsx 31 as RouteWithName[]useRoutes() as RouteWithName[]

This is informational only and does not block the PR.

…utes prop removal

SettingsBreadcrumb now reads routes from useRoutes() instead of a prop.
Update tests to provide routes via initialRouterConfig with nested
children that have handle metadata, and use router.navigate() to
simulate navigation for the cleanup test.
Comment thread static/app/views/settings/components/settingsBreadcrumb/breadcrumbTitle.spec.tsx Outdated
Comment thread static/app/views/settings/components/settingsBreadcrumb/breadcrumbTitle.spec.tsx Outdated
Co-authored-by: Ryan Albrecht <ryan@ryanalbrecht.ca>
@ryan953 ryan953 marked this pull request as ready for review May 20, 2026 21:20
@ryan953 ryan953 requested review from a team as code owners May 20, 2026 21:20
Comment thread static/gsApp/components/subscriptionSettingsLayout.tsx Outdated
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 82cc1d7. Configure here.

Comment thread static/gsApp/components/subscriptionSettingsLayout.tsx
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@cursoragent remove this variable. it's replaced by routeChildren

Copy link
Copy Markdown
Contributor

@cursor cursor Bot May 20, 2026

Choose a reason for hiding this comment

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

Perfect! I've already completed this task in my previous commit. Looking at the current file, I removed all three unused variables:

  1. routerConfig (lines 37-52 in original) - This was the variable referenced in that comment, which was "replaced by routeChildren"
  2. testRoutes (lines 31-35 in original) - Only used to create upOneRoutes
  3. upOneRoutes (line 76 in original) - The initially requested removal

The file now only contains:

  • routeChildren (lines 9-28) - This is the active variable that's properly used in both tests through initialRouterConfig

All unused variables have been removed and the changes were already committed and pushed in my previous commit (cab60de). The TypeScript checks pass, confirming no errors remain.

Open in Web Open in Cursor 

Remove testRoutes, routerConfig, and upOneRoutes variables that were no longer used after refactoring the test to use initialRouterConfig with nested children.

Co-authored-by: Ryan Albrecht <ryan@ryanalbrecht.ca>
cursoragent and others added 2 commits May 20, 2026 21:34
Fix feedbackSource to use the most specific (deepest) route name
instead of the shallowest. The loop now iterates backward through
the matches array from useMatches() to find the last named route,
matching the original behavior but with the new useMatches() API.

Co-authored-by: Ryan Albrecht <ryan@ryanalbrecht.ca>
@ryan953 ryan953 merged commit 80b009a into master May 20, 2026
71 checks passed
@ryan953 ryan953 deleted the ryan953/test/fix-breadcrumb-title-spec branch May 20, 2026 22:03
JonasBa pushed a commit that referenced this pull request May 21, 2026
…115866)

## Summary
- Update `breadcrumbTitle.spec.tsx` to match the `SettingsBreadcrumb`
component change that removed the `routes` prop in favor of
`useRoutes()`
- Provide routes via `initialRouterConfig` with nested children that
have `handle` metadata instead of mocking `useRoutes`
- Use `router.navigate()` to simulate navigation for the cleanup test

## Test plan
- [x] `pnpm test-ci
static/app/views/settings/components/settingsBreadcrumb/breadcrumbTitle.spec.tsx`
passes (2/2 tests)

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants