Skip to content

Commit 8ee2a38

Browse files
mdonnalleymdonnalley
committed
chore: code review
1 parent 079622d commit 8ee2a38

File tree

4 files changed

+54
-73
lines changed

4 files changed

+54
-73
lines changed

‎src/commands/force/org/clone.ts‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,11 @@ export class OrgCloneCommand extends SfCommand<SandboxProcessObject> {
7878
loglevel,
7979
};
8080

81+
private logger!: Logger;
82+
8183
public async run(): Promise<SandboxProcessObject> {
8284
const { flags, args, argv } = await this.parse(OrgCloneCommand);
83-
const logger = await Logger.child(this.constructor.name);
85+
this.logger = await Logger.child(this.constructor.name);
8486
const varargs = parseVarArgs(args, argv as string[]);
8587

8688
const lifecycle = Lifecycle.getInstance();
@@ -104,24 +106,19 @@ export class OrgCloneCommand extends SfCommand<SandboxProcessObject> {
104106
});
105107

106108
if (results?.sandboxRes?.authUserName) {
107-
if (flags.setalias) {
108-
const stateAggregator = await StateAggregator.getInstance();
109-
stateAggregator.aliases.set(flags.setalias, results.sandboxRes.authUserName);
110-
const result = stateAggregator.aliases.getAll();
111-
logger.debug('Set Alias: %s result: %s', flags.setalias, result);
112-
}
113-
if (flags.setdefaultusername) {
114-
const globalConfig = this.configAggregator.getGlobalConfig();
115-
globalConfig.set(OrgConfigProperties.TARGET_ORG, results.sandboxRes.authUserName);
116-
const result = await globalConfig.write();
117-
logger.debug('Set defaultUsername: %s result: %s', flags.setdefaultusername, result);
118-
}
109+
if (flags.setalias) await this.setAlias(flags.setalias, results.sandboxRes.authUserName);
110+
if (flags.setdefaultusername) await this.setDefaultUsername(results.sandboxRes.authUserName);
119111
}
120112
});
121113

122-
const { sandboxReq, srcSandboxName } = await createSandboxRequest(true, flags.definitionfile, logger, varargs);
114+
const { sandboxReq, srcSandboxName } = await createSandboxRequest(
115+
true,
116+
flags.definitionfile,
117+
this.logger,
118+
varargs
119+
);
123120

124-
logger.debug('Calling clone with SandboxRequest: %s and SandboxName: %s ', sandboxReq, srcSandboxName);
121+
this.logger.debug('Calling clone with SandboxRequest: %s and SandboxName: %s ', sandboxReq, srcSandboxName);
125122
flags['target-org'].getConnection(flags['api-version']);
126123
return flags['target-org'].cloneSandbox(sandboxReq, srcSandboxName, { wait: flags.wait });
127124
} else {
@@ -131,4 +128,18 @@ export class OrgCloneCommand extends SfCommand<SandboxProcessObject> {
131128
);
132129
}
133130
}
131+
132+
public async setAlias(alias: string, username: string): Promise<void> {
133+
const stateAggregator = await StateAggregator.getInstance();
134+
stateAggregator.aliases.set(alias, username);
135+
const result = stateAggregator.aliases.getAll();
136+
this.logger.debug('Set Alias: %s result: %s', alias, result);
137+
}
138+
139+
public async setDefaultUsername(username: string): Promise<void> {
140+
const globalConfig = this.configAggregator.getGlobalConfig();
141+
globalConfig.set(OrgConfigProperties.TARGET_ORG, username);
142+
const result = await globalConfig.write();
143+
this.logger.debug('Set defaultUsername: %s result: %s', username, result);
144+
}
134145
}

‎test/unit/force/org/clone.test.ts‎

Lines changed: 20 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,9 @@
55
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
66
*/
77
import * as fs from 'fs';
8-
import {
9-
Org,
10-
Lifecycle,
11-
StateAggregator,
12-
SandboxEvents,
13-
// SfdxConfigAggregator,
14-
// OrgConfigProperties,
15-
// ConfigAggregator,
16-
SandboxProcessObject,
17-
Logger,
18-
} from '@salesforce/core';
8+
import { Org, Lifecycle, SandboxEvents, SandboxProcessObject, Logger, SfError } from '@salesforce/core';
199
import { stubMethod } from '@salesforce/ts-sinon';
20-
import { TestContext } from '@salesforce/core/lib/testSetup';
10+
import { shouldThrow, TestContext } from '@salesforce/core/lib/testSetup';
2111
import { stubSfCommandUx, stubUx } from '@salesforce/sf-plugins-core';
2212
import { assert, expect, config } from 'chai';
2313
import * as sinon from 'sinon';
@@ -84,30 +74,17 @@ describe('org:clone', () => {
8474

8575
// stubs
8676
let requestStub: sinon.SinonStub;
87-
// let aliasSetStub: sinon.SinonSpy;
88-
// let configSetStub: sinon.SinonStub;
89-
// let configWriteStub: sinon.SinonStub;
9077
let onStub: sinon.SinonStub;
91-
// let readJsonDefFileStub: sinon.SinonStub;
9278
let cloneSandboxStub: sinon.SinonStub;
93-
// let configAggregatorStub;
9479
let sfCommandUxStubs: ReturnType<typeof stubSfCommandUx>;
80+
let setAliasStub: sinon.SinonStub;
81+
let setDefaultUsernameStub: sinon.SinonStub;
9582

9683
const runCloneCommand = async (params: string[], fails?: boolean): Promise<SandboxProcessObject> => {
9784
cloneSandboxStub = fails
9885
? $$.SANDBOX.stub(Org.prototype, 'cloneSandbox').rejects(new Error('MyError'))
9986
: $$.SANDBOX.stub(Org.prototype, 'cloneSandbox').resolves(sandboxProcessObj);
10087

101-
// configSetStub = $$.SANDBOX.stub().returns(true);
102-
// configWriteStub = $$.SANDBOX.stub().resolves(true);
103-
// const configAggregatorStubOptions = {
104-
// getGlobalConfig: () => ({
105-
// set: configSetStub,
106-
// write: configWriteStub,
107-
// }),
108-
// };
109-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
110-
// cmd.setConfigAggregator(configAggregatorStub);
11188
if (!fails) {
11289
onStub = $$.SANDBOX.stub()
11390
.callsArgWith(1, sandboxProcessObj)
@@ -124,6 +101,9 @@ describe('org:clone', () => {
124101
onWarning: $$.SANDBOX.stub(),
125102
});
126103

104+
setAliasStub = $$.SANDBOX.stub(OrgCloneCommand.prototype, 'setAlias').resolves();
105+
setDefaultUsernameStub = $$.SANDBOX.stub(OrgCloneCommand.prototype, 'setDefaultUsername').resolves();
106+
127107
return OrgCloneCommand.run(params);
128108
};
129109

@@ -159,10 +139,11 @@ describe('org:clone', () => {
159139

160140
expect(sfCommandUxStubs.table.firstCall.args[0].length).to.be.equal(12);
161141
expect(sfCommandUxStubs.log.callCount).to.be.equal(3);
162-
const stateAggregator = await StateAggregator.getInstance();
163-
expect(stateAggregator.aliases.getAll()).to.deep.equal({});
164-
// expect(configSetStub.callCount).to.be.equal(0);
165-
// expect(configWriteStub.callCount).to.be.equal(0);
142+
143+
// Alias and default username should not be set.
144+
expect(setAliasStub.callCount).to.equal(0);
145+
expect(setDefaultUsernameStub.callCount).to.equal(0);
146+
166147
expect(onStub.firstCall.firstArg).to.be.equal(SandboxEvents.EVENT_ASYNC_RESULT);
167148
expect(onStub.secondCall.firstArg).to.be.equal(SandboxEvents.EVENT_STATUS);
168149
expect(onStub.thirdCall.firstArg).to.be.equal(SandboxEvents.EVENT_RESULT);
@@ -185,19 +166,15 @@ describe('org:clone', () => {
185166
expect(sfCommandUxStubs.table.firstCall.args[0].length).to.be.equal(12);
186167
expect(sfCommandUxStubs.log.callCount).to.be.equal(3);
187168

188-
const stateAggregator = await StateAggregator.getInstance();
189-
expect(stateAggregator.aliases.getAll()).to.deep.equal({ [sandboxAlias]: authUserName });
169+
expect(setAliasStub.firstCall.args).to.deep.equal([sandboxAlias, authUserName]);
190170

191-
expect($$.stubs.configWrite.callCount).to.equal(2);
192171
expect(sfCommandUxStubs.styledHeader.callCount).to.equal(1);
193172
expect(sfCommandUxStubs.table.firstCall.args[0].length).to.be.equal(12);
194173
expect(sfCommandUxStubs.log.callCount).to.be.equal(3);
195174

196-
// expect($$.configAggregator?.getGlobalConfig()?.get(OrgConfigProperties.TARGET_ORG)).to.equal(authUserName);
197-
// expect(configSetStub.callCount).to.be.equal(1);
175+
expect(setDefaultUsernameStub.calledOnce).to.be.true;
176+
expect(setDefaultUsernameStub.firstCall.args[0]).to.be.equal(authUserName);
198177

199-
// expect(configSetStub.firstCall.args[0]).to.be.equal(OrgConfigProperties.TARGET_ORG);
200-
// expect(configSetStub.firstCall.args[1]).to.be.equal(authUserName);
201178
expect(onStub.firstCall.firstArg).to.be.equal(SandboxEvents.EVENT_ASYNC_RESULT);
202179
expect(onStub.secondCall.firstArg).to.be.equal(SandboxEvents.EVENT_STATUS);
203180
expect(onStub.thirdCall.firstArg).to.be.equal(SandboxEvents.EVENT_RESULT);
@@ -206,7 +183,6 @@ describe('org:clone', () => {
206183
authUserName,
207184
SandboxName: sandboxName,
208185
});
209-
// expect(configWriteStub.calledOnce).to.be.true;
210186
expect(res).to.deep.equal(sandboxProcessObj);
211187
});
212188

@@ -216,21 +192,15 @@ describe('org:clone', () => {
216192
srcSandboxName: 'TheOriginal',
217193
});
218194
try {
219-
// await shouldThrow(
220-
// runCloneCommand(['-t', 'sandbox', '-u', 'DevHub', '-f', 'sandbox-def.json', '-a', sandboxAlias, '-s'], true)
221-
// );
222-
await runCloneCommand(
223-
['-t', 'sandbox', '-u', 'DevHub', '-f', 'sandbox-def.json', '-a', sandboxAlias, '-s'],
224-
true
195+
await shouldThrow(
196+
runCloneCommand(['-t', 'sandbox', '-u', 'DevHub', '-f', 'sandbox-def.json', '-a', sandboxAlias, '-s'], true)
225197
);
226198
} catch (error) {
227-
const e = error as Error;
228-
expect(e.name === 'MyError');
199+
assert(error instanceof SfError, 'should be an SfError');
200+
expect(error.name === 'MyError');
229201
expect(sfCommandUxStubs.styledHeader.calledOnce).to.be.false;
230202
expect(sfCommandUxStubs.table.calledOnce).to.be.false;
231-
// expect(configSetStub.callCount).to.be.equal(0);
232-
const stateAggregator = await StateAggregator.getInstance();
233-
expect(stateAggregator.aliases.getAll()).to.deep.equal({});
203+
expect(setAliasStub.callCount).to.equal(0);
234204

235205
expect(onStub.firstCall.firstArg).to.be.equal(SandboxEvents.EVENT_ASYNC_RESULT);
236206
expect(onStub.secondCall.firstArg).to.be.equal(SandboxEvents.EVENT_STATUS);
@@ -240,7 +210,6 @@ describe('org:clone', () => {
240210
// license type is not set in sandbox cloning
241211
SandboxName: sandboxName,
242212
});
243-
// expect(configWriteStub.calledOnce).to.be.false;
244213
}
245214
});
246215
});

‎test/unit/force/org/sandboxCreate.test.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
import { stubMethod } from '@salesforce/ts-sinon';
2020
import * as sinon from 'sinon';
2121
import { MockTestOrgData, shouldThrow, TestContext } from '@salesforce/core/lib/testSetup';
22-
import { expect, config } from 'chai';
22+
import { expect, config, assert } from 'chai';
2323
import { stubSfCommandUx } from '@salesforce/sf-plugins-core';
2424
import { Create } from '../../../../src/commands/force/org/create';
2525
import * as requestFunctions from '../../../../src/shared/sandboxRequest';
@@ -91,7 +91,8 @@ describe('org:create (sandbox paths)', () => {
9191
try {
9292
await shouldThrow(Create.run(['--type', 'sandbox', '--retry', '1', '-u', testOrg.username]));
9393
} catch (e) {
94-
expect((e as Error).name).to.equal('retryIsNotValidForSandboxes');
94+
assert(e instanceof SfError, 'Expect error to be an instance of SfError');
95+
expect(e.name).to.equal('retryIsNotValidForSandboxes');
9596
expect(createSandboxStub.callCount).to.equal(0);
9697
}
9798
});
@@ -176,9 +177,9 @@ describe('org:create (sandbox paths)', () => {
176177
await shouldThrow(Create.run(['--type', 'sandbox', '-u', testOrg.username]));
177178
} catch (err) {
178179
// shouldThrow doesn't necessarily throw an SfError
179-
const e = err as SfError;
180-
expect(e.code).to.equal(68);
181-
expect(e.actions).to.deep.equal([messages.getMessage('dnsTimeout'), messages.getMessage('partialSuccess')]);
180+
assert(err instanceof SfError, 'Expect error to be an instance of SfError');
181+
expect(err.code).to.equal(68);
182+
expect(err.actions).to.deep.equal([messages.getMessage('dnsTimeout'), messages.getMessage('partialSuccess')]);
182183
}
183184
});
184185
});

‎test/unit/org/open.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ describe('org:open', () => {
103103
} catch (e) {
104104
expect(spies.get('resolver').callCount).to.equal(1);
105105
expect(spies.get('open').callCount).to.equal(0);
106-
const error = e as Error;
107-
expect(error.message).to.equal(messages.getMessage('domainTimeoutError'));
106+
assert(e instanceof SfError, 'should be an SfError');
107+
expect(e.message).to.equal(messages.getMessage('domainTimeoutError'));
108108
}
109109
});
110110
});

0 commit comments

Comments
 (0)