-
Notifications
You must be signed in to change notification settings - Fork 0
test(releases): Migrate release components off deprecated route mocks #135
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
base: master
Are you sure you want to change the base?
Conversation
just two last test files
|
Replace deprecated router mocks in release tests The PR updates the last two release-related test files to stop using the deprecated Key Changes• Removed Affected Areas• This summary was automatically generated by @propel-code-bot |
WalkthroughTwo test files in the releases module are refactored to replace deprecated router initialization patterns with a new OrganizationFixture and explicit RouterConfig-based approach for test setup. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
static/app/views/releases/detail/commitsAndFiles/commits.spec.tsx (1)
84-84: Consider using dynamic slug values in mock URLs (pre-existing).These mock URLs use hardcoded
'org-slug'and'project-slug'instead oforganization.slugandproject.slug. While this works because the fixtures default to these values, using the dynamic references (as done in lines 54, 58, 67, 78, 95, 115, 150) would improve consistency and reduce fragility if fixture defaults change.Example refactor for line 84:
- url: `/projects/org-slug/project-slug/releases/${encodeURIComponent( + url: `/projects/${organization.slug}/${project.slug}/releases/${encodeURIComponent(Also applies to: 101-101, 128-128, 156-156
static/app/views/releases/detail/commitsAndFiles/filesChanged.spec.tsx (1)
98-98: Consider using dynamic organization slug in mock URLs (pre-existing).These mock URLs use hardcoded
'org-slug'instead oforganization.slug. While functional (since the fixture defaults to 'org-slug'), using the dynamic reference (as done in lines 68, 72, 80, 92, 109, 127) would improve consistency and maintainability.Example refactor for line 98:
- url: `/organizations/org-slug/releases/${encodeURIComponent( + url: `/organizations/${organization.slug}/releases/${encodeURIComponent(Also applies to: 115-115, 140-140
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
static/app/views/releases/detail/commitsAndFiles/commits.spec.tsx(5 hunks)static/app/views/releases/detail/commitsAndFiles/filesChanged.spec.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
static/app/views/releases/detail/commitsAndFiles/commits.spec.tsx (2)
tests/js/fixtures/organization.ts (1)
OrganizationFixture(5-93)tests/js/sentry-test/reactTestingLibrary.tsx (1)
RouterConfig(88-118)
static/app/views/releases/detail/commitsAndFiles/filesChanged.spec.tsx (2)
tests/js/fixtures/organization.ts (1)
OrganizationFixture(5-93)tests/js/sentry-test/reactTestingLibrary.tsx (1)
RouterConfig(88-118)
🔇 Additional comments (5)
static/app/views/releases/detail/commitsAndFiles/commits.spec.tsx (3)
2-2: LGTM! Clean migration to the new test pattern.The introduction of
OrganizationFixtureandRouterConfigproperly replaces the deprecated router initialization. The route configuration accurately reflects the commits view path structure.Also applies to: 8-8, 19-19, 21-27
45-46: Correct render configuration.The
renderComponentfunction now properly passesorganizationandinitialRouterConfigto the render options, completing the migration from deprecated router mocks.
181-188: Proper query parameter override pattern.The inline
rendercall correctly spreadsinitialRouterConfigand overrides the location to test the active repository selection scenario. The explicit pathname redefinition is clear and maintains consistency.static/app/views/releases/detail/commitsAndFiles/filesChanged.spec.tsx (2)
2-2: LGTM! Consistent migration pattern.The migration properly introduces
OrganizationFixtureandRouterConfig, matching the pattern used in the commits test file. The route configuration correctly reflects the files-changed view path.Also applies to: 8-8, 33-33, 35-41
59-60: Correct render configuration.The render options now properly pass
organizationandinitialRouterConfig, completing the migration away from deprecated router mocks.
just two last test files
Copied from getsentry#103707
Original PR: getsentry#103707
Move release component tests to new router helpers
This PR updates the last two release-related test files to stop using the deprecated
initializeOrg/deprecatedRouterMockshelpers and migrate to the newerOrganizationFixture+ explicitRouterConfigpattern. The only changes are within test code; no production code is touched.Key Changes
• Removed
initializeOrganddeprecatedRouterMocksusage fromcommits.spec.tsxandfilesChanged.spec.tsx• Added
OrganizationFixtureandRouterConfigimports and constructedinitialRouterConfigobjects to provide routing context explicitly• Updated render helpers to pass
{organization, initialRouterConfig}instead of{router, deprecatedRouterMocks}• Deleted obsolete mocks that relied on query params built via
initializeOrgAffected Areas
•
static/app/views/releases/detail/commitsAndFiles/commits.spec.tsx•
static/app/views/releases/detail/commitsAndFiles/filesChanged.spec.tsxThis summary was automatically generated by @propel-code-bot
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.