Add more unit tests for SmartTransactionsController (#23)

* comments about where to add unit tests

* add more test coverage for smart transactions controller

* chore: add tests for getStxProcessingTime

* chore: clean up tests

* chore: more clean up

* fix test leak

Author: meppsilon

jest.config.js

@@ -6,7 +6,7 @@ module.exports = {
   coverageReporters: ['text', 'html'],
   coverageThreshold: {
     global: {
-      branches: 81,
+      branches: 79,
       functions: 91,
       lines: 93,
       statements: 93,

src/SmartTransactionsController.test.ts

@@ -4,7 +4,7 @@ import SmartTransactionsController, {
   DEFAULT_INTERVAL,
 } from './SmartTransactionsController';
 import { API_BASE_URL, CHAIN_IDS, CHAIN_IDS_HEX_TO_DEC } from './constants';
-import { SmartTransaction } from './types';
+import { SmartTransaction, SmartTransactionStatuses } from './types';
 
 const confirmExternalMock = jest.fn();
 
@@ -31,9 +31,11 @@ jest.mock('ethers', () => ({
   },
 }));
 
+const addressFrom = '0x268392a24B6b093127E8581eAfbD1DA228bAdAe3';
+
 const createUnsignedTransaction = () => {
   return {
-    from: '0x268392a24B6b093127E8581eAfbD1DA228bAdAe3',
+    from: addressFrom,
     to: '0x0000000000000000000000000000000000000000',
     value: 0,
     data: '0x',
@@ -195,6 +197,8 @@ const testHistory = [
 
 const ethereumChainIdDec = CHAIN_IDS_HEX_TO_DEC[CHAIN_IDS.ETHEREUM];
 
+const trackMetaMetricsEventSpy = jest.fn();
+
 describe('SmartTransactionsController', () => {
   let smartTransactionsController: SmartTransactionsController;
   let networkListener: (networkState: NetworkState) => void;
@@ -217,12 +221,10 @@ describe('SmartTransactionsController', () => {
       txController: {
         confirmExternalTransaction: confirmExternalMock,
       },
-      trackMetaMetricsEvent: jest.fn(),
+      trackMetaMetricsEvent: trackMetaMetricsEventSpy,
     });
-
-    jest
-      .spyOn(smartTransactionsController, 'checkPoll')
-      .mockImplementation(() => ({}));
+    // eslint-disable-next-line jest/prefer-spy-on
+    smartTransactionsController.subscribe = jest.fn();
   });
 
   afterEach(async () => {
@@ -264,6 +266,35 @@ describe('SmartTransactionsController', () => {
     });
   });
 
+  describe('checkPoll', () => {
+    it('calls poll if there is no pending transaction and pending transactions', () => {
+      const pollSpy = jest
+        .spyOn(smartTransactionsController, 'poll')
+        .mockImplementation(() => {
+          return new Promise(() => ({}));
+        });
+      const { smartTransactionsState } = smartTransactionsController.state;
+      const pendingStx = createStateAfterPending();
+      smartTransactionsController.update({
+        smartTransactionsState: {
+          ...smartTransactionsState,
+          smartTransactions: {
+            [CHAIN_IDS.ETHEREUM]: pendingStx as SmartTransaction[],
+          },
+        },
+      });
+      expect(pollSpy).toHaveBeenCalled();
+    });
+
+    it('calls stop if there is a timeoutHandle and no pending transactions', () => {
+      const stopSpy = jest.spyOn(smartTransactionsController, 'stop');
+      smartTransactionsController.timeoutHandle = setInterval(() => ({}));
+      smartTransactionsController.checkPoll(smartTransactionsController.state);
+      expect(stopSpy).toHaveBeenCalled();
+      clearInterval(smartTransactionsController.timeoutHandle);
+    });
+  });
+
   describe('poll', () => {
     it('does not call updateSmartTransactions on unsupported networks', async () => {
       const updateSmartTransactionsSpy = jest.spyOn(
@@ -276,6 +307,59 @@ describe('SmartTransactionsController', () => {
     });
   });
 
+  describe('updateSmartTransactions', () => {
+    it('calls fetchSmartTransactionsStatus if there are pending transactions', () => {
+      const fetchSmartTransactionsStatusSpy = jest
+        .spyOn(smartTransactionsController, 'fetchSmartTransactionsStatus')
+        .mockImplementation(() => {
+          return new Promise(() => ({}));
+        });
+      const { smartTransactionsState } = smartTransactionsController.state;
+      const pendingStx = createStateAfterPending();
+      smartTransactionsController.update({
+        smartTransactionsState: {
+          ...smartTransactionsState,
+          smartTransactions: {
+            [CHAIN_IDS.ETHEREUM]: pendingStx as SmartTransaction[],
+          },
+        },
+      });
+      expect(fetchSmartTransactionsStatusSpy).toHaveBeenCalled();
+    });
+  });
+
+  describe('trackStxStatusChange', () => {
+    it('does not track if no prevSmartTransactions', () => {
+      const smartTransaction = createStateAfterPending()[0];
+      smartTransactionsController.trackStxStatusChange(
+        smartTransaction as SmartTransaction,
+      );
+      expect(trackMetaMetricsEventSpy).not.toHaveBeenCalled();
+    });
+
+    it('does not track if smartTransaction and prevSmartTransaction have the same status', () => {
+      const smartTransaction = createStateAfterPending()[0];
+      smartTransactionsController.trackStxStatusChange(
+        smartTransaction as SmartTransaction,
+        smartTransaction as SmartTransaction,
+      );
+      expect(trackMetaMetricsEventSpy).not.toHaveBeenCalled();
+    });
+
+    it('tracks status change if smartTransaction and prevSmartTransaction have different statuses', () => {
+      const smartTransaction = {
+        ...createStateAfterPending()[0],
+        swapMetaData: {},
+      };
+      const prevSmartTransaction = { ...smartTransaction, status: '' };
+      smartTransactionsController.trackStxStatusChange(
+        smartTransaction as SmartTransaction,
+        prevSmartTransaction as SmartTransaction,
+      );
+      expect(trackMetaMetricsEventSpy).toHaveBeenCalled();
+    });
+  });
+
   describe('setOptInState', () => {
     it('sets optIn state', () => {
       smartTransactionsController.setOptInState(true);
@@ -310,6 +394,11 @@ describe('SmartTransactionsController', () => {
   });
 
   describe('submitSignedTransactions', () => {
+    beforeEach(() => {
+      // eslint-disable-next-line jest/prefer-spy-on
+      smartTransactionsController.checkPoll = jest.fn(() => ({}));
+    });
+
     it('submits a smart transaction with signed transactions', async () => {
       const signedTransaction = createSignedTransaction();
       const signedCanceledTransaction = createSignedCanceledTransaction();
@@ -332,6 +421,11 @@ describe('SmartTransactionsController', () => {
   });
 
   describe('fetchSmartTransactionsStatus', () => {
+    beforeEach(() => {
+      // eslint-disable-next-line jest/prefer-spy-on
+      smartTransactionsController.checkPoll = jest.fn(() => ({}));
+    });
+
     it('fetches a pending status for a single smart transaction via batchStatus API', async () => {
       const uuids = ['uuid1'];
       const pendingBatchStatusApiResponse = createPendingBatchStatusApiResponse();
@@ -395,6 +489,11 @@ describe('SmartTransactionsController', () => {
   });
 
   describe('updateSmartTransaction', () => {
+    beforeEach(() => {
+      // eslint-disable-next-line jest/prefer-spy-on
+      smartTransactionsController.checkPoll = jest.fn(() => ({}));
+    });
+
     it('updates smart transaction based on uuid', () => {
       const pendingStx = {
         ...createStateAfterPending()[0],
@@ -453,6 +552,11 @@ describe('SmartTransactionsController', () => {
   });
 
   describe('confirmSmartTransaction', () => {
+    beforeEach(() => {
+      // eslint-disable-next-line jest/prefer-spy-on
+      smartTransactionsController.checkPoll = jest.fn(() => ({}));
+    });
+
     it('calls confirm external transaction', async () => {
       const successfulStx = {
         ...createStateAfterSuccess()[0],
@@ -463,6 +567,20 @@ describe('SmartTransactionsController', () => {
       );
       expect(confirmExternalMock).toHaveBeenCalled();
     });
+
+    it('throws an error if ethersProvider fails', async () => {
+      smartTransactionsController.ethersProvider.getTransactionReceipt.mockRejectedValueOnce(
+        'random error' as never,
+      );
+      const successfulStx = {
+        ...createStateAfterSuccess()[0],
+        history: testHistory,
+      };
+      await smartTransactionsController.confirmSmartTransaction(
+        successfulStx as SmartTransaction,
+      );
+      expect(trackMetaMetricsEventSpy).toHaveBeenCalled();
+    });
   });
 
   describe('cancelSmartTransaction', () => {
@@ -487,4 +605,43 @@ describe('SmartTransactionsController', () => {
       expect(configureSpy).toHaveBeenCalledTimes(0);
     });
   });
+
+  describe('getTransactions', () => {
+    beforeEach(() => {
+      // eslint-disable-next-line jest/prefer-spy-on
+      smartTransactionsController.checkPoll = jest.fn(() => ({}));
+    });
+
+    it('retrieves smart transactions by addressFrom and status', () => {
+      const { smartTransactionsState } = smartTransactionsController.state;
+      const pendingStx = {
+        ...createStateAfterPending()[0],
+        history: testHistory,
+        txParams: {
+          from: addressFrom,
+        },
+      };
+      smartTransactionsController.update({
+        smartTransactionsState: {
+          ...smartTransactionsState,
+          smartTransactions: {
+            [CHAIN_IDS.ETHEREUM]: [pendingStx] as SmartTransaction[],
+          },
+        },
+      });
+      const pendingStxs = smartTransactionsController.getTransactions({
+        addressFrom,
+        status: SmartTransactionStatuses.PENDING,
+      });
+      expect(pendingStxs).toStrictEqual([pendingStx]);
+    });
+
+    it('returns empty array if there are no smart transactions', () => {
+      const transactions = smartTransactionsController.getTransactions({
+        addressFrom,
+        status: SmartTransactionStatuses.PENDING,
+      });
+      expect(transactions).toStrictEqual([]);
+    });
+  });
 });

src/SmartTransactionsController.ts

@@ -56,13 +56,13 @@ export default class SmartTransactionsController extends BaseController<
   SmartTransactionsControllerConfig,
   SmartTransactionsControllerState
 > {
-  private timeoutHandle?: NodeJS.Timeout;
+  public timeoutHandle?: NodeJS.Timeout;
 
   private nonceTracker: any;
 
   private getNetwork: any;
 
-  private ethersProvider: any;
+  public ethersProvider: any;
 
   public txController: any;
 
@@ -172,7 +172,7 @@ export default class SmartTransactionsController extends BaseController<
   async poll(interval?: number): Promise<void> {
     const { chainId, supportedChainIds } = this.config;
     interval && this.configure({ interval }, false, false);
-    this.timeoutHandle && clearTimeout(this.timeoutHandle);
+    this.timeoutHandle && clearInterval(this.timeoutHandle);
     if (!supportedChainIds.includes(chainId)) {
       return;
     }

src/utils.test.ts

@@ -148,4 +148,18 @@ describe('src/utils.js', () => {
       );
     });
   });
+
+  describe('getStxProcessingTime', () => {
+    it('returns undefined if no smartTransactionTimeSubmitted', () => {
+      expect(utils.getStxProcessingTime(undefined)).toBeUndefined();
+    });
+
+    it('returns 2m and 57s if processing time is 3s ago', () => {
+      const THREE_SECONDS_AGO = 3 * 1000;
+      const processingTime = utils.getStxProcessingTime(
+        Date.now() - THREE_SECONDS_AGO,
+      );
+      expect(processingTime).toStrictEqual(3);
+    });
+  });
 });