-
Notifications
You must be signed in to change notification settings - Fork 409
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
Chore: Add jest unit test for salesforcedx-utils package #4470
Changes from 6 commits
7de669e
8d8b1fd
33dd810
9d9826d
a52f696
dea16dc
6325cf2
4517270
12305b9
06f3d5c
5bedcf4
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 |
---|---|---|
@@ -1,8 +1,5 @@ | ||
const baseConfig = require('../../config/jest.base.config'); | ||
|
||
module.exports = Object.assign({}, | ||
baseConfig, | ||
{ | ||
testMatch: [ "**/unit/**/?(*.)+(spec|test).[t]s?(x)" ], | ||
} | ||
baseConfig | ||
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. clean up unnecessary override. 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. 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. cool..yeah it's a js file so can be ignored. I can add a ignore comment I think 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. just an FYI this was only showing u in the utils projects which using eslint vs tslint. So I only updated that config file |
||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,12 @@ import * as kill from 'tree-kill'; | |
import { CancellationToken, CommandExecution } from '../types'; | ||
import { Command } from './command'; | ||
|
||
export const NO_PID_ERROR = 'No process associated with sfdx command.'; | ||
export const NO_STDOUT_ERROR = 'No stdout found for childProcess'; | ||
export const NO_STDERR_ERROR = 'No stderr found for childProcess'; | ||
export const CANCELATION_INTERVAL = 1000; | ||
export const KILL_CODE = 'SIGKILL'; | ||
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. exported constants so they can be verified in unit tests. |
||
|
||
export class CliCommandExecution implements CommandExecution { | ||
public readonly command: Command; | ||
public readonly cancellationToken?: CancellationToken; | ||
|
@@ -25,7 +31,7 @@ export class CliCommandExecution implements CommandExecution { | |
this.cancellationToken = cancellationToken; | ||
|
||
if (!childProcess.pid) { | ||
throw new Error('No process associated with sfdx command.'); | ||
throw new Error(NO_PID_ERROR); | ||
} | ||
this.childProcessPid = childProcess.pid; | ||
|
||
|
@@ -53,17 +59,17 @@ export class CliCommandExecution implements CommandExecution { | |
|
||
// Output | ||
if (!childProcess.stdout) { | ||
throw new Error('No stdout found for childProcess'); | ||
throw new Error(NO_STDOUT_ERROR); | ||
} | ||
this.stdoutSubject = Observable.fromEvent(childProcess.stdout, 'data'); | ||
if (!childProcess.stderr) { | ||
throw new Error('No stderr found for childProcess'); | ||
throw new Error(NO_STDERR_ERROR); | ||
} | ||
this.stderrSubject = Observable.fromEvent(childProcess.stderr, 'data'); | ||
|
||
// Cancellation watcher | ||
if (cancellationToken) { | ||
const timer = Observable.interval(1000); | ||
const timer = Observable.interval(CANCELATION_INTERVAL); | ||
timerSubscriber = timer.subscribe(async () => { | ||
if (cancellationToken.isCancellationRequested) { | ||
try { | ||
|
@@ -76,7 +82,7 @@ export class CliCommandExecution implements CommandExecution { | |
} | ||
} | ||
|
||
public async killExecution(signal = 'SIGKILL') { | ||
public async killExecution(signal = KILL_CODE) { | ||
return killPromise(this.childProcessPid, signal); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,21 +7,13 @@ | |
|
||
import { SpawnOptions } from 'child_process'; | ||
import * as cross_spawn from 'cross-spawn'; | ||
import * as os from 'os'; | ||
import 'rxjs/add/observable/fromEvent'; | ||
import 'rxjs/add/observable/interval'; | ||
import { Observable } from 'rxjs/Observable'; | ||
import { Subject } from 'rxjs/Subject'; | ||
import { Subscription } from 'rxjs/Subscription'; | ||
|
||
import { Command } from '../types'; | ||
import { CancellationToken } from '../types'; | ||
import { CommandExecution } from '../types/commandExecution'; | ||
import { TELEMETRY_HEADER } from '../constants'; | ||
import { CancellationToken, Command } from '../types'; | ||
import { CliCommandExecution } from './cliCommandExecution'; | ||
|
||
export class GlobalCliEnvironment { | ||
public static readonly environmentVariables = new Map<string, string>(); | ||
} | ||
import { GlobalCliEnvironment } from './globalCliEnvironment'; | ||
|
||
export class CliCommandExecutor { | ||
protected static patchEnv( | ||
|
@@ -39,9 +31,8 @@ export class CliCommandExecutor { | |
env[key] = value; | ||
}); | ||
|
||
// telemetry header | ||
if (env) { | ||
env.SFDX_TOOL = 'salesforce-vscode-extensions'; | ||
env.SFDX_TOOL = TELEMETRY_HEADER; | ||
} | ||
|
||
// then specific environment from Spawn Options | ||
|
@@ -83,79 +74,3 @@ export class CliCommandExecutor { | |
); | ||
} | ||
} | ||
|
||
export class CompositeCliCommandExecutor { | ||
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. put class in it's own file for cleaner organization and import. |
||
private readonly command: Command; | ||
|
||
public constructor(commands: Command) { | ||
this.command = commands; | ||
} | ||
|
||
public execute( | ||
cancellationToken?: CancellationToken | ||
): CompositeCliCommandExecution { | ||
return new CompositeCliCommandExecution(this.command, cancellationToken); | ||
} | ||
} | ||
|
||
export class CompositeCliCommandExecution implements CommandExecution { | ||
public readonly command: Command; | ||
public readonly cancellationToken?: CancellationToken; | ||
public readonly processExitSubject: Observable<number | undefined>; | ||
public readonly processErrorSubject: Observable<Error | undefined>; | ||
public readonly stdoutSubject: Observable<string>; | ||
public readonly stderrSubject: Observable<string>; | ||
private readonly exitSubject: Subject<number | undefined>; | ||
private readonly errorSubject: Subject<Error | undefined>; | ||
private readonly stdout: Subject<string>; | ||
private readonly stderr: Subject<string>; | ||
|
||
constructor(command: Command, cancellationToken?: CancellationToken) { | ||
this.exitSubject = new Subject(); | ||
this.errorSubject = new Subject(); | ||
this.stdout = new Subject(); | ||
this.stderr = new Subject(); | ||
this.command = command; | ||
this.cancellationToken = cancellationToken; | ||
this.processExitSubject = this.exitSubject.asObservable(); | ||
this.processErrorSubject = this.errorSubject.asObservable(); | ||
this.stdoutSubject = this.stdout.asObservable(); | ||
this.stderrSubject = this.stderr.asObservable(); | ||
|
||
let timerSubscriber: Subscription | null; | ||
if (cancellationToken) { | ||
const timer = Observable.interval(1000); | ||
timerSubscriber = timer.subscribe(async () => { | ||
if (cancellationToken.isCancellationRequested) { | ||
try { | ||
this.exitSubject.next(); | ||
} catch (e) { | ||
console.log(e); | ||
} | ||
} | ||
}); | ||
} | ||
this.processErrorSubject.subscribe(() => { | ||
if (timerSubscriber) { | ||
timerSubscriber.unsubscribe(); | ||
} | ||
}); | ||
|
||
this.processExitSubject.subscribe(() => { | ||
if (timerSubscriber) { | ||
timerSubscriber.unsubscribe(); | ||
} | ||
}); | ||
} | ||
|
||
public successfulExit() { | ||
this.exitSubject.next(0); | ||
} | ||
|
||
public failureExit(e?: Error | undefined) { | ||
if (e) { | ||
this.stderr.next(`${e}${os.EOL}`); | ||
} | ||
this.exitSubject.next(1); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,10 @@ | |
|
||
import { Command } from './command'; | ||
|
||
export const JSON_FLAG = '--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. Love this centralized place for these. Our libraries could do with the same treatment, if we were investing there. |
||
export const LOG_LEVEL_FLAG = '--loglevel'; | ||
export const FATAL = 'fatal'; | ||
|
||
export class CommandBuilder { | ||
public readonly command: string; | ||
public description?: string; | ||
|
@@ -23,7 +27,7 @@ export class CommandBuilder { | |
} | ||
|
||
public withArg(arg: string): CommandBuilder { | ||
if (arg === '--json') { | ||
if (arg === JSON_FLAG) { | ||
this.withJson(); | ||
} else { | ||
this.args.push(arg); | ||
|
@@ -37,8 +41,8 @@ export class CommandBuilder { | |
} | ||
|
||
public withJson(): CommandBuilder { | ||
this.args.push('--json'); | ||
this.args.push('--loglevel', 'fatal'); | ||
this.args.push(JSON_FLAG); | ||
this.args.push(LOG_LEVEL_FLAG, FATAL); | ||
return this; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
import * as os from 'os'; | ||
import { Observable } from 'rxjs/Observable'; | ||
import { Subject } from 'rxjs/Subject'; | ||
import { Subscription } from 'rxjs/Subscription'; | ||
import { CancellationToken } from '../types/cancellationToken'; | ||
import { CommandExecution } from '../types/commandExecution'; | ||
import { Command } from './command'; | ||
|
||
export class CompositeCliCommandExecution implements CommandExecution { | ||
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 new here just moved to a separate file. |
||
public readonly command: Command; | ||
public readonly cancellationToken?: CancellationToken; | ||
public readonly processExitSubject: Observable<number | undefined>; | ||
public readonly processErrorSubject: Observable<Error | undefined>; | ||
public readonly stdoutSubject: Observable<string>; | ||
public readonly stderrSubject: Observable<string>; | ||
private readonly exitSubject: Subject<number | undefined>; | ||
private readonly errorSubject: Subject<Error | undefined>; | ||
private readonly stdout: Subject<string>; | ||
private readonly stderr: Subject<string>; | ||
|
||
constructor(command: Command, cancellationToken?: CancellationToken) { | ||
this.exitSubject = new Subject(); | ||
this.errorSubject = new Subject(); | ||
this.stdout = new Subject(); | ||
this.stderr = new Subject(); | ||
this.command = command; | ||
this.cancellationToken = cancellationToken; | ||
this.processExitSubject = this.exitSubject.asObservable(); | ||
this.processErrorSubject = this.errorSubject.asObservable(); | ||
this.stdoutSubject = this.stdout.asObservable(); | ||
this.stderrSubject = this.stderr.asObservable(); | ||
|
||
let timerSubscriber: Subscription | null; | ||
if (cancellationToken) { | ||
const timer = Observable.interval(1000); | ||
timerSubscriber = timer.subscribe(async () => { | ||
if (cancellationToken.isCancellationRequested) { | ||
try { | ||
this.exitSubject.next(); | ||
} catch (e) { | ||
console.log(e); | ||
} | ||
} | ||
}); | ||
} | ||
this.processErrorSubject.subscribe(() => { | ||
if (timerSubscriber) { | ||
timerSubscriber.unsubscribe(); | ||
} | ||
}); | ||
|
||
this.processExitSubject.subscribe(() => { | ||
if (timerSubscriber) { | ||
timerSubscriber.unsubscribe(); | ||
} | ||
}); | ||
} | ||
|
||
public successfulExit() { | ||
this.exitSubject.next(0); | ||
} | ||
|
||
public failureExit(e?: Error | undefined) { | ||
if (e) { | ||
this.stderr.next(`${e}${os.EOL}`); | ||
} | ||
this.exitSubject.next(1); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { CancellationToken } from '../types'; | ||
import { Command } from './command'; | ||
import { CompositeCliCommandExecution } from './compositeCliCommandExecution'; | ||
|
||
export class CompositeCliCommandExecutor { | ||
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. same story: 1 class exported to its own file. |
||
private readonly command: Command; | ||
|
||
public constructor(commands: Command) { | ||
this.command = commands; | ||
} | ||
|
||
public execute( | ||
cancellationToken?: CancellationToken | ||
): CompositeCliCommandExecution { | ||
return new CompositeCliCommandExecution(this.command, cancellationToken); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,8 @@ import { CommandOutput } from './commandOutput'; | |
import { SfdxCommandBuilder } from './sfdxCommandBuilder'; | ||
/** | ||
* @deprecated | ||
* NOTE: This code is deprecated in favor of using ConfigUtil.ts | ||
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'm not sure why I deleted this NOTE honestly. Probably need to visit if this command is necessary. 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. Might be good to put this back? Or add to the same line as deprecated 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. ok..yeah I'll put it back. This is only used in the debuggers currently that can't access configUtils in the vscode-utils package FYI. |
||
|
||
|
||
*/ | ||
export class ForceConfigGet { | ||
public async getConfig( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export class GlobalCliEnvironment { | ||
public static readonly environmentVariables = new Map<string, string>(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
import { CommandBuilder } from './commandBuilder'; | ||
|
||
export const SFDX_COMMAND = 'sfdx'; | ||
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 should probably put this in constants. 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. Yeah, good idea! |
||
export class SfdxCommandBuilder extends CommandBuilder { | ||
public constructor() { | ||
super('sfdx'); | ||
super(SFDX_COMMAND); | ||
} | ||
} |
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.
unit tests will now be valid under a unit or jest directory.
The intention is the jest directory is the new location for jest true unit tests.