Skip to content

Commit

Permalink
Debounce: Remove debug logging
Browse files Browse the repository at this point in the history
 * linting
 * Make timeout work with mock-request (FIX ME: total hack)
 * node doesn't like non-transpiled 'const'
  • Loading branch information
pik committed Jan 20, 2017
1 parent 9f011be commit b6200c3
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 62 deletions.
6 changes: 3 additions & 3 deletions spec/integ/matrix-client-syncing.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe("MatrixClient syncing", function() {

beforeEach(function() {
utils.beforeEach(this); // eslint-disable-line no-invalid-this
httpBackend = new HttpBackend();
httpBackend = new HttpBackend(true);
sdk.request(httpBackend.requestFn);
client = sdk.createClient({
baseUrl: baseUrl,
Expand All @@ -44,14 +44,14 @@ describe("MatrixClient syncing", function() {
};

it("should start keepalives if a /sync hits a timeout error", function(done) {
httpBackend.when("GET", "/sync").respond(200, syncData).timeout(61*1000);
// expect(client.startClient).not.toThrow();
httpBackend.when("GET", "/sync").respond(200, syncData).waitFor(61*1000);
client.startClient();
spyOn(client._syncApi, '_startKeepAlives').andCallThrough();
// Work-around since
// Jasmine expect(<method>).toThrow() seems to break self-references in
// that method.
httpBackend.flush().done(function() {
expect(client._syncApi._startKeepAlives).toHaveBeenCalled();
done();
});
});
Expand Down
53 changes: 29 additions & 24 deletions spec/mock-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ let clock = jasmine.Clock;
let fakeDate;
let callbacks = require("../lib/realtime-callbacks");

var tick = function tick(millis) {
let tick = function tick(millis) {
// make sure we tick the fakedate first, otherwise nothing will happen!
fakeDate += millis;
clock.tick(millis);
Expand All @@ -15,10 +15,12 @@ var tick = function tick(millis) {
* Construct a mock HTTP backend, heavily inspired by Angular.js.
* @constructor
*/
function HttpBackend() {
function HttpBackend(useMockClock) {
this.requests = [];
this.expectedRequests = [];
this.useMockClock = useMockClock ? true : false
const self = this;

// the request function dependency that the SDK needs.
this.requestFn = function(opts, callback) {
const req = new Request(opts, callback);
Expand Down Expand Up @@ -51,16 +53,18 @@ HttpBackend.prototype = {
const defer = q.defer();
const self = this;
let flushed = 0;
let triedWaiting = false;
let waitLoops = 0;
console.log(
"HTTP backend flushing... (path=%s numToFlush=%s)", path, numToFlush
);
// Mock out Clock for .timeout() tests
clock.useMock();
fakeDate = Date.now();
callbacks.setNow(function() {
return fakeDate;
});
if (this.useMockClock) {
clock.useMock();
fakeDate = Date.now();
callbacks.setNow(function() {
return fakeDate;
});
}

const tryFlush = function tryFlush() {
// if there's more real requests and more expected requests, flush 'em.
Expand All @@ -74,26 +78,27 @@ HttpBackend.prototype = {
flushed += 1;
if (numToFlush && flushed === numToFlush) {
console.log(" [%s] Flushed assigned amount: %s", path, numToFlush);
// Restore Clock
if (self.useMockClock) {
callbacks.setNow();
}
defer.resolve();
} else {
// setImmediate(tryFlush);
process.nextTick(tryFlush);
// tick(0);
setImmediate(tryFlush);
}
} else if (flushed === 0 && !triedWaiting) {
// we may not have made the request yet, so wait before giving
// up. Don't use setTimeout since that may be mocked during
// tests.
} else if (flushed === 0 && waitLoops < 50) {
waitLoops += 1
setImmediate(tryFlush);
triedWaiting = true;
} else {
console.log(" no more flushes. [%s]", path);
// Restore Clock
if (self.useMockClock) {
callbacks.setNow();
}
defer.resolve();
}
// Restore Clock
callbacks.setNow();
};
process.nextTick(tryFlush);
setImmediate(tryFlush);
return defer.promise;
},

Expand Down Expand Up @@ -137,7 +142,6 @@ HttpBackend.prototype = {
}
testResponse = matchingReq.response;
if (matchingReq._waitMs) {
console.log('TIMEOUT: ', matchingReq._waitMs, matchingReq.method, matchingReq.path)
tick(matchingReq._waitMs);
}
console.log(" responding to %s", matchingReq.path);
Expand Down Expand Up @@ -227,9 +231,9 @@ ExpectedRequest.prototype = {
* @param {Number} code The HTTP status code.
* @param {Object|Function} data The HTTP JSON body. If this is a function,
* it will be invoked when the JSON body is required (which should be returned).
* @return {Request} for chaining calls.
*/
respond: function(code, data) {
console.log('RECEIVED RESPOND: ', this.method, this.path)
this.response = {
response: {
statusCode: code,
Expand Down Expand Up @@ -258,12 +262,13 @@ ExpectedRequest.prototype = {
},

/**
* Timeout a request by waitMs.
* Advance the time before processing a request by waitMs.
*
* @param {Number} waitMs number of miliseconds of timeout.
* @return {Request} for chaining calls.
*/
timeout: function(waitMs) {
this._waitMs = waitMs
waitFor: function(waitMs) {
this._waitMs = waitMs;
return this;
},
};
Expand Down
57 changes: 30 additions & 27 deletions spec/unit/realtime-callbacks.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,30 +27,33 @@ describe("realtime-callbacks", function() {
});

describe("debounce", function() {
// it("should call the callback after the timeout if it has not been debounced", function() {
// let callback = jasmine.createSpy();
// let jobName = callbacks.debounce(undefined, callback, 100)
// expect(callback).not.toHaveBeenCalled();
// tick(100);
// expect(callback).toHaveBeenCalled();
// expect(callbacks.__getDebouncers()[jobName]).toBe(undefined)
// });

// it("should return a randomly generated jobName to refence the debouncer if one is not provided", function() {
// let callback = jasmine.createSpy();
// let jobName = callbacks.debounce(undefined, callback, 100)
// expect(callback).not.toHaveBeenCalled();
// expect(typeof(jobName)).toBe("string");
// expect(callbacks.__getDebouncers()[jobName]).toBeTruthy()
// });

// it("should accept a jobName as an argument to reference the debouncer", function() {
// let callback = jasmine.createSpy();
// let jobName = callbacks.debounce("foo", callback, 100)
// expect(callback).not.toHaveBeenCalled();
// expect(jobName).toEqual("foo");
// expect(callbacks.__getDebouncers()[jobName]).toBeTruthy()
// });
it(`should call the callback after the timeout
if it has not been debounced`, function() {
let callback = jasmine.createSpy();
let jobName = callbacks.debounce(undefined, callback, 100);
expect(callback).not.toHaveBeenCalled();
tick(100);
expect(callback).toHaveBeenCalled();
expect(callbacks.__getDebouncers()[jobName]).toBe(undefined);
});

it(`should return a randomly generated jobName to
reference the debouncer if one is not provided`, function() {
let callback = jasmine.createSpy();
let jobName = callbacks.debounce(undefined, callback, 100);
expect(callback).not.toHaveBeenCalled();
expect(typeof(jobName)).toBe("string");
expect(callbacks.__getDebouncers()[jobName]).toBeTruthy();
});

it(`should accept a jobName as an argument
to reference the debouncer`, function() {
let callback = jasmine.createSpy();
let jobName = callbacks.debounce("foo", callback, 100);
expect(callback).not.toHaveBeenCalled();
expect(jobName).toEqual("foo");
expect(callbacks.__getDebouncers()[jobName]).toBeTruthy();
});

it("should run waitMs after the last debounce call", function() {
let callback = jasmine.createSpy();
Expand All @@ -76,7 +79,7 @@ describe("realtime-callbacks", function() {
expect(callback).not.toHaveBeenCalled();
tick(90);
expect(callback).not.toHaveBeenCalled();
callbacks.debounce(jobName, undefined, 0)
callbacks.debounce(jobName, undefined, 0);
tick(0);
expect(callback).toHaveBeenCalled();
});
Expand All @@ -89,7 +92,7 @@ describe("realtime-callbacks", function() {
expect(callback).not.toHaveBeenCalled();
tick(90);
expect(callback).not.toHaveBeenCalled();
callbacks.debounce(jobName, newCallback)
callbacks.debounce(jobName, newCallback);
tick(100);
expect(callback).not.toHaveBeenCalled();
expect(newCallback).toHaveBeenCalled();
Expand All @@ -100,7 +103,7 @@ describe("realtime-callbacks", function() {
it("clears a debouncer by jobName", function() {
let jobName = callbacks.debounce(undefined, function() {}, 1000);
expect(callbacks.clearDebounce(jobName)).toEqual(true);
expect(callbacks.__getDebouncers()[jobName]).toBe(undefined)
expect(callbacks.__getDebouncers()[jobName]).toBe(undefined);
});

it("returns false if no debouncer was cleared", function() {
Expand Down
5 changes: 1 addition & 4 deletions src/http-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -651,15 +651,13 @@ module.exports.MatrixHttpApi.prototype = {

const defer = q.defer();

let timeoutId;
let timedOut = false;
let req;
let timeoutContext;
const localTimeoutMs = opts.localTimeoutMs || this.opts.localTimeoutMs;

if (localTimeoutMs) {
var timeoutFunction = function() {
console.log('TIMED OUT')
timedOut = true;
if (req && req.abort) {
req.abort();
Expand Down Expand Up @@ -690,7 +688,7 @@ module.exports.MatrixHttpApi.prototype = {
},
function(err, response, body) {
if (timeoutContext) {
timeoutContext.stop()
timeoutContext.stop();
delete req.onprogres;
if (timedOut) {
return; // already rejected promise
Expand All @@ -710,7 +708,6 @@ module.exports.MatrixHttpApi.prototype = {
);
if (timeoutContext) {
req.onprogress = timeoutContext.onProgressCheck;
console.log('START TIMEOUT CONTEXT')
timeoutContext.start();
}
if (req && req.abort) {
Expand Down
2 changes: 1 addition & 1 deletion src/realtime-callbacks.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ module.exports.clearDebounce = function(jobName) {
*/
module.exports.__getDebouncers = function() {
return _debouncers;
}
};

/**
* reimplementation of window.setTimeout, which will call the callback if
Expand Down
5 changes: 2 additions & 3 deletions src/sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const DEBUG = true;
// beyond that and wedge forever, so we need to track how long we are willing
// to keep open the connection. This constant is *ADDED* to the timeout= value
// to determine the max time we're willing to wait.
const BUFFER_PERIOD_MS = 80 * 1000;
const BUFFER_PERIOD_MS = 30 * 1000;

function getFilterName(userId, suffix) {
// scope this on the user ID because people may login on many accounts
Expand Down Expand Up @@ -76,7 +76,7 @@ function SyncApi(client, opts) {
this._keepAliveTimer = null;
this._connectionReturnedDefer = null;
this._notifEvents = []; // accumulator of sync events in the current sync response
console.log(client.constructor)

if (client.getNotifTimelineSet()) {
reEmit(client, client.getNotifTimelineSet(),
["Room.timeline", "Room.timelineReset"]);
Expand Down Expand Up @@ -869,7 +869,6 @@ SyncApi.prototype._processSyncResponse = function(syncToken, data) {
* @return {promise}
*/
SyncApi.prototype._startKeepAlives = function(delay) {
console.log('KEEP ALIVES');
if (delay === undefined) {
delay = 2000 + Math.floor(Math.random() * 5000);
}
Expand Down

0 comments on commit b6200c3

Please sign in to comment.