Skip to content

Commit 273e522

Browse files
chore: subscription controller cleanup (#6529)
## Explanation Some clean up of subscription controller tests. - Use dedicated handleFetch mock. - Simplify error handling and related test cases <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent d53e126 commit 273e522

File tree

2 files changed

+12
-42
lines changed

2 files changed

+12
-42
lines changed

packages/subscription-controller/src/SubscriptionService.test.ts

Lines changed: 11 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ jest.mock('@metamask/controller-utils', () => ({
2525
handleFetch: jest.fn(),
2626
}));
2727

28+
const handleFetchMock = handleFetch as jest.Mock;
29+
2830
// Mock data
2931
const MOCK_SUBSCRIPTION: Subscription = {
3032
id: 'sub_123456789',
@@ -129,7 +131,7 @@ describe('SubscriptionService', () => {
129131
describe('getSubscriptions', () => {
130132
it('should fetch subscriptions successfully', async () => {
131133
await withMockSubscriptionService(async ({ service, config }) => {
132-
(handleFetch as jest.Mock).mockResolvedValue({
134+
handleFetchMock.mockResolvedValue({
133135
customerId: 'cus_1',
134136
subscriptions: [MOCK_SUBSCRIPTION],
135137
trialedProducts: [],
@@ -148,9 +150,7 @@ describe('SubscriptionService', () => {
148150

149151
it('should throw SubscriptionServiceError for error responses', async () => {
150152
await withMockSubscriptionService(async ({ service }) => {
151-
(handleFetch as jest.Mock).mockRejectedValue(
152-
new Error('Network error'),
153-
);
153+
handleFetchMock.mockRejectedValue(new Error('Network error'));
154154

155155
await expect(service.getSubscriptions()).rejects.toThrow(
156156
SubscriptionServiceError,
@@ -160,9 +160,7 @@ describe('SubscriptionService', () => {
160160

161161
it('should throw SubscriptionServiceError for network errors', async () => {
162162
await withMockSubscriptionService(async ({ service }) => {
163-
(handleFetch as jest.Mock).mockRejectedValue(
164-
new Error('Network error'),
165-
);
163+
handleFetchMock.mockRejectedValue(new Error('Network error'));
166164

167165
await expect(service.getSubscriptions()).rejects.toThrow(
168166
SubscriptionServiceError,
@@ -180,33 +178,12 @@ describe('SubscriptionService', () => {
180178
);
181179
});
182180
});
183-
184-
it('should handle null exceptions in catch block', async () => {
185-
const config = createMockConfig({});
186-
const service = new SubscriptionService(config);
187-
(handleFetch as jest.Mock).mockRejectedValue(null);
188-
189-
await expect(
190-
service.cancelSubscription({ subscriptionId: 'sub_123456789' }),
191-
).rejects.toThrow(SubscriptionServiceError);
192-
});
193-
194-
it('should handle non-Error exceptions in catch block', async () => {
195-
await withMockSubscriptionService(async ({ service }) => {
196-
// Mock handleFetch to throw null (not an Error instance)
197-
(handleFetch as jest.Mock).mockRejectedValue(null);
198-
199-
await expect(service.getSubscriptions()).rejects.toThrow(
200-
SubscriptionServiceError,
201-
);
202-
});
203-
});
204181
});
205182

206183
describe('cancelSubscription', () => {
207184
it('should cancel subscription successfully', async () => {
208185
await withMockSubscriptionService(async ({ service, config }) => {
209-
(handleFetch as jest.Mock).mockResolvedValue({});
186+
handleFetchMock.mockResolvedValue({});
210187

211188
await service.cancelSubscription({ subscriptionId: 'sub_123456789' });
212189

@@ -216,9 +193,7 @@ describe('SubscriptionService', () => {
216193

217194
it('should throw SubscriptionServiceError for network errors', async () => {
218195
await withMockSubscriptionService(async ({ service }) => {
219-
(handleFetch as jest.Mock).mockRejectedValue(
220-
new Error('Network error'),
221-
);
196+
handleFetchMock.mockRejectedValue(new Error('Network error'));
222197

223198
await expect(
224199
service.cancelSubscription({ subscriptionId: 'sub_123456789' }),
@@ -230,9 +205,7 @@ describe('SubscriptionService', () => {
230205
describe('startSubscription', () => {
231206
it('should start subscription successfully', async () => {
232207
await withMockSubscriptionService(async ({ service }) => {
233-
(handleFetch as jest.Mock).mockResolvedValue(
234-
MOCK_START_SUBSCRIPTION_RESPONSE,
235-
);
208+
handleFetchMock.mockResolvedValue(MOCK_START_SUBSCRIPTION_RESPONSE);
236209

237210
const result = await service.startSubscriptionWithCard(
238211
MOCK_START_SUBSCRIPTION_REQUEST,
@@ -251,9 +224,7 @@ describe('SubscriptionService', () => {
251224
recurringInterval: RecurringInterval.month,
252225
};
253226

254-
(handleFetch as jest.Mock).mockResolvedValue(
255-
MOCK_START_SUBSCRIPTION_RESPONSE,
256-
);
227+
handleFetchMock.mockResolvedValue(MOCK_START_SUBSCRIPTION_RESPONSE);
257228

258229
const result = await service.startSubscriptionWithCard(request);
259230

@@ -294,7 +265,7 @@ describe('SubscriptionService', () => {
294265
status: SubscriptionStatus.active,
295266
};
296267

297-
(handleFetch as jest.Mock).mockResolvedValue(response);
268+
handleFetchMock.mockResolvedValue(response);
298269

299270
const result = await service.startSubscriptionWithCrypto(request);
300271

@@ -313,7 +284,7 @@ describe('SubscriptionService', () => {
313284
const config = createMockConfig();
314285
const service = new SubscriptionService(config);
315286

316-
(handleFetch as jest.Mock).mockResolvedValue(mockPricingResponse);
287+
handleFetchMock.mockResolvedValue(mockPricingResponse);
317288

318289
const result = await service.getPricing();
319290

packages/subscription-controller/src/SubscriptionService.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,7 @@ export class SubscriptionService implements ISubscriptionService {
8585

8686
return response;
8787
} catch (e) {
88-
const errorMessage =
89-
e instanceof Error ? e.message : JSON.stringify(e ?? 'unknown error');
88+
const errorMessage = e instanceof Error ? e.message : JSON.stringify(e);
9089

9190
throw new SubscriptionServiceError(
9291
`failed to make request. ${errorMessage}`,

0 commit comments

Comments
 (0)