Skip to content
This repository has been archived by the owner on Feb 2, 2021. It is now read-only.

Commit

Permalink
Add no-floating-promises options to tslint in order to ensure all pro…
Browse files Browse the repository at this point in the history 
…mises are properly handled
  • Loading branch information
@Fatme
Fatme committed Jul 20, 2018
1 parent c3aaa6c commit ffb1471
Show file tree
Hide file tree
Showing 12 changed files with 130 additions and 101 deletions.
2 changes: 2 additions & 0 deletions appbuilder/device-emitter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ export class DeviceEmitter extends EventEmitter {
this.attachApplicationChangedHandlers(device);

// await: Do not await as this will require to mark the lambda with async keyword, but there's no way to await the lambda itself.
/* tslint:disable:no-floating-promises */
device.openDeviceLogStream();
/* tslint:enable:no-floating-promises */
});

this.$devicesService.on(DeviceDiscoveryEventNames.DEVICE_LOST, (device: Mobile.IDevice) => {
Expand Down
3 changes: 2 additions & 1 deletion helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ export function settlePromises<T>(promises: Promise<T>[]): Promise<T[]> {
if (settledPromisesCount === length) {
errors.length ? reject(new Error(`Multiple errors were thrown:${EOL}${errors.map(e => e.message || e).join(EOL)}`)) : resolve(results);
}
});
})
.catch();
});
});
}
Expand Down
2 changes: 1 addition & 1 deletion http-client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export class HttpClient implements Server.IHttpClient {
return response;
}

private async setResponseResult(result: IPromiseActions<Server.IResponse>, timerId: number, resultData: { response?: Server.IRequestResponseData, body?: string, err?: Error }): Promise<void> {
private setResponseResult(result: IPromiseActions<Server.IResponse>, timerId: number, resultData: { response?: Server.IRequestResponseData, body?: string, err?: Error }): void {
if (timerId) {
clearTimeout(timerId);
timerId = null;
Expand Down
16 changes: 9 additions & 7 deletions progress-indicator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ export class ProgressIndicator implements IProgressIndicator {
let isFulfilled = false;

const tempPromise = new Promise<T>((resolve, reject) => {
promise.then((res) => {
isFulfilled = true;
resolve(res);
}, (err) => {
isFulfilled = true;
reject(err);
});
promise
.then(res => {
isFulfilled = true;
resolve(res);
})
.catch(err => {
isFulfilled = true;
reject(err);
});
});

if (!isInteractive()) {
Expand Down
2 changes: 1 addition & 1 deletion services/livesync-service-base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class LiveSyncServiceBase implements ILiveSyncServiceBase {
const livesyncData = this.livesyncData[platformName];
await batch.syncFiles(async (filesToSync: string[]) => {
await this.$liveSyncProvider.preparePlatformForSync(platformName, projectId);
this.syncCore([livesyncData], filesToSync);
await this.syncCore([livesyncData], filesToSync);
});
}
} catch (err) {
Expand Down
6 changes: 3 additions & 3 deletions services/livesync/sync-batch.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class SyncBatch {
}
}

public addFile(file: string): void {
public async addFile(file: string): Promise<void> {
if (this.timer) {
clearTimeout(this.timer);
this.timer = null;
Expand All @@ -36,12 +36,12 @@ export class SyncBatch {
this.syncQueue.push(file);

if (!this.syncInProgress) {
this.timer = setTimeout(() => {
this.timer = setTimeout(async () => {
if (this.syncQueue.length > 0) {
this.$logger.trace("Syncing %s", this.syncQueue.join(", "));
try {
this.syncInProgress = true;
this.done();
await this.done();
} finally {
this.syncInProgress = false;
}
Expand Down
6 changes: 4 additions & 2 deletions test/unit-tests/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,10 +462,12 @@ describe("helpers", () => {
helpers.settlePromises<any>(testData.input)
.then(res => {
assert.deepEqual(res, testData.expectedResult);
}, err => {
})
.catch(err => {
assert.deepEqual(err.message, testData.expectedError);
})
.then(done, done);
.then(done)
.catch(done);
});
});

Expand Down
2 changes: 2 additions & 0 deletions test/unit-tests/mobile/android/logcat-helper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ describe("logcat-helper", () => {
});

const logcatHelper = injector.resolve<LogcatHelper>("logcatHelper");
/* tslint:disable:no-floating-promises */
logcatHelper.start("valid-identifier");
/* tslint:enable:no-floating-promises */
});
it("if loghelper start is called multiple times with the same identifier it's a noop the second time", async () => {
const logcatHelper = injector.resolve<LogcatHelper>("logcatHelper");
Expand Down
178 changes: 100 additions & 78 deletions test/unit-tests/mobile/application-manager-base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,9 @@ describe("ApplicationManagerBase", () => {
}
});

/* tslint:disable:no-floating-promises */
applicationManager.checkForApplicationUpdates();
/* tslint:enable:no-floating-promises */
});

it("emits debuggableViewLost when views for debug are removed", (done: mocha.Done) => {
Expand All @@ -259,27 +261,29 @@ describe("ApplicationManagerBase", () => {
const expectedResults = _.cloneDeep(currentlyAvailableAppWebViewsForDebugging);
const currentDebuggableViews: IDictionary<Mobile.IDebugWebViewInfo[]> = {};

applicationManager.checkForApplicationUpdates().then(() => {
applicationManager.on("debuggableViewLost", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
currentDebuggableViews[appIdentifier] = currentDebuggableViews[appIdentifier] || [];
currentDebuggableViews[appIdentifier].push(d);
const numberOfFoundViewsPerApp = _.uniq(_.values(currentDebuggableViews).map(arr => arr.length));
if (_.keys(currentDebuggableViews).length === currentlyAvailableAppsForDebugging.length
&& numberOfFoundViewsPerApp.length === 1 // for all apps we've found exactly two apps.
&& numberOfFoundViewsPerApp[0] === numberOfViewsPerApp) {
_.each(currentDebuggableViews, (webViews, appId) => {
_.each(webViews, webView => {
const expectedWebView = _.find(expectedResults[appId], c => c.id === webView.id);
assert.isTrue(_.isEqual(webView, expectedWebView));
applicationManager.checkForApplicationUpdates()
.then(() => {
applicationManager.on("debuggableViewLost", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
currentDebuggableViews[appIdentifier] = currentDebuggableViews[appIdentifier] || [];
currentDebuggableViews[appIdentifier].push(d);
const numberOfFoundViewsPerApp = _.uniq(_.values(currentDebuggableViews).map(arr => arr.length));
if (_.keys(currentDebuggableViews).length === currentlyAvailableAppsForDebugging.length
&& numberOfFoundViewsPerApp.length === 1 // for all apps we've found exactly two apps.
&& numberOfFoundViewsPerApp[0] === numberOfViewsPerApp) {
_.each(currentDebuggableViews, (webViews, appId) => {
_.each(webViews, webView => {
const expectedWebView = _.find(expectedResults[appId], c => c.id === webView.id);
assert.isTrue(_.isEqual(webView, expectedWebView));
});
});
});
setTimeout(done, 0);
}
});
setTimeout(done, 0);
}
});

currentlyAvailableAppWebViewsForDebugging = _.mapValues(currentlyAvailableAppWebViewsForDebugging, (a) => []);
return applicationManager.checkForApplicationUpdates();
});
currentlyAvailableAppWebViewsForDebugging = _.mapValues(currentlyAvailableAppWebViewsForDebugging, (a) => []);
return applicationManager.checkForApplicationUpdates();
})
.catch();
});

it("emits debuggableViewFound when new views are available for debug", (done: mocha.Done) => {
Expand All @@ -290,30 +294,36 @@ describe("ApplicationManagerBase", () => {
let expectedAppIdentifier = currentlyAvailableAppsForDebugging[0].appIdentifier;
let isLastCheck = false;

applicationManager.checkForApplicationUpdates().then(() => {
applicationManager.on("debuggableViewFound", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
assert.deepEqual(appIdentifier, expectedAppIdentifier);
assert.isTrue(_.isEqual(d, expectedViewToBeFound));
applicationManager.checkForApplicationUpdates()
.then(() => {
applicationManager.on("debuggableViewFound", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
assert.deepEqual(appIdentifier, expectedAppIdentifier);
assert.isTrue(_.isEqual(d, expectedViewToBeFound));

if (isLastCheck) {
setTimeout(done, 0);
}
});
if (isLastCheck) {
setTimeout(done, 0);
}
});

currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].push(_.cloneDeep(expectedViewToBeFound));
return applicationManager.checkForApplicationUpdates();
}).then(() => {
expectedViewToBeFound = createDebuggableWebView("uniqueId1");
currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].push(_.cloneDeep(expectedViewToBeFound));
return applicationManager.checkForApplicationUpdates();
}).then(() => {
expectedViewToBeFound = createDebuggableWebView("uniqueId2");
expectedAppIdentifier = currentlyAvailableAppsForDebugging[1].appIdentifier;
isLastCheck = true;

currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].push(_.cloneDeep(expectedViewToBeFound));
return applicationManager.checkForApplicationUpdates();
});
currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].push(_.cloneDeep(expectedViewToBeFound));
return applicationManager.checkForApplicationUpdates();
})
.catch()
.then(() => {
expectedViewToBeFound = createDebuggableWebView("uniqueId1");
currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].push(_.cloneDeep(expectedViewToBeFound));
return applicationManager.checkForApplicationUpdates();
})
.catch()
.then(() => {
expectedViewToBeFound = createDebuggableWebView("uniqueId2");
expectedAppIdentifier = currentlyAvailableAppsForDebugging[1].appIdentifier;
isLastCheck = true;

currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].push(_.cloneDeep(expectedViewToBeFound));
return applicationManager.checkForApplicationUpdates();
})
.catch();
});

it("emits debuggableViewLost when views for debug are not available anymore", (done: mocha.Done) => {
Expand All @@ -324,27 +334,33 @@ describe("ApplicationManagerBase", () => {
let expectedViewToBeLost = currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].splice(0, 1)[0];
let isLastCheck = false;

applicationManager.checkForApplicationUpdates().then(() => {
applicationManager.on("debuggableViewLost", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
assert.deepEqual(appIdentifier, expectedAppIdentifier);
assert.isTrue(_.isEqual(d, expectedViewToBeLost));
applicationManager.checkForApplicationUpdates()
.then(() => {
applicationManager.on("debuggableViewLost", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
assert.deepEqual(appIdentifier, expectedAppIdentifier);
assert.isTrue(_.isEqual(d, expectedViewToBeLost));

if (isLastCheck) {
setTimeout(done, 0);
}
});

return applicationManager.checkForApplicationUpdates();
}).then(() => {
expectedViewToBeLost = currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].splice(0, 1)[0];
return applicationManager.checkForApplicationUpdates();
}).then(() => {
expectedAppIdentifier = currentlyAvailableAppsForDebugging[1].appIdentifier;
expectedViewToBeLost = currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].splice(0, 1)[0];
if (isLastCheck) {
setTimeout(done, 0);
}
});

isLastCheck = true;
return applicationManager.checkForApplicationUpdates();
});
return applicationManager.checkForApplicationUpdates();
})
.catch()
.then(() => {
expectedViewToBeLost = currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].splice(0, 1)[0];
return applicationManager.checkForApplicationUpdates();
})
.catch()
.then(() => {
expectedAppIdentifier = currentlyAvailableAppsForDebugging[1].appIdentifier;
expectedViewToBeLost = currentlyAvailableAppWebViewsForDebugging[expectedAppIdentifier].splice(0, 1)[0];

isLastCheck = true;
return applicationManager.checkForApplicationUpdates();
})
.catch();
});

it("emits debuggableViewChanged when view's property is modified (each one except id)", (done: mocha.Done) => {
Expand All @@ -359,10 +375,12 @@ describe("ApplicationManagerBase", () => {
setTimeout(done, 0);
});

applicationManager.checkForApplicationUpdates().then(() => {
viewToChange.title = "new title";
return applicationManager.checkForApplicationUpdates();
});
applicationManager.checkForApplicationUpdates()
.then(() => {
viewToChange.title = "new title";
return applicationManager.checkForApplicationUpdates();
})
.catch();
});

it("does not emit debuggableViewChanged when id is modified", (done: mocha.Done) => {
Expand All @@ -371,23 +389,27 @@ describe("ApplicationManagerBase", () => {
const viewToChange = currentlyAvailableAppWebViewsForDebugging[currentlyAvailableAppsForDebugging[0].appIdentifier][0];
const expectedView = _.cloneDeep(viewToChange);

applicationManager.checkForApplicationUpdates().then(() => {
applicationManager.on("debuggableViewChanged", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
setTimeout(() => done(new Error("When id is changed, debuggableViewChanged must not be emitted.")), 0);
});
applicationManager.checkForApplicationUpdates()
.then(() => {
applicationManager.on("debuggableViewChanged", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
setTimeout(() => done(new Error("When id is changed, debuggableViewChanged must not be emitted.")), 0);
});

applicationManager.on("debuggableViewLost", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
assert.isTrue(_.isEqual(d, expectedView));
});
applicationManager.on("debuggableViewLost", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
assert.isTrue(_.isEqual(d, expectedView));
});

applicationManager.on("debuggableViewFound", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
expectedView.id = "new id";
assert.isTrue(_.isEqual(d, expectedView));
setTimeout(done, 0);
});
applicationManager.on("debuggableViewFound", (appIdentifier: string, d: Mobile.IDebugWebViewInfo) => {
expectedView.id = "new id";
assert.isTrue(_.isEqual(d, expectedView));
setTimeout(done, 0);
});

viewToChange.id = "new id";
}).then(() => applicationManager.checkForApplicationUpdates());
viewToChange.id = "new id";
})
.catch()
.then(() => applicationManager.checkForApplicationUpdates())
.catch();
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/unit-tests/mobile/devices-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ describe("devicesService", () => {
androidDeviceDiscovery.emit(DeviceDiscoveryEventNames.DEVICE_LOST, tempDevice);
iOSDeviceDiscovery.emit(DeviceDiscoveryEventNames.DEVICE_LOST, iOSDevice);
counter = 0;
devicesService.execute(() => { counter++; return Promise.resolve(); }, () => true, { allowNoDevices: true });
await devicesService.execute(() => { counter++; return Promise.resolve(); }, () => true, { allowNoDevices: true });
assert.deepEqual(counter, 0, "The action must not be executed when there are no devices.");
assert.isTrue(logger.output.indexOf(constants.ERROR_NO_DEVICES) !== -1);
});
Expand Down
11 changes: 4 additions & 7 deletions test/unit-tests/mobile/ios-simulator-discovery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,25 +58,22 @@ describe("ios-simulator-discovery", () => {
let expectedDeviceInfo: Mobile.IDeviceInfo = null;

const detectNewSimulatorAttached = async (runningSimulator: any): Promise<Mobile.IiOSSimulator> => {
return new Promise<Mobile.IiOSSimulator>((resolve, reject) => {
return new Promise<Mobile.IiOSSimulator>(async (resolve, reject) => {
currentlyRunningSimulators.push(_.cloneDeep(runningSimulator));
iOSSimulatorDiscovery.once(DeviceDiscoveryEventNames.DEVICE_FOUND, (device: Mobile.IDevice) => {
resolve(device);
});
// no need to await startLookingForDevices, current promise will be resolved after execution of startLookingForDevices is completed.
iOSSimulatorDiscovery.startLookingForDevices();
await iOSSimulatorDiscovery.startLookingForDevices();
});
};

const detectSimulatorDetached = async (simulatorId: string): Promise<Mobile.IiOSSimulator> => {
_.remove(currentlyRunningSimulators, simulator => simulator.id === simulatorId);
return new Promise<Mobile.IDevice>((resolve, reject) => {
return new Promise<Mobile.IDevice>(async (resolve, reject) => {
iOSSimulatorDiscovery.once(DeviceDiscoveryEventNames.DEVICE_LOST, (device: Mobile.IDevice) => {
resolve(device);
});

// no need to await startLookingForDevices, current promise will be resolved after execution of startLookingForDevices is completed.
iOSSimulatorDiscovery.startLookingForDevices();
await iOSSimulatorDiscovery.startLookingForDevices();
});
};

Expand Down
1 change: 1 addition & 0 deletions tslint.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"check-open-brace",
"check-whitespace"
],
"no-floating-promises": true,
"quotemark": [
false,
"double"
Expand Down

0 comments on commit ffb1471

Please sign in to comment.