From 1f5d73bb44e2dd6f30a11bc98517b46e1d5f19f4 Mon Sep 17 00:00:00 2001 From: Emmanuel Ogbizi Date: Fri, 5 Apr 2019 21:06:53 -0400 Subject: [PATCH] fix: commands break inelegantly without internet connection (#352) * wip: handle undefined response * wip: handle get version tags error * wip: messaging and exit code for list remote * wip: move process exit to command action * test: update logic * test: util functions * test: list remote command * fix: missing Makefile node call --- Makefile | 2 +- src/commands/listRemote.js | 8 +- src/util/utils.js | 16 ++- src/util/version.js | 3 +- src/yvm.js | 14 +- test/commands/listRemote.test.js | 27 ++-- test/util/__snapshots__/utils.test.js.snap | 8 +- test/util/utils.test.js | 148 ++++++++++++++++++++- 8 files changed, 199 insertions(+), 27 deletions(-) diff --git a/Makefile b/Makefile index 0fb4b210..70d54dec 100644 --- a/Makefile +++ b/Makefile @@ -56,7 +56,7 @@ help: .PHONY: install install: build-production - @USE_LOCAL=true scripts/install.js + @USE_LOCAL=true node scripts/install.js .PHONY: install-watch install-watch: node_modules diff --git a/src/commands/listRemote.js b/src/commands/listRemote.js index ee4bac4b..b43e5949 100644 --- a/src/commands/listRemote.js +++ b/src/commands/listRemote.js @@ -12,13 +12,19 @@ export const listRemote = async () => { const [remoteVersions, versionInUse, localVersions] = await Promise.all( [getVersionsFromTags(), getVersionInUse(), getYarnVersions()], ) + if (!remoteVersions.length) { + throw new Error('No versions available for install') + } await printVersions({ list: remoteVersions, message: 'Versions available for install:', versionInUse, localVersions, }) + return 0 } catch (e) { - log.error(e) + log(e.message) + log.info(e.stack) + return 1 } } diff --git a/src/util/utils.js b/src/util/utils.js index 672cc346..0efd26bd 100644 --- a/src/util/utils.js +++ b/src/util/utils.js @@ -27,12 +27,13 @@ export const getRequest = memoize(async url => { } return new Promise((resolve, reject) => { request.get(options, (error, response, body) => { - if (error || response.statusCode !== 200) { - if (response.body) { + const { statusCode, body: responseBody } = response || {} + if (error || statusCode !== 200) { + if (responseBody) { if (error) { log(error) } - return reject(response.body) + return reject(responseBody) } return reject(error) } @@ -57,5 +58,12 @@ export const getReleasesFromTags = memoize(async () => { }) export const getVersionsFromTags = memoize(async () => { - return Object.keys(await getReleasesFromTags()) + try { + return Object.keys(await getReleasesFromTags()) + } catch (e) { + log.error( + 'Unable to retrieve remote versions. Please check your network connection', + ) + return [] + } }) diff --git a/src/util/version.js b/src/util/version.js index 9e5f6ec0..bef02419 100644 --- a/src/util/version.js +++ b/src/util/version.js @@ -137,11 +137,10 @@ export const getSplitVersionAndArgs = async (maybeVersionArg, ...rest) => { let versionToUse try { if (maybeVersionArg) { - log.info(`Attempting to resolve ${maybeVersionArg}.`) + log.info(`Attempting to resolve ${maybeVersionArg}`) const parsedVersionString = await resolveVersion({ versionString: maybeVersionArg, }).catch(e => { - log.info(e.message) log.info(e.stack) }) if (parsedVersionString) { diff --git a/src/yvm.js b/src/yvm.js index f65fe8fb..8dac96e7 100644 --- a/src/yvm.js +++ b/src/yvm.js @@ -103,7 +103,12 @@ argParser .action(async () => { log.info('Checking for installed yarn versions...') const { listVersions } = await import('./commands/list') - listVersions() + try { + await listVersions() + } catch (e) { + log(e.message) + process.exit(2) + } }) argParser @@ -113,7 +118,12 @@ argParser .action(async () => { log.info('Checking for available yarn versions...') const { listRemote } = await import('./commands/listRemote') - listRemote() + try { + process.exit(await listRemote()) + } catch (e) { + log(e.message) + process.exit(2) + } }) argParser.command('alias []', 'Show all aliases matching ') diff --git a/test/commands/listRemote.test.js b/test/commands/listRemote.test.js index e7b5dade..078b5596 100644 --- a/test/commands/listRemote.test.js +++ b/test/commands/listRemote.test.js @@ -13,9 +13,7 @@ jest.mock('../../src/util/version', () => { getYarnVersions: jest.fn().mockImplementation(() => { return MOCK_LOCAL_VERSIONS }), - printVersions: jest.fn().mockImplementation(() => { - throw new Error('mock error') - }), + printVersions: jest.fn(), } }) jest.mock('../../src/util/utils', () => { @@ -28,8 +26,14 @@ jest.mock('../../src/util/utils', () => { }) describe('yvm list-remote', () => { + beforeAll(() => { + jest.spyOn(log, 'default') + }) + beforeEach(jest.clearAllMocks) + afterAll(jest.restoreAllMocks) + it('lists the response from the API', async () => { - await listRemote() + expect(await listRemote()).toEqual(0) expect(getVersionInUse).toHaveBeenCalled() expect(getVersionsFromTags).toHaveBeenCalled() const callArgs = printVersions.mock.calls[0][0] @@ -39,13 +43,16 @@ describe('yvm list-remote', () => { }) it('prints error if any fail', async () => { - jest.spyOn(log, 'error') - await listRemote() - expect(log.error).toHaveBeenCalledWith(new Error('mock error')) - log.error.mockRestore() + printVersions.mockRejectedValue(new Error('mock error')) + expect(await listRemote()).toEqual(1) + expect(log.default).toHaveBeenCalledWith('mock error') }) - afterAll(() => { - jest.clearMocks() + it('prints error when no remote version available', async () => { + getVersionsFromTags.mockResolvedValue([]) + expect(await listRemote()).toEqual(1) + expect(log.default).toHaveBeenCalledWith( + expect.stringContaining('No versions available'), + ) }) }) diff --git a/test/util/__snapshots__/utils.test.js.snap b/test/util/__snapshots__/utils.test.js.snap index 0902284b..e6c2d54f 100644 --- a/test/util/__snapshots__/utils.test.js.snap +++ b/test/util/__snapshots__/utils.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Util functions Highlights all installed versions 1`] = ` +exports[`printVersions Highlights all installed versions 1`] = ` Array [ Array [ "highlight installed versions", @@ -23,7 +23,7 @@ Array [ ] `; -exports[`Util functions Highlights the default version 1`] = ` +exports[`printVersions Highlights the default version 1`] = ` Array [ Array [ "highlight default version", @@ -46,7 +46,7 @@ Array [ ] `; -exports[`Util functions Highlights the version currently in use 1`] = ` +exports[`printVersions Highlights the version currently in use 1`] = ` Array [ Array [ "highlight current version", @@ -69,7 +69,7 @@ Array [ ] `; -exports[`Util functions Prints all versions passed to printVersion function 1`] = ` +exports[`printVersions Prints all versions passed to printVersion function 1`] = ` Array [ Array [ "print all versions", diff --git a/test/util/utils.test.js b/test/util/utils.test.js index 1c6f39ee..71ac5b0e 100644 --- a/test/util/utils.test.js +++ b/test/util/utils.test.js @@ -1,3 +1,5 @@ +import request from 'request' + import log from '../../src/util/log' import { DEFAULT_VERSION_TEXT, @@ -5,19 +7,159 @@ import { VERSION_INSTALLED_SYMBOL, printVersions, } from '../../src/util/version' +import { + getReleasesFromTags, + getRequest, + getVersionsFromTags, +} from '../../src/util/utils' const versionInUse = '1.2.0' const defaultVersion = '1.3.0' const localVersions = ['1.1.0', versionInUse, defaultVersion] const versions = [...localVersions, '1.4.0', '1.5.0'] -describe('Util functions', () => { +beforeAll(() => { jest.spyOn(log, 'default') +}) +beforeEach(jest.clearAllMocks) +afterAll(jest.restoreAllMocks) + +describe('getRequest', () => { + beforeAll(() => { + jest.spyOn(request, 'get') + }) + afterAll(() => { + request.get.mockRestore() + }) + + it('calls request with correct options', async () => { + const url = 'https://some.url' + await getRequest(url).catch(() => {}) + expect(request.get).toHaveBeenCalledWith( + expect.objectContaining({ + url, + gzip: true, + headers: { + 'User-Agent': 'YVM', + }, + }), + expect.any(Function), + ) + }) + + it('resolves with body if there is no error', async () => { + const mockResponseBody = 'mock body' + request.get.mockImplementationOnce((_, callback) => { + callback(null, { statusCode: 200 }, mockResponseBody) + }) + expect(await getRequest('https://any.url')).toEqual(mockResponseBody) + }) + + it('rejects with error if there is one', async () => { + const mockError = new Error('mock error') + request.get.mockImplementationOnce((_, callback) => { + callback(mockError, null, null) + }) + try { + await getRequest('https://invalid.url') + } catch (rejection) { + expect(rejection).toBe(mockError) + } + }) + + it('rejects with response body', async () => { + const mockResponseBody = 'mock response body' + request.get.mockImplementationOnce((_, callback) => { + callback(null, { statusCode: 400, body: mockResponseBody }) + }) + try { + await getRequest('https://wrong.url') + } catch (rejection) { + expect(log.default).not.toHaveBeenCalled() + expect(rejection).toBe(mockResponseBody) + } + }) + + it('rejects with response body and logs error', async () => { + const mockError = new Error('mock error') + const mockResponseBody = 'mock response body' + request.get.mockImplementationOnce((_, callback) => { + callback(mockError, { statusCode: 400, body: mockResponseBody }) + }) + try { + await getRequest('https://wrong.invalid.url') + } catch (rejection) { + expect(log.default).toHaveBeenCalledWith(mockError) + expect(rejection).toBe(mockResponseBody) + } + }) +}) + +describe('getVersionsFromTags', () => { + beforeAll(() => { + jest.spyOn(log, 'error') + jest.spyOn(request, 'get') + }) + beforeEach(() => { + getRequest.cache.clear() + getReleasesFromTags.cache.clear() + getVersionsFromTags.cache.clear() + }) + + it('gets version from tags', async () => { + request.get.mockImplementationOnce((_, callback) => { + const body = [ + { + name: 'v1.15.2', + zipball_url: + 'https://api.github.com/repos/yarnpkg/yarn/zipball/v1.15.2', + tarball_url: + 'https://api.github.com/repos/yarnpkg/yarn/tarball/v1.15.2', + commit: { + sha: '6065f7ac8f7cec2b2c35094788117be0deae2f37', + url: + 'https://api.github.com/repos/yarnpkg/yarn/commits/6065f7ac8f7cec2b2c35094788117be0deae2f37', + }, + node_id: 'MDM6UmVmNDk5NzA2NDI6djEuMTUuMg==', + }, + { + name: 'v0.9.8', + zipball_url: + 'https://api.github.com/repos/yarnpkg/yarn/zipball/v0.9.8', + tarball_url: + 'https://api.github.com/repos/yarnpkg/yarn/tarball/v0.9.8', + commit: { + sha: 'mocksha', + url: + 'https://api.github.com/repos/yarnpkg/yarn/commits/mockcommit', + }, + node_id: 'mocknodeid==', + }, + ] + callback(null, { statusCode: 200 }, JSON.stringify(body)) + }) + expect(await getVersionsFromTags()).toEqual(['1.15.2']) + }) + + it('logs error and returns empty on failure', async () => { + const mockError = new Error('mock error') + request.get.mockImplementationOnce((_, callback) => { + callback(mockError) + }) + expect(await getVersionsFromTags()).toEqual([]) + expect(log.error).toHaveBeenCalledWith( + expect.stringContaining('Unable to retrieve remote versions'), + ) + expect(log.error).toHaveBeenCalledWith( + expect.stringContaining('Please check your network connection'), + ) + }) +}) + +describe('printVersions', () => { afterEach(() => { jest.resetAllMocks() }) - afterAll(jest.restoreAllMocks) - it('Prints all versions passed to printVersion function', async () => { const versionsObject = await printVersions({ message: 'print all versions',