test(billing): decouple subscriptions component test from tenant plan IDs#4191
Conversation
WalkthroughThis PR mocks the ChangesSubscription test tenant decoupling
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/billing/tests/billing.subscriptions.component.unit.tests.js (1)
397-410:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse mocked highest plan ID in the “highest plan” CTA test.
This suite now mocks plans as
p1/p2/p3, but the highest-plan test still usesplan: 'pro'. That can make the assertion pass via an unknown-plan path instead of true highest-plan behavior.Suggested fix
- it('hides the paid plan upgrade CTA on the highest plan', async () => { - store.subscription = { status: 'active', plan: 'pro', currentPeriodEnd: new Date().toISOString() }; + it('hides the paid plan upgrade CTA on the highest plan', async () => { + store.subscription = { status: 'active', plan: 'p3', currentPeriodEnd: new Date().toISOString() }; wrapper = mountSubscriptions({ serverConfig: { billing: { meterMode: false } } }); await flushPromises(); expect(wrapper.text()).not.toContain('Change Plan'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/billing/tests/billing.subscriptions.component.unit.tests.js` around lines 397 - 410, The "hides the paid plan upgrade CTA on the highest plan" test uses a hardcoded plan 'pro' which doesn't match the mocked plans (p1/p2/p3); update the test to set store.subscription.plan to the mocked highest plan (e.g., 'p3') so the code path for the actual highest plan is exercised, leaving the rest of the test (mountSubscriptions call and assertions) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/modules/billing/tests/billing.subscriptions.component.unit.tests.js`:
- Around line 397-410: The "hides the paid plan upgrade CTA on the highest plan"
test uses a hardcoded plan 'pro' which doesn't match the mocked plans
(p1/p2/p3); update the test to set store.subscription.plan to the mocked highest
plan (e.g., 'p3') so the code path for the actual highest plan is exercised,
leaving the rest of the test (mountSubscriptions call and assertions) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 09857790-7907-4dc5-89d9-17c1a91d3217
📒 Files selected for processing (1)
src/modules/billing/tests/billing.subscriptions.component.unit.tests.js
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4191 +/- ##
=======================================
Coverage 99.56% 99.56%
=======================================
Files 31 31
Lines 1140 1140
Branches 329 329
=======================================
Hits 1135 1135
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the billing subscriptions component unit tests to avoid coupling assertions to tenant-specific billing plan IDs by mocking resolveStaticContent() with generic plan identifiers.
Changes:
- Added a
vi.mock('../lib/billing.resolveStaticContent.js', ...)in the subscriptions component test to provide stable, generic plan IDs (p1,p2,p3). - Updated the “Change Plan” CTA test fixture to use a generic plan ID (
p2) instead of a tenant-specific one (starter).
|
|
||
| vi.mock('../lib/billing.resolveStaticContent.js', () => ({ | ||
| resolveStaticContent: () => ({ | ||
| plans: [{ id: 'p1' }, { id: 'p2' }, { id: 'p3' }], |
Summary
Closes #4189
vi.mock('../lib/billing.resolveStaticContent.js', ...)returning generic plan IDs (p1,p2,p3) to decouple the subscriptions component test from tenant-specific plan IDs (e.g.starter)p2plan instead of the hardcodedstarterplan IDWhy
The billing subscriptions component test was relying on tenant-specific plan IDs that live in resolved static content. When the plan IDs change (e.g. new pricing tiers, GTM plan rename), tests break for non-structural reasons. This mock makes the test fixture-agnostic and aligned with the billing module's own test isolation pattern.
Test plan
npm run test:unit)Summary by CodeRabbit