-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: add progress bar to deploy #65
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,15 +9,19 @@ import * as os from 'os'; | |
import * as path from 'path'; | ||
import { flags, FlagsConfig } from '@salesforce/command'; | ||
import { Lifecycle, Messages } from '@salesforce/core'; | ||
import { DeployResult } from '@salesforce/source-deploy-retrieve'; | ||
import { DeployResult, MetadataApiDeploy } from '@salesforce/source-deploy-retrieve'; | ||
import { Duration } from '@salesforce/kit'; | ||
import { asString, asArray, getBoolean, JsonCollection } from '@salesforce/ts-types'; | ||
import * as chalk from 'chalk'; | ||
import cli from 'cli-ux'; | ||
import { env } from '@salesforce/kit'; | ||
import { SourceCommand } from '../../../sourceCommand'; | ||
|
||
Messages.importMessagesDirectory(__dirname); | ||
const messages = Messages.loadMessages('@salesforce/plugin-source', 'deploy'); | ||
|
||
type TestLevel = 'NoTestRun' | 'RunSpecifiedTests' | 'RunLocalTests' | 'RunAllTestsInOrg'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a type in SDR. We should ask them to export it so we don't have to redefine it. |
||
|
||
export class Deploy extends SourceCommand { | ||
public static readonly description = messages.getMessage('description'); | ||
public static readonly examples = messages.getMessage('examples').split(os.EOL); | ||
|
@@ -111,19 +115,24 @@ export class Deploy extends SourceCommand { | |
|
||
await hookEmitter.emit('predeploy', { packageXmlPath: cs.getPackageXml() }); | ||
|
||
const results = await cs | ||
.deploy({ | ||
usernameOrConnection: this.org.getUsername(), | ||
apiOptions: { | ||
ignoreWarnings: getBoolean(this.flags, 'ignorewarnings', false), | ||
rollbackOnError: !getBoolean(this.flags, 'ignoreerrors', false), | ||
checkOnly: getBoolean(this.flags, 'checkonly', false), | ||
runTests: asArray<string>(this.flags.runtests), | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
testLevel: this.flags.testlevel, | ||
}, | ||
}) | ||
.start(); | ||
const deploy = cs.deploy({ | ||
usernameOrConnection: this.org.getUsername(), | ||
apiOptions: { | ||
ignoreWarnings: getBoolean(this.flags, 'ignorewarnings', false), | ||
rollbackOnError: !getBoolean(this.flags, 'ignoreerrors', false), | ||
checkOnly: getBoolean(this.flags, 'checkonly', false), | ||
runTests: asArray<string>(this.flags.runtests), | ||
testLevel: this.flags.testlevel as TestLevel, | ||
}, | ||
}); | ||
|
||
// if SFDX_USE_PROGRESS_BAR is true and no --json flag use progress bar, if not, skip | ||
if (env.getBoolean('SFDX_USE_PROGRESS_BAR', true) && !this.flags.json) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought the default was to use the progress bar? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @RodEsp that's what this is. If the env var is unset default to true. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, got'cha. I didn't realize that's what // if SFDX_USE_PROGRESS_BAR is true or not set and no --json flag use progress bar, otherwise skip There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @RodEsp we're going to merge this as is, but we need to move the progress bar logic and I'll update this comment when I do that |
||
this.progress(deploy); | ||
} | ||
|
||
const results = await deploy.start(); | ||
|
||
await hookEmitter.emit('postdeploy', results); | ||
|
||
// skip a lot of steps that would do nothing | ||
|
@@ -134,6 +143,51 @@ export class Deploy extends SourceCommand { | |
return results; | ||
} | ||
|
||
private progress(deploy: MetadataApiDeploy): void { | ||
// cli.progress doesn't have typings | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
const progressBar = cli.progress({ | ||
format: 'SOURCE PROGRESS | {bar} | {value}/{total} Components', | ||
barCompleteChar: '\u2588', | ||
barIncompleteChar: '\u2591', | ||
linewrap: true, | ||
}); | ||
let printOnce = true; | ||
deploy.onUpdate((data) => { | ||
// the numCompTot. isn't computed right away, wait to start until we know how many we have | ||
if (data.numberComponentsTotal && printOnce) { | ||
this.ux.log(`Job ID | ${data.id}`); | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access | ||
progressBar.start(data.numberComponentsTotal + data.numberTestsTotal); | ||
printOnce = false; | ||
} | ||
|
||
// the numTestsTot. isn't computed until validated as tests by the server, update the PB once we know | ||
if (data.numberTestsTotal) { | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access | ||
progressBar.setTotal(data.numberComponentsTotal + data.numberTestsTotal); | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-call | ||
progressBar.update(data.numberComponentsDeployed + data.numberTestsCompleted); | ||
}); | ||
|
||
deploy.onFinish(() => { | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access | ||
progressBar.stop(); | ||
}); | ||
|
||
deploy.onCancel(() => { | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access | ||
progressBar.stop(); | ||
}); | ||
|
||
deploy.onError(() => { | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access | ||
progressBar.stop(); | ||
}); | ||
} | ||
|
||
private printComponentFailures(result: DeployResult): void { | ||
if (result.response.status === 'Failed' && result.components) { | ||
// sort by filename then fullname | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ describe('force:source:deploy', () => { | |
|
||
// Stubs | ||
let createComponentSetStub: sinon.SinonStub; | ||
let progressStub: sinon.SinonStub; | ||
let deployStub: sinon.SinonStub; | ||
let startStub: sinon.SinonStub; | ||
let lifecycleEmitStub: sinon.SinonStub; | ||
|
@@ -44,12 +45,15 @@ describe('force:source:deploy', () => { | |
getUsername: () => username, | ||
}, | ||
createComponentSet: createComponentSetStub, | ||
progress: progressStub, | ||
print: () => {}, | ||
}) as Promise<DeployResult>; | ||
}; | ||
|
||
beforeEach(() => { | ||
startStub = sandbox.stub().returns(stubbedResults); | ||
deployStub = sandbox.stub().returns({ start: startStub }); | ||
progressStub = sandbox.stub(); | ||
createComponentSetStub = sandbox.stub().returns({ | ||
deploy: deployStub, | ||
getPackageXml: () => packageXml, | ||
|
@@ -107,13 +111,18 @@ describe('force:source:deploy', () => { | |
expect(lifecycleEmitStub.secondCall.args[1]).to.deep.equal(stubbedResults); | ||
}; | ||
|
||
const ensureProgressBar = (callCount: number) => { | ||
expect(progressStub.callCount).to.equal(callCount); | ||
}; | ||
|
||
it('should pass along sourcepath', async () => { | ||
const sourcepath = ['somepath']; | ||
const result = await run({ sourcepath, json: true }); | ||
expect(result).to.deep.equal(stubbedResults); | ||
ensureCreateComponentSetArgs({ sourcepath }); | ||
ensureDeployArgs(); | ||
ensureHookArgs(); | ||
ensureProgressBar(0); | ||
}); | ||
|
||
it('should pass along metadata', async () => { | ||
|
@@ -123,6 +132,7 @@ describe('force:source:deploy', () => { | |
ensureCreateComponentSetArgs({ metadata }); | ||
ensureDeployArgs(); | ||
ensureHookArgs(); | ||
ensureProgressBar(0); | ||
}); | ||
|
||
it('should pass along manifest', async () => { | ||
|
@@ -132,6 +142,7 @@ describe('force:source:deploy', () => { | |
ensureCreateComponentSetArgs({ manifest }); | ||
ensureDeployArgs(); | ||
ensureHookArgs(); | ||
ensureProgressBar(0); | ||
}); | ||
|
||
it('should pass along apiversion', async () => { | ||
|
@@ -142,6 +153,7 @@ describe('force:source:deploy', () => { | |
ensureCreateComponentSetArgs({ apiversion, manifest }); | ||
ensureDeployArgs(); | ||
ensureHookArgs(); | ||
ensureProgressBar(0); | ||
}); | ||
|
||
it('should pass along all deploy options', async () => { | ||
|
@@ -170,5 +182,42 @@ describe('force:source:deploy', () => { | |
}, | ||
}); | ||
ensureHookArgs(); | ||
ensureProgressBar(0); | ||
}); | ||
|
||
it('should NOT call progress bar because of environment variable', async () => { | ||
try { | ||
process.env.SFDX_USE_PROGRESS_BAR = 'false'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nothing wrong with this but jsyk you can stub env.getBoolean instead of wrapping in try/finally |
||
const sourcepath = ['somepath']; | ||
const result = await run({ sourcepath }); | ||
expect(result).to.deep.equal(stubbedResults); | ||
ensureCreateComponentSetArgs({ sourcepath }); | ||
ensureDeployArgs(); | ||
ensureHookArgs(); | ||
ensureProgressBar(0); | ||
} finally { | ||
delete process.env.SFDX_USE_PROGRESS_BAR; | ||
} | ||
}); | ||
|
||
it('should NOT call progress bar because of --json', async () => { | ||
const sourcepath = ['somepath']; | ||
const result = await run({ sourcepath, json: true }); | ||
expect(result).to.deep.equal(stubbedResults); | ||
expect(progressStub.called).to.be.false; | ||
ensureCreateComponentSetArgs({ sourcepath }); | ||
ensureDeployArgs(); | ||
ensureHookArgs(); | ||
ensureProgressBar(0); | ||
}); | ||
|
||
it('should call progress bar', async () => { | ||
const sourcepath = ['somepath']; | ||
const result = await run({ sourcepath }); | ||
expect(result).to.deep.equal(stubbedResults); | ||
ensureCreateComponentSetArgs({ sourcepath }); | ||
ensureDeployArgs(); | ||
ensureHookArgs(); | ||
ensureProgressBar(1); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is supposed to be available on
this
when you extend SfdxCommand but it's a different library version. I wonder if we have a story to bump the version in@salesforce/command
so we don't have to import the version we need for the progress bar.