Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn of potential memory leaks #2357

Merged
merged 4 commits into from Apr 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/release-source/release/sandbox.md
Expand Up @@ -334,3 +334,7 @@ Verifies all mocks created through the sandbox.
#### `sandbox.verifyAndRestore();`

Verifies all mocks and restores all fakes created through the sandbox.

#### `sandbox.leakThreshold`

Gets/sets the threshold at which memory leak detection warnings are logged.
39 changes: 29 additions & 10 deletions lib/sinon/sandbox.js
@@ -1,6 +1,7 @@
"use strict";

var arrayProto = require("@sinonjs/commons").prototypes.array;
var logger = require("@sinonjs/commons").deprecated;
var collectOwnMethods = require("./collect-own-methods");
var getPropertyDescriptor = require("./util/core/get-property-descriptor");
var isPropertyConfigurable = require("./util/core/is-property-configurable");
Expand All @@ -16,6 +17,8 @@ var fakeServer = require("nise").fakeServer;
var fakeXhr = require("nise").fakeXhr;
var usePromiseLibrary = require("./util/core/use-promise-library");

var DEFAULT_LEAK_THRESHOLD = 10000;

var filter = arrayProto.filter;
var forEach = arrayProto.forEach;
var push = arrayProto.push;
Expand All @@ -33,10 +36,26 @@ function applyOnEach(fakes, method) {

function Sandbox() {
var sandbox = this;
var collection = [];
var fakeRestorers = [];
var promiseLib;

var collection = [];
var loggedLeakWarning = false;
sandbox.leakThreshold = DEFAULT_LEAK_THRESHOLD;

function addToCollection(object) {
if (
push(collection, object) > sandbox.leakThreshold &&
!loggedLeakWarning
) {
// eslint-disable-next-line no-console
logger.printWarning(
"Potential memory leak detected; be sure to call restore() to clean up your sandbox. To suppress this warning, modify the leakThreshold property of your sandbox."
);
loggedLeakWarning = true;
}
}

sandbox.assert = sinonAssert.createAssertObject();

sandbox.serverPrototype = fakeServer;
Expand All @@ -57,7 +76,7 @@ function Sandbox() {
var ownMethods = collectOwnMethods(stubbed);

forEach(ownMethods, function (method) {
push(collection, method);
addToCollection(method);
});

usePromiseLibrary(promiseLib, ownMethods);
Expand Down Expand Up @@ -115,7 +134,7 @@ function Sandbox() {
sandbox.mock = function mock() {
var m = sinonMock.apply(null, arguments);

push(collection, m);
addToCollection(m);
usePromiseLibrary(promiseLib, m);

return m;
Expand Down Expand Up @@ -348,12 +367,12 @@ function Sandbox() {
var ownMethods = collectOwnMethods(spy);

forEach(ownMethods, function (method) {
push(collection, method);
addToCollection(method);
});

usePromiseLibrary(promiseLib, ownMethods);
} else {
push(collection, spy);
addToCollection(spy);
usePromiseLibrary(promiseLib, spy);
}

Expand All @@ -374,7 +393,7 @@ function Sandbox() {
sandbox.fake = function fake(f) {
var s = sinonFake.apply(sinonFake, arguments);

push(collection, s);
addToCollection(s);

return s;
};
Expand All @@ -385,7 +404,7 @@ function Sandbox() {
sandbox.fake[key] = function () {
var s = fakeBehavior.apply(fakeBehavior, arguments);

push(collection, s);
addToCollection(s);

return s;
};
Expand All @@ -396,7 +415,7 @@ function Sandbox() {
var clock = sinonClock.useFakeTimers.call(null, args);

sandbox.clock = clock;
push(collection, clock);
addToCollection(clock);

return clock;
};
Expand Down Expand Up @@ -429,14 +448,14 @@ function Sandbox() {
}

sandbox.server = proto.create();
push(collection, sandbox.server);
addToCollection(sandbox.server);

return sandbox.server;
};

sandbox.useFakeXMLHttpRequest = function useFakeXMLHttpRequest() {
var xhr = fakeXhr.useFakeXMLHttpRequest();
push(collection, xhr);
addToCollection(xhr);
return xhr;
};

Expand Down
45 changes: 45 additions & 0 deletions test/sandbox-test.js
Expand Up @@ -134,6 +134,51 @@ describe("Sandbox", function () {

assert.equals(fakes.length, 2);
});

describe("warns of potential leak when", function () {
var warn;

beforeEach(function () {
warn = this.sandbox.stub(deprecated, "printWarning");
});

afterEach(function () {
warn.restore();
});

it("many fakes are created", function () {
assert.equals(typeof this.sandbox.leakThreshold, "number");

createTooManyFakes(this.sandbox);

assert(warn.called);
});

it("a configurable number of fakes are created", function () {
this.sandbox.leakThreshold = 20;

createTooManyFakes(this.sandbox);

assert(warn.called);
});

it("a leak warning has not already been output", function () {
this.sandbox.leakThreshold = 20;

createTooManyFakes(this.sandbox);
this.sandbox.restore();
warn.resetHistory();

createTooManyFakes(this.sandbox);
assert(!warn.called);
});

function createTooManyFakes(sandbox) {
for (var i = 0; i < sandbox.leakThreshold; i++) {
sandbox.spy();
}
}
});
});

describe(".spy", function () {
Expand Down