Skip to content

Commit 36578aa

Browse files
waldekmastykarzwaldekmastykarz
committed
Fixes bug in 'spo tenant appcatalog add'. Closes #2129 
1 parent 7d5d60e commit 36578aa

4 files changed

Lines changed: 66 additions & 21 deletions

File tree

‎src/cli/Cli.spec.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as markshell from 'markshell';
66
import * as os from 'os';
77
import * as path from 'path';
88
import * as sinon from 'sinon';
9-
import { Cli } from '.';
9+
import { Cli, CommandOutput } from '.';
1010
import appInsights from '../appInsights';
1111
import Command, { CommandArgs, CommandError, CommandOption, CommandTypes } from '../Command';
1212
import AnonymousCommand from '../m365/base/AnonymousCommand';
@@ -571,9 +571,9 @@ describe('Cli', () => {
571571
const commandWithOutput: MockCommandWithOutput = new MockCommandWithOutput();
572572
Cli
573573
.executeCommandWithOutput(commandWithOutput, { options: { _: [] } })
574-
.then((output: string) => {
574+
.then((output: CommandOutput) => {
575575
try {
576-
assert.strictEqual(output, 'Command output');
576+
assert.strictEqual(output.stdout, 'Command output');
577577
done();
578578
}
579579
catch (e) {
@@ -586,9 +586,9 @@ describe('Cli', () => {
586586
const commandWithOutput: MockCommandWithRawOutput = new MockCommandWithRawOutput();
587587
Cli
588588
.executeCommandWithOutput(commandWithOutput, { options: { _: [] } })
589-
.then((output: string) => {
589+
.then((output: CommandOutput) => {
590590
try {
591-
assert.strictEqual(output, 'Raw output');
591+
assert.strictEqual(output.stdout, 'Raw output');
592592
done();
593593
}
594594
catch (e) {
@@ -601,9 +601,10 @@ describe('Cli', () => {
601601
const commandWithOutput: MockCommandWithRawOutput = new MockCommandWithRawOutput();
602602
Cli
603603
.executeCommandWithOutput(commandWithOutput, { options: { _: [], debug: true } })
604-
.then((output: string) => {
604+
.then((output: CommandOutput) => {
605605
try {
606-
assert.strictEqual(output, 'Debug output,Raw output');
606+
assert.strictEqual(output.stdout, 'Raw output');
607+
assert.strictEqual(output.stderr, 'Debug output');
607608
done();
608609
}
609610
catch (e) {
@@ -668,7 +669,7 @@ describe('Cli', () => {
668669
done('Command succeeded while expected fail');
669670
}, e => {
670671
try {
671-
assert.strictEqual(e, 'Error');
672+
assert.strictEqual(e.error, 'Error');
672673
done();
673674
}
674675
catch (e) {

‎src/cli/Cli.ts‎

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,16 @@ import { CommandOptionInfo } from './CommandOptionInfo';
1313
import Utils from '../Utils';
1414
const packageJSON = require('../../package.json');
1515

16+
export interface CommandOutput {
17+
stdout: string;
18+
stderr: string;
19+
}
20+
21+
export interface CommandErrorWithOutput {
22+
error: CommandError;
23+
stderr: string;
24+
}
25+
1626
export class Cli {
1727
public commands: CommandInfo[] = [];
1828
/**
@@ -163,8 +173,17 @@ export class Cli {
163173
logger.logToStderr(`Executing command ${command.name} with options ${JSON.stringify(args)}`);
164174
}
165175

176+
// store the current command name, if any and set the name to the name of
177+
// the command to execute
178+
const cli = Cli.getInstance();
179+
const parentCommandName: string | undefined = cli.currentCommandName;
180+
cli.currentCommandName = command.getCommandName();
181+
166182
command
167183
.action(logger, args as any, (err: any): void => {
184+
// restore the original command name
185+
cli.currentCommandName = parentCommandName;
186+
168187
if (err) {
169188
return reject(err);
170189
}
@@ -174,31 +193,47 @@ export class Cli {
174193
});
175194
}
176195

177-
public static executeCommandWithOutput(command: Command, args: { options: minimist.ParsedArgs }): Promise<string> {
178-
return new Promise((resolve: (result: string) => void, reject: (error: any) => void): void => {
196+
public static executeCommandWithOutput(command: Command, args: { options: minimist.ParsedArgs }): Promise<CommandOutput> {
197+
return new Promise((resolve: (result: CommandOutput) => void, reject: (error: any) => void): void => {
179198
const log: string[] = [];
199+
const logErr: string[] = [];
180200
const logger: Logger = {
181201
log: (message: any): void => {
182-
log.push(message);
202+
log.push(Cli.formatOutput(message, args.options));
183203
},
184204
logRaw: (message: any): void => {
185-
log.push(message);
205+
log.push(Cli.formatOutput(message, args.options));
186206
},
187207
logToStderr: (message: any): void => {
188-
log.push(message);
208+
logErr.push(message);
189209
}
190210
};
191211

192212
if (args.options.debug) {
193213
Cli.log(`Executing command ${command.name} with options ${JSON.stringify(args)}`);
194214
}
195215

216+
// store the current command name, if any and set the name to the name of
217+
// the command to execute
218+
const cli = Cli.getInstance();
219+
const parentCommandName: string | undefined = cli.currentCommandName;
220+
cli.currentCommandName = command.getCommandName();
221+
196222
command.action(logger, args as any, (err: any): void => {
223+
// restore the original command name
224+
cli.currentCommandName = parentCommandName;
225+
197226
if (err) {
198-
return reject(err);
227+
return reject({
228+
error: err,
229+
stderr: logErr.join(os.EOL)
230+
});
199231
}
200232

201-
resolve(log.join());
233+
resolve({
234+
stdout: log.join(os.EOL),
235+
stderr: logErr.join(os.EOL)
236+
});
202237
});
203238
});
204239
}

‎src/m365/spo/commands/tenant/tenant-appcatalog-add.spec.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ describe(commands.TENANT_APPCATALOG_ADD, () => {
132132
});
133133
sinon.stub(Cli, 'executeCommandWithOutput').callsFake((command, args): Promise<any> => {
134134
if (command === spoTenantAppCatalogUrlGetCommand) {
135-
return Promise.resolve('https://contoso.sharepoint.com/sites/old-app-catalog');
135+
return Promise.resolve({
136+
stdout: 'https://contoso.sharepoint.com/sites/old-app-catalog'
137+
});
136138
}
137139

138140
if (command === spoSiteGetCommand) {
@@ -460,7 +462,9 @@ describe(commands.TENANT_APPCATALOG_ADD, () => {
460462
});
461463
sinon.stub(Cli, 'executeCommandWithOutput').callsFake((command, args): Promise<any> => {
462464
if (command === spoTenantAppCatalogUrlGetCommand) {
463-
return Promise.resolve('https://contoso.sharepoint.com/sites/old-app-catalog');
465+
return Promise.resolve({
466+
stdout: 'https://contoso.sharepoint.com/sites/old-app-catalog'
467+
});
464468
}
465469

466470
if (command === spoSiteGetCommand) {
@@ -488,7 +492,9 @@ describe(commands.TENANT_APPCATALOG_ADD, () => {
488492
it('handles error app catalog exists and no force used', (done) => {
489493
sinon.stub(Cli, 'executeCommandWithOutput').callsFake((command, args): Promise<any> => {
490494
if (command === spoTenantAppCatalogUrlGetCommand) {
491-
return Promise.resolve('https://contoso.sharepoint.com/sites/old-app-catalog');
495+
return Promise.resolve({
496+
stdout: 'https://contoso.sharepoint.com/sites/old-app-catalog'
497+
});
492498
}
493499

494500
if (command === spoSiteGetCommand) {
@@ -775,7 +781,9 @@ describe(commands.TENANT_APPCATALOG_ADD, () => {
775781
});
776782
sinon.stub(Cli, 'executeCommandWithOutput').callsFake((command, args): Promise<any> => {
777783
if (command === spoTenantAppCatalogUrlGetCommand) {
778-
return Promise.resolve('https://contoso.sharepoint.com/sites/old-app-catalog');
784+
return Promise.resolve({
785+
stdout: 'https://contoso.sharepoint.com/sites/old-app-catalog'
786+
});
779787
}
780788

781789
if (command === spoSiteGetCommand) {

‎src/m365/spo/commands/tenant/tenant-appcatalog-add.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as chalk from 'chalk';
2-
import { Cli, Logger } from '../../../../cli';
2+
import { Cli, CommandOutput, Logger } from '../../../../cli';
33
import Command, { CommandError, CommandOption } from '../../../../Command';
44
import GlobalOptions from '../../../../GlobalOptions';
55
import SpoCommand from '../../../base/SpoCommand';
@@ -37,7 +37,8 @@ class SpoTenantAppCatalogAddCommand extends SpoCommand {
3737

3838
Cli
3939
.executeCommandWithOutput(spoTenantAppCatalogUrlGetCommand as Command, { options: { _: [] } })
40-
.then((appCatalogUrl: string): Promise<void> => {
40+
.then((spoTenantAppCatalogUrlGetCommandOutput: CommandOutput): Promise<void> => {
41+
const appCatalogUrl: string | undefined = spoTenantAppCatalogUrlGetCommandOutput.stdout;
4142
if (!appCatalogUrl) {
4243
if (this.verbose) {
4344
logger.logToStderr('No app catalog URL found');

0 commit comments

Comments
 (0)