Skip to content

Commit 65e816c

Browse files
elribonazoelribonazo
committed
fix: improve tests and separation of concerns
1 parent 112ae92 commit 65e816c

14 files changed

Lines changed: 522 additions & 543 deletions

File tree

‎packages/sdk-multichain/src/connect.test.ts‎

Lines changed: 39 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { runTestsInNodeEnv, runTestsInRNEnv, runTestsInWebEnv, runTestsInWebMobi
77
import { Store } from './store';
88
import { mockSessionData, mockSessionRequestData } from '../tests/data';
99
import type { TestSuiteOptions, MockedData } from '../tests/types';
10+
import { SessionStore } from '@metamask/mobile-wallet-protocol-core';
1011

1112
async function waitForInstallModal(sdk: MultichainCore) {
1213
const onShowInstallModal = t.vi.spyOn(sdk as any, 'showInstallModal');
@@ -124,92 +125,52 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
124125
const caipAccountIds = ['eip155:1:0x1234567890abcdef1234567890abcdef12345678'] as any;
125126

126127
//Empty initial session
127-
mockedData.mockWalletGetSession.mockImplementation(() => undefined as any);
128-
mockedData.mockWalletCreateSession.mockImplementation(() => mockSessionData);
129-
mockedData.mockSessionRequest.mockImplementation(() => mockSessionRequestData);
130-
131-
let showModalPromise!: Promise<void>;
132-
let unloadSpy!: t.MockInstance<() => void>;
133-
let onConnectionSuccessSpy!: t.MockInstance<any>;
128+
mockedData.mockWalletGetSession.mockImplementation(async () => undefined as any);
129+
mockedData.mockWalletCreateSession.mockImplementation(async () => mockSessionData);
130+
mockedData.mockSessionRequest.mockImplementation(async () => mockSessionRequestData);
134131

135132
sdk = await createSDK(testOptions);
136133

137-
onConnectionSuccessSpy = t.vi.spyOn(sdk as any, 'onConnectionSuccess');
138-
unloadSpy = t.vi.spyOn((sdk as any).options.ui.factory, 'unload');
139-
140134
t.expect(sdk.state).toBe('loaded');
141135
t.expect(() => sdk.provider).toThrow();
142136
t.expect(() => sdk.transport).toThrow();
143137

144-
if (platform !== 'web' && platform !== 'web-mobile') {
145-
// Platform web is browser with metamask extension, won't have install modal
146-
showModalPromise = waitForInstallModal(sdk);
147-
}
148-
149-
const connectPromise = sdk.connect(scopes, caipAccountIds);
150-
151-
if (isMWPPlatform) {
152-
if (platform !== 'web-mobile') {
153-
(mockedData.mockDappClient as any).__state = 'CONNECTED';
154-
//For MWP we simulate a connection with DappClient after showing the QRCode
155-
await expectUIFactoryRenderInstallModal(sdk);
156-
}
157-
158-
if (platform !== 'web-mobile') {
159-
// Connect to MWP using dappClient mock
160-
mockedData.mockDappClient.connect();
161-
await showModalPromise;
162-
// Should have unloaded the modal and calling successCallback
163-
t.expect(unloadSpy).toHaveBeenCalledWith();
164-
t.expect(onConnectionSuccessSpy).toHaveBeenCalled();
165-
} else {
166-
}
167-
}
168-
await connectPromise;
138+
await sdk.connect(scopes, caipAccountIds);
169139

170140
t.expect(sdk.state).toBe('connected');
171141
t.expect(sdk.storage).toBeDefined();
172142
t.expect(sdk.transport).toBeDefined();
173-
174-
const { sessionScopes, expiry: _, ...rest } = mockSessionData;
175-
const expectedSessionResponse = {
176-
method: 'wallet_createSession',
177-
params: {
178-
...rest,
179-
optionalScopes: sessionScopes,
180-
},
181-
};
143+
t.expect(() => sdk.provider).toThrow();
182144

183145
if (isMWPPlatform) {
184146
t.expect(mockedData.mockDappClient.state).toBe('CONNECTED');
185-
t.expect(mockedData.mockDappClient.sendRequest).toHaveBeenCalledWith(t.expect.objectContaining(expectedSessionResponse));
147+
t.expect(mockedData.mockDappClient.sendRequest).toHaveBeenCalled();
186148
} else {
187149
t.expect(mockedData.mockDefaultTransport.__isConnected).toBe(true);
188-
t.expect(mockedData.mockDefaultTransport.request).toHaveBeenCalledWith(expectedSessionResponse);
150+
t.expect(mockedData.mockDefaultTransport.request).toHaveBeenCalled();
189151
}
190152
});
191153

192154
t.it(`${platform} should reconnect to the same transport when already connected in the past`, async () => {
193155
mockedData.nativeStorageStub.setItem('multichain-transport', transportString);
194-
mockedData.mockSessionRequest.mockImplementation(() => mockSessionRequestData);
195-
mockedData.mockWalletGetSession.mockImplementation(() => undefined as any);
156+
mockedData.mockSessionRequest.mockImplementation(async () => mockSessionRequestData);
157+
mockedData.mockWalletGetSession.mockImplementation(async () => undefined as any);
158+
mockedData.mockWalletCreateSession.mockImplementation(async () => mockSessionData);
196159

197160
sdk = await createSDK(testOptions);
198161
t.expect(sdk.state).toBe('connected');
199162
t.expect(sdk.transport).toBeDefined();
163+
t.expect(() => sdk.provider).toThrow();
200164
t.expect(sdk.storage).toBeDefined();
201165

202166
await t.expect(sdk.storage.getTransport()).resolves.toBe(transportString);
203167
});
204168

205169
t.it(`${platform} should skip transport connection when already connected`, async () => {
206-
const scopes = ['eip155:1'] as Scope[];
207-
const caipAccountIds = ['eip155:1:0x1234567890abcdef1234567890abcdef12345678'] as any;
208-
209170
mockedData.nativeStorageStub.setItem('multichain-transport', transportString);
210-
mockedData.mockWalletGetSession.mockImplementation(() => mockSessionData);
211-
mockedData.mockSessionRequest.mockImplementation(() => mockSessionRequestData);
212-
171+
mockedData.mockWalletGetSession.mockImplementation(async () => mockSessionData);
172+
mockedData.mockSessionRequest.mockImplementation(async () => mockSessionRequestData);
173+
mockedData.mockWalletCreateSession.mockImplementation(async () => mockSessionData);
213174
if (isWebEnv) {
214175
await mockedData.mockDefaultTransport.connect();
215176
} else {
@@ -218,6 +179,7 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
218179

219180
sdk = await createSDK(testOptions);
220181
t.expect(sdk.transport).toBeDefined();
182+
t.expect(() => sdk.provider).toThrow();
221183
t.expect(sdk.storage).toBeDefined();
222184
t.expect(sdk.state).toBe('connected');
223185

@@ -231,34 +193,20 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
231193
mockedData.mockDappClient.connect.mockClear();
232194
}
233195

234-
await sdk.connect(scopes, caipAccountIds);
235-
236-
if (isWebEnv) {
237-
t.expect(mockedData.mockDefaultTransport.__isConnected).toBe(true);
238-
t.expect(mockedData.mockDefaultTransport.connect).not.toHaveBeenCalled();
239-
} else {
240-
t.expect(mockedData.mockDappClient.state).toBe('CONNECTED');
241-
t.expect(mockedData.mockDappClient.connect).not.toHaveBeenCalled();
242-
}
243-
244196
await t.expect(sdk.storage.getTransport()).resolves.toBe(transportString);
245197
});
246198

247199
t.it(`${platform} should handle invalid CAIP account IDs gracefully`, async () => {
248-
const consoleErrorSpy = t.vi.spyOn(console, 'error').mockImplementation(() => {});
249-
250200
const scopes = ['eip155:1'] as Scope[];
251201
const caipAccountIds = ['invalid-account-id', 'eip155:1:0x1234567890abcdef1234567890abcdef12345678'] as any;
252202
let unloadSpy!: t.MockInstance<() => void>;
253-
let onConnectionSuccessSpy!: t.MockInstance<any>;
254203
let showModalPromise!: Promise<void>;
255204

256-
mockedData.mockSessionRequest.mockImplementation(() => mockSessionRequestData);
257-
mockedData.mockWalletGetSession.mockImplementation(() => undefined as any);
258-
mockedData.mockWalletCreateSession.mockImplementation(() => mockSessionData);
205+
mockedData.mockSessionRequest.mockImplementation(async () => mockSessionRequestData);
206+
mockedData.mockWalletGetSession.mockImplementation(async () => undefined as any);
207+
mockedData.mockWalletCreateSession.mockImplementation(async () => mockSessionData);
259208
sdk = await createSDK(testOptions);
260209

261-
onConnectionSuccessSpy = t.vi.spyOn(sdk as any, 'onConnectionSuccess');
262210
unloadSpy = t.vi.spyOn((sdk as any).options.ui.factory, 'unload');
263211

264212
t.expect(sdk.state).toBe('loaded');
@@ -283,16 +231,14 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
283231
await showModalPromise;
284232
// Should have unloaded the modal and calling successCallback
285233
t.expect(unloadSpy).toHaveBeenCalledWith();
286-
t.expect(onConnectionSuccessSpy).toHaveBeenCalled();
287-
} else {
288234
}
289235
}
290236

291237
await connectPromise;
292238

293239
t.expect(sdk.state).toBe('connected');
294240
t.expect(sdk.storage).toBeDefined();
295-
t.expect(sdk.provider).toBeDefined();
241+
t.expect(() => sdk.provider).toThrow();
296242
t.expect(sdk.transport).toBeDefined();
297243

298244
if (isMWPPlatform) {
@@ -302,50 +248,23 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
302248
}
303249

304250
await t.expect(sdk.storage.getTransport()).resolves.toBe(transportString);
305-
t.expect(consoleErrorSpy).toHaveBeenCalledWith('Invalid CAIP account ID: "invalid-account-id"', t.expect.any(Error));
306251
});
307252

308253
t.it(`${platform} should handle session creation errors`, async () => {
309254
const sessionError = new Error('Failed to create session');
310255
const scopes = ['eip155:1'] as Scope[];
311256
const caipAccountIds = ['eip155:1:0x1234567890abcdef1234567890abcdef12345678'] as any;
312257

313-
let unloadSpy!: t.MockInstance<() => void>;
314-
let onConnectionSuccessSpy!: t.MockInstance<any>;
315-
let showModalPromise!: Promise<void>;
316-
317-
mockedData.mockWalletGetSession.mockImplementation(() => undefined as any);
318-
mockedData.mockSessionRequest.mockImplementation(() => mockSessionRequestData);
258+
mockedData.mockWalletGetSession.mockImplementation(async () => undefined as any);
259+
mockedData.mockSessionRequest.mockImplementation(async () => mockSessionRequestData);
319260
mockedData.mockWalletCreateSession.mockRejectedValue(sessionError);
320261

321262
sdk = await createSDK(testOptions);
322263

323-
onConnectionSuccessSpy = t.vi.spyOn(sdk as any, 'onConnectionSuccess');
324-
unloadSpy = t.vi.spyOn((sdk as any).options.ui.factory, 'unload');
325-
326264
t.expect(sdk.state).toBe('loaded');
327265
t.expect(() => sdk.transport).toThrow();
328266

329-
if (platform !== 'web' && platform !== 'web-mobile') {
330-
showModalPromise = waitForInstallModal(sdk);
331-
}
332-
333-
const connectPromise = sdk.connect(scopes, caipAccountIds);
334-
335-
if (platform !== 'web' && platform !== 'web-mobile') {
336-
(mockedData.mockDappClient as any).__state = 'CONNECTED';
337-
//For MWP we simulate a connection with DappClient after showing the QRCode
338-
await expectUIFactoryRenderInstallModal(sdk);
339-
340-
// Connect to MWP using dappClient mock
341-
mockedData.mockDappClient.connect();
342-
await showModalPromise;
343-
// Should have unloaded the modal and calling successCallback
344-
t.expect(unloadSpy).toHaveBeenCalledWith();
345-
t.expect(onConnectionSuccessSpy).toHaveBeenCalled();
346-
}
347-
348-
await t.expect(() => connectPromise).rejects.toThrow(sessionError);
267+
await t.expect(() => sdk.connect(scopes, caipAccountIds)).rejects.toThrow(sessionError);
349268
});
350269

351270
t.it(`${platform} should handle session revocation errors on session upgrade`, async () => {
@@ -362,18 +281,25 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
362281

363282
const revocationError = new Error('Failed to revoke session');
364283

284+
mockedData.mockWalletCreateSession.mockResolvedValue(existingSessionData);
365285
mockedData.mockWalletGetSession.mockResolvedValue(existingSessionData);
366-
mockedData.mockWalletRevokeSession.mockRejectedValue(revocationError);
286+
mockedData.mockWalletRevokeSession.mockImplementation(async () => {
287+
throw revocationError;
288+
});
289+
t.vi.spyOn(SessionStore.prototype, 'list').mockImplementation(async () => Promise.resolve([await (mockedData as any).mockWalletGetSession()]));
367290

368291
const scopes = ['eip155:137'] as Scope[]; // Different scope to trigger upgrade
369292
const caipAccountIds = ['eip155:137:0x1234567890abcdef1234567890abcdef12345678'] as any;
370293

371294
mockedData.nativeStorageStub.setItem('multichain-transport', transportString);
372295
sdk = await createSDK(testOptions);
373296

374-
await t.expect(sdk.connect(scopes, caipAccountIds)).rejects.toThrow(revocationError);
297+
t.expect(sdk.state).toBe('connected');
298+
t.expect(sdk.storage).toBeDefined();
299+
t.expect(() => sdk.provider).toThrow();
300+
t.expect(sdk.transport).toBeDefined();
375301

376-
await t.expect(mockedData.mockLogger).toHaveBeenCalledWith('MetaMaskSDK error during onConnectionSuccess', revocationError);
302+
await t.expect(sdk.connect(scopes, caipAccountIds)).rejects.toThrow(revocationError);
377303
});
378304

379305
t.it(`${platform} should disconnect transport successfully`, async () => {
@@ -409,6 +335,10 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
409335

410336
sdk = await createSDK(testOptions);
411337
await sdk.connect(scopes, caipAccountIds);
338+
t.expect(sdk.state).toBe('connected');
339+
t.expect(() => sdk.provider).toThrow();
340+
t.expect(sdk.transport).toBeDefined();
341+
412342
await t.expect(sdk.disconnect()).rejects.toThrow('Failed to disconnect transport');
413343
});
414344

@@ -422,6 +352,8 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
422352
sdk = await createSDK(testOptions);
423353

424354
t.expect(sdk.state).toBe('connected');
355+
t.expect(() => sdk.provider).toThrow();
356+
t.expect(sdk.transport).toBeDefined();
425357

426358
t.expect(mockedData.mockDappClient.state).toBe('CONNECTED');
427359
await mockedData.mockDappClient.disconnect();

‎packages/sdk-multichain/src/domain/errors/rpc.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,6 @@ export class RPCReadonlyRequestErr extends BaseErr<'RPC', RPCErrorCodes> {
2929
export class RPCInvokeMethodErr extends BaseErr<'RPC', RPCErrorCodes> {
3030
static readonly code = 53;
3131
constructor(public readonly reason: string) {
32-
super(`RPCErr${RPCInvokeMethodErr.code}: RPC Client invoke method reason ${reason}`, RPCInvokeMethodErr.code);
32+
super(`RPCErr${RPCInvokeMethodErr.code}: RPC Client invoke method reason (${reason})`, RPCInvokeMethodErr.code);
3333
}
3434
}

‎packages/sdk-multichain/src/domain/multichain/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ export abstract class MultichainCore extends EventEmitter<SDKEvents> {
2525
abstract provider: MultichainApiClient<RPCAPI>;
2626
abstract transport: Transport;
2727

28-
abstract getCurrentSession(): Promise<SessionData | undefined>;
2928
/**
3029
* Establishes a connection to the multichain provider, or re-use existing session
3130
*

‎packages/sdk-multichain/src/domain/platform/index.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,10 @@ export function isSecure() {
7979
const platformType = getPlatformType();
8080
return isReactNative() || platformType === PlatformType.MobileWeb;
8181
}
82+
83+
export function hasExtension() {
84+
if (typeof window !== 'undefined') {
85+
return window.ethereum?.isMetaMask ?? false;
86+
}
87+
return false;
88+
}

‎packages/sdk-multichain/src/init.test.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { runTestsInNodeEnv, runTestsInRNEnv, runTestsInWebEnv, runTestsInWebMobi
88
import { analytics } from '@metamask/sdk-analytics';
99
import * as loggerModule from './domain/logger';
1010
import type { TestSuiteOptions, MockedData } from '../tests/types';
11-
import { mockSessionData } from '../tests/data';
11+
import { mockSessionData, mockSessionRequestData } from '../tests/data';
1212

1313
function testSuite<T extends MultichainOptions>({ platform, createSDK, options: sdkOptions, ...options }: TestSuiteOptions<T>) {
1414
const { beforeEach, afterEach } = options;
@@ -95,7 +95,10 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
9595
t.it(`${platform} should properly initialize if existing session transport if found during init`, async () => {
9696
// Set the transport type as a string in storage (this is how it's stored)
9797
mockedData.nativeStorageStub.setItem('multichain-transport', transportString);
98-
mockedData.mockWalletGetSession.mockImplementation(() => mockSessionData);
98+
99+
mockedData.mockSessionRequest.mockImplementation(async () => mockSessionRequestData);
100+
mockedData.mockWalletGetSession.mockImplementation(async () => mockSessionData);
101+
mockedData.mockWalletCreateSession.mockImplementation(async () => mockSessionData);
99102

100103
sdk = await createSDK(testOptions);
101104

@@ -105,10 +108,12 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
105108
t.expect(sdk.storage).toBeDefined();
106109
});
107110

108-
t.it(`${platform} should emit sessionChanged event when existing valid session is found during init`, async () => {
111+
t.it(`${platform} should emit stateChanged event when existing valid session is found during init`, async () => {
109112
// Set the transport type as a string in storage (this is how it's stored)
110113
mockedData.nativeStorageStub.setItem('multichain-transport', transportString);
111-
mockedData.mockWalletGetSession.mockImplementation(() => mockSessionData);
114+
mockedData.mockSessionRequest.mockImplementation(async () => mockSessionRequestData);
115+
mockedData.mockWalletCreateSession.mockImplementation(async () => mockSessionData);
116+
mockedData.mockWalletGetSession.mockImplementation(async () => mockSessionData);
112117

113118
const onNotification = t.vi.fn();
114119
const optionsWithEvent = {
@@ -121,10 +126,11 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
121126
sdk = await createSDK(optionsWithEvent);
122127

123128
t.expect(sdk).toBeDefined();
129+
124130
t.expect(sdk.state).toBe('connected');
125131
t.expect(onNotification).toHaveBeenCalledWith({
126-
method: 'wallet_sessionChanged',
127-
params: mockSessionData,
132+
method: 'stateChanged',
133+
params: 'connected',
128134
});
129135
});
130136

0 commit comments

Comments
 (0)