Skip to content

Commit

Permalink
Call sinon.restore() after each test using sinon (MetaMask#789)
Browse files Browse the repository at this point in the history 
`sinon.restore()` is now called after each test in any test suite that
uses `sinon` for anything. This is recommended by the Sinon docs in
their general setup instructions to avoid memory leaks.
  • Loading branch information
@Gudahtt
Gudahtt authored Apr 22, 2022
1 parent 7221c69 commit a8d4a87
Show file tree
Hide file tree
Showing 17 changed files with 159 additions and 129 deletions.
12 changes: 8 additions & 4 deletions src/BaseController.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { stub } from 'sinon';
import sinon from 'sinon';
import { BaseController, BaseConfig, BaseState } from './BaseController';

const STATE = { name: 'foo' };
Expand All @@ -12,6 +12,10 @@ class TestController extends BaseController<BaseConfig, BaseState> {
}

describe('BaseController', () => {
afterEach(() => {
sinon.restore();
});

it('should set initial state', () => {
const controller = new TestController(undefined, STATE);
expect(controller.state).toStrictEqual(STATE);
Expand Down Expand Up @@ -45,8 +49,8 @@ describe('BaseController', () => {

it('should notify all listeners', () => {
const controller = new TestController(undefined, STATE);
const listenerOne = stub();
const listenerTwo = stub();
const listenerOne = sinon.stub();
const listenerTwo = sinon.stub();
controller.subscribe(listenerOne);
controller.subscribe(listenerTwo);
controller.notify();
Expand All @@ -58,7 +62,7 @@ describe('BaseController', () => {

it('should not notify unsubscribed listeners', () => {
const controller = new TestController();
const listener = stub();
const listener = sinon.stub();
controller.subscribe(listener);
controller.unsubscribe(listener);
controller.unsubscribe(() => null);
Expand Down
4 changes: 4 additions & 0 deletions src/BaseControllerV2.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ class CountController extends BaseController<
}

describe('BaseController', () => {
afterEach(() => {
sinon.restore();
});

it('should set initial state', () => {
const controller = new CountController({
messenger: getCountMessenger(),
Expand Down
14 changes: 9 additions & 5 deletions src/ComposableController.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { stub } from 'sinon';
import sinon from 'sinon';
import type { Patch } from 'immer';
import { TokensController } from './assets/TokensController';
import { CollectiblesController } from './assets/CollectiblesController';
Expand Down Expand Up @@ -87,6 +87,10 @@ class BarController extends BaseController<never, BarControllerState> {
}

describe('ComposableController', () => {
afterEach(() => {
sinon.restore();
});

describe('BaseController', () => {
it('should compose controller state', () => {
const preferencesController = new PreferencesController();
Expand Down Expand Up @@ -268,7 +272,7 @@ describe('ComposableController', () => {
it('should notify listeners of nested state change', () => {
const addressBookController = new AddressBookController();
const controller = new ComposableController([addressBookController]);
const listener = stub();
const listener = sinon.stub();
controller.subscribe(listener);
addressBookController.set(
'0x32Be343B94f860124dC4fEe278FDCBD38C102D88',
Expand Down Expand Up @@ -381,7 +385,7 @@ describe('ComposableController', () => {
composableControllerMessenger,
);

const listener = stub();
const listener = sinon.stub();
composableController.subscribe(listener);
fooController.updateFoo('bar');

Expand Down Expand Up @@ -486,7 +490,7 @@ describe('ComposableController', () => {
composableControllerMessenger,
);

const listener = stub();
const listener = sinon.stub();
composableController.subscribe(listener);
barController.updateBar('foo');

Expand Down Expand Up @@ -528,7 +532,7 @@ describe('ComposableController', () => {
composableControllerMessenger,
);

const listener = stub();
const listener = sinon.stub();
composableController.subscribe(listener);
fooController.updateFoo('bar');

Expand Down
9 changes: 7 additions & 2 deletions src/approval/ApprovalController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,13 @@ function getRestrictedMessenger() {
}

describe('approval controller', () => {
const clock = sinon.useFakeTimers(1);
afterAll(() => clock.restore());
beforeEach(() => {
sinon.useFakeTimers(1);
});

afterEach(() => {
sinon.restore();
});

describe('add', () => {
let approvalController: ApprovalController;
Expand Down
24 changes: 14 additions & 10 deletions src/assets/AccountTrackerController.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { stub, spy } from 'sinon';
import sinon from 'sinon';
import HttpProvider from 'ethjs-provider-http';
import type { ContactEntry } from '../user/AddressBookController';
import { PreferencesController } from '../user/PreferencesController';
Expand All @@ -10,9 +10,13 @@ const provider = new HttpProvider(
);

describe('AccountTrackerController', () => {
afterEach(() => {
sinon.restore();
});

it('should set default state', () => {
const controller = new AccountTrackerController({
onPreferencesStateChange: stub(),
onPreferencesStateChange: sinon.stub(),
getIdentities: () => ({}),
});
expect(controller.state).toStrictEqual({
Expand All @@ -22,7 +26,7 @@ describe('AccountTrackerController', () => {

it('should throw when provider property is accessed', () => {
const controller = new AccountTrackerController({
onPreferencesStateChange: stub(),
onPreferencesStateChange: sinon.stub(),
getIdentities: () => ({}),
});
expect(() => console.log(controller.provider)).toThrow(
Expand All @@ -34,7 +38,7 @@ describe('AccountTrackerController', () => {
const address = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d';
const controller = new AccountTrackerController(
{
onPreferencesStateChange: stub(),
onPreferencesStateChange: sinon.stub(),
getIdentities: () => {
return { [address]: {} as ContactEntry };
},
Expand All @@ -47,10 +51,10 @@ describe('AccountTrackerController', () => {

it('should sync balance with addresses', async () => {
const address = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d';
const queryStub = stub(utils, 'query');
const queryStub = sinon.stub(utils, 'query');
const controller = new AccountTrackerController(
{
onPreferencesStateChange: stub(),
onPreferencesStateChange: sinon.stub(),
getIdentities: () => {
return {};
},
Expand All @@ -65,7 +69,7 @@ describe('AccountTrackerController', () => {
it('should sync addresses', () => {
const controller = new AccountTrackerController(
{
onPreferencesStateChange: stub(),
onPreferencesStateChange: sinon.stub(),
getIdentities: () => {
return { baz: {} as ContactEntry };
},
Expand Down Expand Up @@ -93,7 +97,7 @@ describe('AccountTrackerController', () => {
},
{ provider },
);
controller.refresh = stub();
controller.refresh = sinon.stub();

preferences.setFeatureFlag('foo', true);
expect((controller.refresh as any).called).toBe(true);
Expand All @@ -102,7 +106,7 @@ describe('AccountTrackerController', () => {
it('should call refresh every ten seconds', async () => {
await new Promise<void>((resolve) => {
const preferences = new PreferencesController();
const poll = spy(AccountTrackerController.prototype, 'poll');
const poll = sinon.spy(AccountTrackerController.prototype, 'poll');
const controller = new AccountTrackerController(
{
onPreferencesStateChange: (listener) =>
Expand All @@ -111,7 +115,7 @@ describe('AccountTrackerController', () => {
},
{ provider, interval: 100 },
);
stub(controller, 'refresh');
sinon.stub(controller, 'refresh');

expect(poll.called).toBe(true);
expect(poll.calledTwice).toBe(false);
Expand Down
2 changes: 0 additions & 2 deletions src/assets/CollectibleDetectionController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ describe('CollectibleDetectionController', () => {
expect(mockCollectibles.calledOnce).toBe(true);
setTimeout(() => {
expect(mockCollectibles.calledTwice).toBe(true);
mockCollectibles.restore();
resolve('');
}, 15);
});
Expand Down Expand Up @@ -273,7 +272,6 @@ describe('CollectibleDetectionController', () => {
{ interval: 10, networkType: ROPSTEN },
);
expect(mockCollectibles.called).toBe(false);
mockCollectibles.restore();
resolve('');
});
});
Expand Down
23 changes: 12 additions & 11 deletions src/assets/CurrencyRateController.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { stub } from 'sinon';
import sinon from 'sinon';
import nock from 'nock';
import { ControllerMessenger } from '../ControllerMessenger';
import {
Expand Down Expand Up @@ -36,10 +36,11 @@ const getStubbedDate = () => {
describe('CurrencyRateController', () => {
afterEach(() => {
nock.cleanAll();
sinon.restore();
});

it('should set default state', () => {
const fetchExchangeRateStub = stub();
const fetchExchangeRateStub = sinon.stub();
const messenger = getRestrictedMessenger();
const controller = new CurrencyRateController({
fetchExchangeRate: fetchExchangeRateStub,
Expand All @@ -59,7 +60,7 @@ describe('CurrencyRateController', () => {
});

it('should initialize with initial state', () => {
const fetchExchangeRateStub = stub();
const fetchExchangeRateStub = sinon.stub();
const messenger = getRestrictedMessenger();
const existingState = { currentCurrency: 'rep' };
const controller = new CurrencyRateController({
Expand All @@ -81,7 +82,7 @@ describe('CurrencyRateController', () => {
});

it('should not poll before being started', async () => {
const fetchExchangeRateStub = stub();
const fetchExchangeRateStub = sinon.stub();
const messenger = getRestrictedMessenger();
const controller = new CurrencyRateController({
interval: 100,
Expand All @@ -96,7 +97,7 @@ describe('CurrencyRateController', () => {
});

it('should poll and update rate in the right interval', async () => {
const fetchExchangeRateStub = stub();
const fetchExchangeRateStub = sinon.stub();
const messenger = getRestrictedMessenger();
const controller = new CurrencyRateController({
interval: 100,
Expand All @@ -115,7 +116,7 @@ describe('CurrencyRateController', () => {
});

it('should not poll after being stopped', async () => {
const fetchExchangeRateStub = stub();
const fetchExchangeRateStub = sinon.stub();
const messenger = getRestrictedMessenger();
const controller = new CurrencyRateController({
interval: 100,
Expand All @@ -136,7 +137,7 @@ describe('CurrencyRateController', () => {
});

it('should poll correctly after being started, stopped, and started again', async () => {
const fetchExchangeRateStub = stub();
const fetchExchangeRateStub = sinon.stub();
const messenger = getRestrictedMessenger();
const controller = new CurrencyRateController({
interval: 100,
Expand All @@ -160,7 +161,7 @@ describe('CurrencyRateController', () => {
});

it('should update exchange rate', async () => {
const fetchExchangeRateStub = stub().resolves({ conversionRate: 10 });
const fetchExchangeRateStub = sinon.stub().resolves({ conversionRate: 10 });
const messenger = getRestrictedMessenger();
const controller = new CurrencyRateController({
interval: 10,
Expand All @@ -175,7 +176,7 @@ describe('CurrencyRateController', () => {
});

it('should update current currency', async () => {
const fetchExchangeRateStub = stub().resolves({ conversionRate: 10 });
const fetchExchangeRateStub = sinon.stub().resolves({ conversionRate: 10 });
const messenger = getRestrictedMessenger();
const controller = new CurrencyRateController({
interval: 10,
Expand All @@ -190,7 +191,7 @@ describe('CurrencyRateController', () => {
});

it('should update native currency', async () => {
const fetchExchangeRateStub = stub().resolves({ conversionRate: 10 });
const fetchExchangeRateStub = sinon.stub().resolves({ conversionRate: 10 });
const messenger = getRestrictedMessenger();
const controller = new CurrencyRateController({
interval: 10,
Expand All @@ -205,7 +206,7 @@ describe('CurrencyRateController', () => {
});

it('should add usd rate to state when includeUsdRate is configured true', async () => {
const fetchExchangeRateStub = stub().resolves({});
const fetchExchangeRateStub = sinon.stub().resolves({});
const messenger = getRestrictedMessenger();
const controller = new CurrencyRateController({
includeUsdRate: true,
Expand Down
2 changes: 0 additions & 2 deletions src/assets/TokenBalancesController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ describe('TokenBalancesController', () => {
expect(mock.calledTwice).toBe(false);
setTimeout(() => {
expect(mock.calledTwice).toBe(true);
mock.restore();
resolve();
}, 15);
});
Expand Down Expand Up @@ -103,7 +102,6 @@ describe('TokenBalancesController', () => {
setTimeout(() => {
tokenBalances.poll(1338);
expect(mock.called).toBe(true);
mock.restore();
resolve();
}, 100);
});
Expand Down
2 changes: 0 additions & 2 deletions src/assets/TokenDetectionController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ describe('TokenDetectionController', () => {
expect(mockTokens.calledOnce).toBe(true);
setTimeout(() => {
expect(mockTokens.calledTwice).toBe(true);
mockTokens.restore();
resolve('');
}, 15);
});
Expand Down Expand Up @@ -234,7 +233,6 @@ describe('TokenDetectionController', () => {
{ interval: 10, networkType: ROPSTEN },
);
expect(mockTokens.called).toBe(false);
mockTokens.restore();
resolve('');
});
});
Expand Down
Loading

0 comments on commit a8d4a87

Please sign in to comment.