diff --git a/src/cli.ts b/src/cli.ts index a7c8c1525..3446c04c0 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -36,15 +36,9 @@ export class Cli implements ICli { childProcess.exec(cmd, opts, (error: ExecException, stdout: string, stderr: string) => { this.odoChannel.print(stdout); this.odoChannel.print(stderr); - if (error) { - if (error.code === 1 && stderr.trim().length === 0) { - resolve({ error, stdout: stdout.replace(/---[\s\S]*---/g, '').trim(), stderr }); - } else { - reject(error); - } - } else { - resolve({ error, stdout: stdout.replace(/---[\s\S]*---/g, '').trim(), stderr }); - } + // do not reject it here, because caller in some cases need the error and the streams + // to make a decision + resolve({ error, stdout: stdout.replace(/---[\s\S]*---/g, '').trim(), stderr }); }); }); } diff --git a/src/odo.ts b/src/odo.ts index c6d5a364f..85b5a00b3 100644 --- a/src/odo.ts +++ b/src/odo.ts @@ -111,7 +111,7 @@ export interface Odo { getServiceTemplatePlans(svc: string): Promise; getServices(application: OpenShiftObject): Promise; getApplicationChildren(application: OpenShiftObject): Promise; - execute(command: string, cwd?: string): Promise; + execute(command: string, cwd?: string, fail?: boolean): Promise; requireLogin(): Promise; } @@ -207,9 +207,9 @@ export class OdoImpl implements Odo { return clusters; } - public async getClustersWithOc(): Promise { + private async getClustersWithOc(): Promise { let clusters: OpenShiftObject[] = []; - const result: cliInstance.CliExitData = await this.execute(`oc version`); + const result: cliInstance.CliExitData = await this.execute(`oc version`, process.cwd(), false); clusters = result.stdout.trim().split('\n').filter((value) => { return value.indexOf('Server ') !== -1; }).map((value) => { @@ -219,7 +219,7 @@ export class OdoImpl implements Odo { return clusters; } - public async getClustersWithOdo(): Promise { + private async getClustersWithOdo(): Promise { let clusters: OpenShiftObject[] = []; const result: cliInstance.CliExitData = await this.execute( `odo version && odo project list` @@ -294,13 +294,19 @@ export class OdoImpl implements Odo { terminal.show(); } - public async execute(command: string, cwd?: string): Promise { + public async execute(command: string, cwd?: string, fail: boolean = true): Promise { const cmd = command.split(' ')[0]; const toolLocation = await ToolsConfig.detectOrDownload(cmd); return OdoImpl.cli.execute( toolLocation ? command.replace(cmd, `"${toolLocation}"`).replace(new RegExp(`&& ${cmd}`, 'g'), `&& "${toolLocation}"`) : command, cwd ? {cwd} : { } - ); + ).then(async (result) => { + if (result.error && fail) { + return Promise.reject(result.error); + } else { + return result; + } + }); } public async requireLogin(): Promise { diff --git a/src/tools.ts b/src/tools.ts index f1207f3db..7d042cbf6 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -162,18 +162,14 @@ export class ToolsConfig { let detectedVersion: string; if (fs.existsSync(location)) { const version = new RegExp(`${cmd} v([\\d\\.]+)`); - try { - const result = await Cli.getInstance().execute(`"${location}" version`); - if (!result.error) { - const toolVersion: string[] = result.stdout.trim().split('\n').filter((value) => { - return value.match(version); - }).map((value)=>version.exec(value)[1]); - if (toolVersion.length) { - detectedVersion = toolVersion[0]; - } + const result = await Cli.getInstance().execute(`"${location}" version`); + if (result.stdout) { + const toolVersion: string[] = result.stdout.trim().split('\n').filter((value) => { + return value.match(version); + }).map((value)=>version.exec(value)[1]); + if (toolVersion.length) { + detectedVersion = toolVersion[0]; } - } catch (ignore) { - // if `${tool} version` failed, then there is no tool at specified location } } return detectedVersion; diff --git a/src/util/progress.ts b/src/util/progress.ts index b38bf71c6..80a994c0d 100644 --- a/src/util/progress.ts +++ b/src/util/progress.ts @@ -7,6 +7,8 @@ import * as vscode from 'vscode'; import * as odoctl from '../odo'; +import { CliExitData } from '../cli'; +import { ExecException } from 'child_process'; export interface Step { command: string; @@ -17,33 +19,29 @@ export interface Step { export class Progress { static execWithProgress(options, steps: Step[], odo: odoctl.Odo): Thenable { return vscode.window.withProgress(options, - (progress: vscode.Progress<{increment: number, message: string}>, token: vscode.CancellationToken) => { - const calls: (()=>Promise)[] = []; - steps.reduce((previous: Step, current: Step, currentIndex: number, steps: Step[])=> { - calls.push(()=> { - let result: Promise = Promise.resolve(); - progress.report({ - increment: previous.increment, - message: `${previous.total}%` - }); - result = odo.execute(current.command); + (progress: vscode.Progress<{increment: number, message: string}>, token: vscode.CancellationToken) => { + const calls: (()=>Promise)[] = []; + steps.reduce((previous: Step, current: Step, currentIndex: number, steps: Step[])=> { current.total = previous.total + current.increment; - return currentIndex+1 === steps.length ? result.then(()=> { - progress.report({ - increment: previous.increment, - message: `${previous.total}%` - }); - }) : result; - }); - return current; - }, {increment: 0, command: "", total: 0}); + calls.push (() => { + return Promise.resolve() + .then(() => progress.report({increment: previous.increment, message: `${previous.total}%` })) + .then(() => odo.execute(current.command)) + .then(() => { + if (currentIndex+1 === steps.length) { + progress.report({ + increment: current.increment, + message: `${current.total}%` + }); + } + }); + }); + return current; + }, {increment: 0, command: "", total: 0}); - return calls.reduce>((previous: Promise, current: ()=>Promise, index: number, calls: (()=>Promise)[])=> { - return previous.then(current); - }, Promise.resolve()).catch((error) => { - vscode.window.showErrorMessage(`${error}`); - return; + return calls.reduce>((previous: Promise, current: ()=>Promise, index: number, calls: (()=>Promise)[])=> { + return previous.then(current); + }, Promise.resolve()); }); - }); } } \ No newline at end of file diff --git a/test/util/progress.test.ts b/test/util/progress.test.ts index d96011d49..b5bb48597 100644 --- a/test/util/progress.test.ts +++ b/test/util/progress.test.ts @@ -30,7 +30,6 @@ suite('Progress Utility', () => { setup(() => { sandbox = sinon.createSandbox(); - execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves(); }); teardown(() => { @@ -38,6 +37,7 @@ suite('Progress Utility', () => { }); test('calls cli commands in sequence', async () => { + execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves({ error: undefined, stdout: "", stderr: "" }); await Progress.execWithProgress(options, steps, OdoImpl.getInstance()); expect(execStub).calledTwice; @@ -46,17 +46,26 @@ suite('Progress Utility', () => { }); test('calls progress with given options', async () => { + execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves({ error: undefined, stdout: "", stderr: "" }); const spy = sandbox.spy(vscode.window, 'withProgress'); await Progress.execWithProgress(options, steps, OdoImpl.getInstance()); expect(spy).calledOnceWith(options, sinon.match.func); }); - test('shows an error message if a command fails', async () => { - execStub.rejects(errorMessage); + test('throw an error if a command fails', async () => { + const error = new Error(errorMessage); + execStub = sandbox.stub(OdoImpl.prototype, 'execute').rejects(error); const spy = sandbox.spy(vscode.window, 'showErrorMessage'); - await Progress.execWithProgress(options, steps, OdoImpl.getInstance()); - - expect(spy).calledOnceWith(errorMessage); + let e; + try { + await Progress.execWithProgress(options, steps, OdoImpl.getInstance()); + } catch (err) { + e = err; + expect(err.message).equals(errorMessage); + } + if (!e) { + expect.fail('no error thrown'); + } }); }); \ No newline at end of file