From 16aae0a937a9c86e648d09d5df43352f76b965d3 Mon Sep 17 00:00:00 2001 From: Goh Jia Hao Date: Thu, 24 May 2018 22:24:09 -0700 Subject: [PATCH] Refactor tests to use async/await --- src/helpers/convertToIcns.test.js | 37 +++++++------- src/infer/inferUserAgent.test.js | 29 ++++------- src/options/asyncConfig.test.js | 7 ++- src/options/fields/icon.test.js | 28 +++++----- src/options/fields/name.test.js | 76 ++++++++++++++-------------- src/options/fields/userAgent.test.js | 8 +-- src/options/optionsMain.test.js | 9 ++-- src/utils/sanitizeFilename.test.js | 2 +- 8 files changed, 90 insertions(+), 106 deletions(-) diff --git a/src/helpers/convertToIcns.test.js b/src/helpers/convertToIcns.test.js index 5f3e664..275cc27 100644 --- a/src/helpers/convertToIcns.test.js +++ b/src/helpers/convertToIcns.test.js @@ -5,35 +5,36 @@ import convertToIcns from './convertToIcns'; // Prerequisite for test: to use OSX with sips, iconutil and imagemagick convert -function testConvertPng(pngName, done) { +function testConvertPng(pngName) { if (os.platform() !== 'darwin') { // Skip png conversion tests, OSX is required - done(); - return; + return Promise.resolve(); } - convertToIcns( - path.join(__dirname, '../../', 'test-resources', pngName), - (error, icnsPath) => { - if (error) { - done(error); - return; - } + return new Promise((resolve, reject) => + convertToIcns( + path.join(__dirname, '../../', 'test-resources', pngName), + (error, icnsPath) => { + if (error) { + reject(error); + return; + } - const stat = fs.statSync(icnsPath); + const stat = fs.statSync(icnsPath); - expect(stat.isFile()).toBe(true); - done(); - }, + expect(stat.isFile()).toBe(true); + resolve(); + }, + ), ); } describe('Get Icon Module', () => { - test('Can convert a rgb png to icns', (done) => { - testConvertPng('iconSample.png', done); + test('Can convert a rgb png to icns', async () => { + await testConvertPng('iconSample.png'); }); - test('Can convert a grey png to icns', (done) => { - testConvertPng('iconSampleGrey.png', done); + test('Can convert a grey png to icns', async () => { + await testConvertPng('iconSampleGrey.png'); }); }); diff --git a/src/infer/inferUserAgent.test.js b/src/infer/inferUserAgent.test.js index 599ea8b..1b2038e 100644 --- a/src/infer/inferUserAgent.test.js +++ b/src/infer/inferUserAgent.test.js @@ -13,36 +13,25 @@ const TEST_RESULT = { }; function testPlatform(platform) { - return inferUserAgent('0.37.1', platform).then((userAgent) => { - expect(userAgent).toBe(TEST_RESULT[platform]); - }); + return expect(inferUserAgent('0.37.1', platform)).resolves.toBe( + TEST_RESULT[platform], + ); } describe('Infer User Agent', () => { - test('Can infer userAgent for all platforms', (done) => { + test('Can infer userAgent for all platforms', async () => { const testPromises = _.keys(TEST_RESULT).map((platform) => testPlatform(platform), ); - Promise.all(testPromises) - .then(() => { - done(); - }) - .catch((error) => { - done(error); - }); + await Promise.all(testPromises); }); - test('Connection error will still get a user agent', (done) => { + test('Connection error will still get a user agent', async () => { jest.setTimeout(6000); const TIMEOUT_URL = 'http://www.google.com:81/'; - inferUserAgent('1.6.7', 'darwin', TIMEOUT_URL) - .then((userAgent) => { - expect(userAgent).toBe( - 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36', - ); - done(); - }) - .catch(done); + await expect(inferUserAgent('1.6.7', 'darwin', TIMEOUT_URL)).resolves.toBe( + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36', + ); }); }); diff --git a/src/options/asyncConfig.test.js b/src/options/asyncConfig.test.js index 1ab906f..11b64f4 100644 --- a/src/options/asyncConfig.test.js +++ b/src/options/asyncConfig.test.js @@ -9,11 +9,10 @@ fields.mockImplementation(() => [ }), ]); -test('it should merge the result of the promise', () => { +test('it should merge the result of the promise', async () => { const param = { another: 'field', someField: 'oldValue' }; const expected = { another: 'field', someField: 'newValue' }; - return asyncConfig(param).then((result) => { - expect(result).toEqual(expected); - }); + const result = await asyncConfig(param); + expect(result).toEqual(expected); }); diff --git a/src/options/fields/icon.test.js b/src/options/fields/icon.test.js index 50c5d77..b65a6af 100644 --- a/src/options/fields/icon.test.js +++ b/src/options/fields/icon.test.js @@ -8,40 +8,36 @@ jest.mock('loglevel'); const mockedResult = 'icon path'; describe('when the icon parameter is passed', () => { - test('it should return the icon parameter', () => { + test('it should return the icon parameter', async () => { expect(inferIcon).toHaveBeenCalledTimes(0); const params = { icon: './icon.png' }; - expect(icon(params)).resolves.toBe(params.icon); + await expect(icon(params)).resolves.toBe(params.icon); }); }); describe('when the icon parameter is not passed', () => { - test('it should call inferIcon', () => { + test('it should call inferIcon', async () => { inferIcon.mockImplementationOnce(() => Promise.resolve(mockedResult)); const params = { targetUrl: 'some url', platform: 'mac' }; - return icon(params).then((result) => { - expect(result).toBe(mockedResult); - expect(inferIcon).toHaveBeenCalledWith(params.targetUrl, params.platform); - }); + const result = await icon(params); + + expect(result).toBe(mockedResult); + expect(inferIcon).toHaveBeenCalledWith(params.targetUrl, params.platform); }); describe('when inferIcon resolves with an error', () => { - test('it should handle the error', () => { + test('it should handle the error', async () => { inferIcon.mockImplementationOnce(() => Promise.reject(new Error('some error')), ); const params = { targetUrl: 'some url', platform: 'mac' }; - return icon(params).then((result) => { - expect(result).toBe(null); - expect(inferIcon).toHaveBeenCalledWith( - params.targetUrl, - params.platform, - ); - expect(log.warn).toHaveBeenCalledTimes(1); - }); + const result = await icon(params); + expect(result).toBe(null); + expect(inferIcon).toHaveBeenCalledWith(params.targetUrl, params.platform); + expect(log.warn).toHaveBeenCalledTimes(1); }); }); }); diff --git a/src/options/fields/name.test.js b/src/options/fields/name.test.js index b668572..d01ff79 100644 --- a/src/options/fields/name.test.js +++ b/src/options/fields/name.test.js @@ -14,16 +14,17 @@ const mockedResult = 'mock name'; describe('well formed name parameters', () => { const params = { name: 'appname', platform: 'something' }; - test('it should not call inferTitle', () => - name(params).then((result) => { - expect(inferTitle).toHaveBeenCalledTimes(0); - expect(result).toBe(params.name); - })); + test('it should not call inferTitle', async () => { + const result = await name(params); - test('it should call sanitize filename', () => - name(params).then((result) => { - expect(sanitizeFilename).toHaveBeenCalledWith(params.platform, result); - })); + expect(inferTitle).toHaveBeenCalledTimes(0); + expect(result).toBe(params.name); + }); + + test('it should call sanitize filename', async () => { + const result = await name(params); + expect(sanitizeFilename).toHaveBeenCalledWith(params.platform, result); + }); }); describe('bad name parameters', () => { @@ -33,19 +34,21 @@ describe('bad name parameters', () => { const params = { targetUrl: 'some url' }; describe('when the name is undefined', () => { - test('it should call inferTitle', () => - name(params).then(() => { - expect(inferTitle).toHaveBeenCalledWith(params.targetUrl); - })); + test('it should call inferTitle', async () => { + await name(params); + expect(inferTitle).toHaveBeenCalledWith(params.targetUrl); + }); }); describe('when the name is an empty string', () => { - test('it should call inferTitle', () => { - const testParams = Object.assign({}, params, { name: '' }); + test('it should call inferTitle', async () => { + const testParams = { + ...params, + name: '', + }; - return name(testParams).then(() => { - expect(inferTitle).toHaveBeenCalledWith(params.targetUrl); - }); + await name(testParams); + expect(inferTitle).toHaveBeenCalledWith(params.targetUrl); }); }); @@ -57,37 +60,34 @@ describe('bad name parameters', () => { describe('handling inferTitle results', () => { const params = { targetUrl: 'some url', name: '', platform: 'something' }; - test('it should return the result from inferTitle', () => { + test('it should return the result from inferTitle', async () => { inferTitle.mockImplementationOnce(() => Promise.resolve(mockedResult)); - return name(params).then((result) => { - expect(result).toBe(mockedResult); + const result = await name(params); + expect(result).toBe(mockedResult); + expect(inferTitle).toHaveBeenCalledWith(params.targetUrl); + }); + + describe('when the returned pageTitle is falsey', () => { + test('it should return the default app name', async () => { + inferTitle.mockImplementationOnce(() => Promise.resolve(null)); + + const result = await name(params); + expect(result).toBe(DEFAULT_APP_NAME); expect(inferTitle).toHaveBeenCalledWith(params.targetUrl); }); }); - describe('when the returned pageTitle is falsey', () => { - test('it should return the default app name', () => { - inferTitle.mockImplementationOnce(() => Promise.resolve(null)); - - return name(params).then((result) => { - expect(result).toBe(DEFAULT_APP_NAME); - expect(inferTitle).toHaveBeenCalledWith(params.targetUrl); - }); - }); - }); - describe('when inferTitle resolves with an error', () => { - test('it should return the default app name', () => { + test('it should return the default app name', async () => { inferTitle.mockImplementationOnce(() => Promise.reject(new Error('some error')), ); - return name(params).then((result) => { - expect(result).toBe(DEFAULT_APP_NAME); - expect(inferTitle).toHaveBeenCalledWith(params.targetUrl); - expect(log.warn).toHaveBeenCalledTimes(1); - }); + const result = await name(params); + expect(result).toBe(DEFAULT_APP_NAME); + expect(inferTitle).toHaveBeenCalledWith(params.targetUrl); + expect(log.warn).toHaveBeenCalledTimes(1); }); }); }); diff --git a/src/options/fields/userAgent.test.js b/src/options/fields/userAgent.test.js index 03b3a55..3e9e1cf 100644 --- a/src/options/fields/userAgent.test.js +++ b/src/options/fields/userAgent.test.js @@ -3,16 +3,16 @@ import { inferUserAgent } from './../../infer'; jest.mock('./../../infer/inferUserAgent'); -test('when a userAgent parameter is passed', () => { +test('when a userAgent parameter is passed', async () => { expect(inferUserAgent).toHaveBeenCalledTimes(0); const params = { userAgent: 'valid user agent' }; - expect(userAgent(params)).resolves.toBe(params.userAgent); + await expect(userAgent(params)).resolves.toBe(params.userAgent); }); -test('no userAgent parameter is passed', () => { +test('no userAgent parameter is passed', async () => { const params = { electronVersion: '123', platform: 'mac' }; - userAgent(params); + await userAgent(params); expect(inferUserAgent).toHaveBeenCalledWith( params.electronVersion, params.platform, diff --git a/src/options/optionsMain.test.js b/src/options/optionsMain.test.js index 370955a..345fd41 100644 --- a/src/options/optionsMain.test.js +++ b/src/options/optionsMain.test.js @@ -5,14 +5,13 @@ jest.mock('./asyncConfig'); const mockedAsyncConfig = { some: 'options' }; asyncConfig.mockImplementation(() => Promise.resolve(mockedAsyncConfig)); -test('it should call the async config', () => { +test('it should call the async config', async () => { const params = { targetUrl: 'http://example.com', }; - return optionsMain(params).then((result) => { - expect(asyncConfig).toHaveBeenCalledWith(expect.objectContaining(params)); - expect(result).toEqual(mockedAsyncConfig); - }); + const result = await optionsMain(params); + expect(asyncConfig).toHaveBeenCalledWith(expect.objectContaining(params)); + expect(result).toEqual(mockedAsyncConfig); }); // TODO add more tests diff --git a/src/utils/sanitizeFilename.test.js b/src/utils/sanitizeFilename.test.js index 73f04bc..bbef768 100644 --- a/src/utils/sanitizeFilename.test.js +++ b/src/utils/sanitizeFilename.test.js @@ -13,7 +13,7 @@ test('it should call the sanitize-filename npm module', () => { describe('replacing non ascii characters', () => { const nonAscii = '�'; - test('it should return a result without non ascii cahracters', () => { + test('it should return a result without non ascii characters', () => { const param = `${nonAscii}abc`; const expectedResult = 'abc'; const result = sanitizeFilename('', param);