Skip to content
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

feat: allow to run specific tests on source delete #659

Merged
merged 12 commits into from
Jun 15, 2023
1 change: 1 addition & 0 deletions command-snapshot.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"source-dir",
"target-org",
"test-level",
"tests",
"track-source",
"verbose",
"wait"
Expand Down
8 changes: 8 additions & 0 deletions messages/delete.source.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,11 @@ If the command continues to run after the wait period, the CLI returns control o
Valid values are:

- NoTestRun — No tests are run. This test level applies only to deployments to development environments, such as sandbox, Developer Edition, or trial orgs. This test level is the default for development environments.

- RunSpecifiedTests — Runs only the tests that you specify with the --tests flag. Code coverage requirements differ from the default coverage requirements when using this test level. Executed tests must comprise a minimum of 75% code coverage for each class and trigger in the deployment package. This coverage is computed for each class and trigger individually and is different than the overall coverage percentage.

- RunLocalTests — All tests in your org are run, except the ones that originate from installed managed and unlocked packages. This test level is the default for production deployments that include Apex classes or triggers.

- RunAllTestsInOrg — All tests in your org are run, including tests of managed packages.

If you don’t specify a test level, the default behavior depends on the contents of your deployment package and target org. For more information, see “Running Tests in a Deployment” in the Metadata API Developer Guide.
Expand Down Expand Up @@ -130,3 +134,7 @@ Are you sure you want to proceed (this is only a check and won't actually delete
# conflictMsg

We couldn't complete the operation due to conflicts. Verify that you want to keep the existing versions, then run the command again with the --force-overwrite (-f) option.

# error.NoTestsSpecified

You must specify tests using the --tests flag if the --test-level flag is set to RunSpecifiedTests.
4 changes: 4 additions & 0 deletions messages/deploy.metadata.validate.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,7 @@ Run "%s project deploy quick --job-id %s" to execute this deploy

Failed to validate the deployment (%s). Due To:
%s

# error.NoTestsSpecified

You must specify tests using the --tests flag if the --test-level flag is set to RunSpecifiedTests.
20 changes: 5 additions & 15 deletions schemas/hooks/sf-deploy.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@
"description": "Deployables are individual pieces that can be deployed on their own. For example, each package in a salesforce project is considered a deployable that can be deployed on its own."
}
},
"required": [
"deployables"
],
"required": ["deployables"],
"additionalProperties": false
},
"DeployablePackage": {
Expand All @@ -25,9 +23,7 @@
"$ref": "#/definitions/NamedPackageDir"
}
},
"required": [
"pkg"
],
"required": ["pkg"],
"additionalProperties": false
},
"NamedPackageDir": {
Expand Down Expand Up @@ -91,11 +87,7 @@
"type": "string"
}
},
"required": [
"fullPath",
"name",
"path"
]
"required": ["fullPath", "name", "path"]
},
"PackageDirDependency": {
"type": "object",
Expand All @@ -107,10 +99,8 @@
"type": "string"
}
},
"required": [
"package"
],
"required": ["package"],
"additionalProperties": {}
}
}
}
}
19 changes: 15 additions & 4 deletions src/commands/project/delete/source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,14 @@ import {
import * as chalk from 'chalk';
import { DeleteSourceJson, isSourceComponent } from '../../../utils/types';
import { getPackageDirs, getSourceApiVersion } from '../../../utils/project';
import { resolveApi } from '../../../utils/deploy';
import { resolveApi, validateTests } from '../../../utils/deploy';
import { DeployResultFormatter } from '../../../formatters/deployResultFormatter';
import { DeleteResultFormatter } from '../../../formatters/deleteResultFormatter';
import { DeployProgress } from '../../../utils/progressBar';
import { DeployCache } from '../../../utils/deployCache';
import { testLevelFlag } from '../../../utils/flags';
import { testLevelFlag, testsFlag } from '../../../utils/flags';
const fsPromises = fs.promises;
const testFlags = 'Test';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: move this to a string in the flag definition

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just noticed this group should include --test-level so just reused it in that flag.


Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/plugin-deploy-retrieve', 'delete.source');
Expand Down Expand Up @@ -71,12 +72,16 @@ export class Source extends SfCommand<DeleteSourceJson> {
description: messages.getMessage('flags.wait.description'),
summary: messages.getMessage('flags.wait.summary'),
}),
tests: {
...testsFlag,
helpGroup: testFlags,
char: undefined,
cristiand391 marked this conversation as resolved.
Show resolved Hide resolved
},
'test-level': testLevelFlag({
aliases: ['testlevel'],
deprecateAliases: true,
description: messages.getMessage('flags.test-Level.description'),
summary: messages.getMessage('flags.test-Level.summary'),
options: ['NoTestRun', 'RunLocalTests', 'RunAllTestsInOrg'],
cristiand391 marked this conversation as resolved.
Show resolved Hide resolved
}),
'no-prompt': Flags.boolean({
char: 'r',
Expand Down Expand Up @@ -153,6 +158,10 @@ export class Source extends SfCommand<DeleteSourceJson> {
if (this.flags['track-source']) {
this.tracking = await SourceTracking.create({ org: this.org, project: this.project });
}

if (!validateTests(this.flags['test-level'], this.flags.tests)) {
throw messages.createError('error.NoTestsSpecified');
cristiand391 marked this conversation as resolved.
Show resolved Hide resolved
}
}

protected async delete(): Promise<void> {
Expand Down Expand Up @@ -226,6 +235,7 @@ export class Source extends SfCommand<DeleteSourceJson> {
apiOptions: {
rest: this.isRest,
checkOnly: this.flags['check-only'] ?? false,
...(this.flags.tests ? { runTests: this.flags.tests } : {}),
...(this.flags['test-level'] ? { testLevel: this.flags['test-level'] } : {}),
},
});
Expand Down Expand Up @@ -270,11 +280,12 @@ export class Source extends SfCommand<DeleteSourceJson> {
protected formatResult(): DeleteSourceJson {
const formatterOptions = {
verbose: this.flags.verbose ?? false,
testLevel: this.flags['test-level'],
};

this.deleteResultFormatter = this.mixedDeployDelete.deploy.length
? new DeployResultFormatter(this.deployResult, formatterOptions)
: new DeleteResultFormatter(this.deployResult);
: new DeleteResultFormatter(this.deployResult, formatterOptions);

// Only display results to console when JSON flag is unset.
if (!this.jsonEnabled()) {
Expand Down
7 changes: 6 additions & 1 deletion src/commands/project/deploy/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { AsyncDeployResultFormatter } from '../../../formatters/asyncDeployResul
import { DeployResultFormatter } from '../../../formatters/deployResultFormatter';
import { DeployProgress } from '../../../utils/progressBar';
import { DeployResultJson, TestLevel } from '../../../utils/types';
import { executeDeploy, resolveApi, determineExitCode } from '../../../utils/deploy';
import { executeDeploy, resolveApi, determineExitCode, validateTests } from '../../../utils/deploy';
import { DEPLOY_STATUS_CODES_DESCRIPTIONS } from '../../../utils/errorCodes';
import { ConfigVars } from '../../../configMeta';
import { fileOrDirFlag, testLevelFlag, testsFlag } from '../../../utils/flags';
Expand Down Expand Up @@ -120,6 +120,11 @@ export default class DeployMetadataValidate extends SfCommand<DeployResultJson>

public async run(): Promise<DeployResultJson> {
const [{ flags }, api] = await Promise.all([this.parse(DeployMetadataValidate), resolveApi(this.configAggregator)]);

if (!validateTests(flags['test-level'], flags.tests)) {
throw messages.createError('error.NoTestsSpecified');
}
cristiand391 marked this conversation as resolved.
Show resolved Hide resolved

const username = flags['target-org'].getUsername();

// eslint-disable-next-line @typescript-eslint/require-await
Expand Down
111 changes: 107 additions & 4 deletions src/formatters/deleteResultFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,40 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import * as os from 'os';
import { ux } from '@oclif/core';
import * as chalk from 'chalk';
import { DeployResult, FileResponse, RequestStatus } from '@salesforce/source-deploy-retrieve';
import { dim, underline } from 'chalk';
import { CodeCoverageWarnings, DeployResult, FileResponse, RequestStatus } from '@salesforce/source-deploy-retrieve';
import { ensureArray } from '@salesforce/kit';
import { bold } from 'chalk';
import { StandardColors } from '@salesforce/sf-plugins-core';
import { DeleteSourceJson, Formatter } from '../utils/types';
import { sortFileResponses, asRelativePaths } from '../utils/output';
import { DeleteSourceJson, Formatter, TestLevel, Verbosity } from '../utils/types';
import {
sortFileResponses,
asRelativePaths,
tableHeader,
sortTestResults,
error,
success,
check,
} from '../utils/output';
import { coverageOutput } from '../utils/coverage';

export class DeleteResultFormatter implements Formatter<DeleteSourceJson> {
public constructor(private result: DeployResult) {}
private testLevel: TestLevel | undefined;
private verbosity: Verbosity;

public constructor(
protected result: DeployResult,
protected flags: Partial<{
'test-level': TestLevel;
verbose: boolean;
}>
) {
this.testLevel = flags['test-level'];
this.verbosity = this.determineVerbosity();
}
/**
* Get the JSON output from the DeployResult.
*
Expand All @@ -31,6 +54,7 @@ export class DeleteResultFormatter implements Formatter<DeleteSourceJson> {
}

public display(): void {
this.displayTestResults();
if ([0, 69].includes(process.exitCode ?? 0)) {
const successes: FileResponse[] = [];
const fileResponseSuccesses: Map<string, FileResponse> = new Map<string, FileResponse>();
Expand Down Expand Up @@ -82,6 +106,85 @@ export class DeleteResultFormatter implements Formatter<DeleteSourceJson> {
}
}

private displayTestResults(): void {
if (this.testLevel === TestLevel.NoTestRun || !this.result.response.runTestsEnabled) {
ux.log();
return;
}

this.displayVerboseTestFailures();

if (this.verbosity === 'verbose') {
this.displayVerboseTestSuccesses();
this.displayVerboseTestCoverage();
}

ux.log();
ux.log(tableHeader('Test Results Summary'));
ux.log(`Passing: ${this.result.response.numberTestsCompleted ?? 0}`);
ux.log(`Failing: ${this.result.response.numberTestErrors ?? 0}`);
ux.log(`Total: ${this.result.response.numberTestsTotal ?? 0}`);
const time = this.result.response.details.runTestResult?.totalTime ?? 0;
if (time) ux.log(`Time: ${time}`);
// I think the type might be wrong in SDR
ensureArray(this.result.response.details.runTestResult?.codeCoverageWarnings).map(
(warning: CodeCoverageWarnings & { name?: string }) =>
ux.warn(`${warning.name ? `${warning.name} - ` : ''}${warning.message}`)
);
}

private displayVerboseTestCoverage(): void {
const codeCoverage = ensureArray(this.result.response.details.runTestResult?.codeCoverage);
if (codeCoverage.length) {
const coverage = codeCoverage.sort((a, b) => (a.name.toUpperCase() > b.name.toUpperCase() ? 1 : -1));
ux.log();
ux.log(tableHeader('Apex Code Coverage'));

ux.table(coverage.map(coverageOutput), {
name: { header: 'Name' },
numLocations: { header: '% Covered' },
lineNotCovered: { header: 'Uncovered Lines' },
});
}
}

private displayVerboseTestSuccesses(): void {
const successes = ensureArray(this.result.response.details.runTestResult?.successes);
if (successes.length > 0) {
const testSuccesses = sortTestResults(successes);
ux.log();
ux.log(success(`Test Success [${successes.length}]`));
for (const test of testSuccesses) {
const testName = underline(`${test.name}.${test.methodName}`);
ux.log(`${check} ${testName}`);
}
}
}

private displayVerboseTestFailures(): void {
if (!this.result.response.numberTestErrors) return;
const failures = ensureArray(this.result.response.details.runTestResult?.failures);
const failureCount = this.result.response.details.runTestResult?.numFailures;
const testFailures = sortTestResults(failures);
ux.log();
ux.log(error(`Test Failures [${failureCount}]`));
for (const test of testFailures) {
const testName = underline(`${test.name}.${test.methodName}`);
ux.log(`• ${testName}`);
ux.log(` ${dim('message')}: ${test.message}`);
if (test.stackTrace) {
const stackTrace = test.stackTrace.replace(/\n/g, `${os.EOL} `);
ux.log(` ${dim('stacktrace')}: ${os.EOL} ${stackTrace}`);
}
ux.log();
}
}

private determineVerbosity(): Verbosity {
if (this.flags.verbose) return 'verbose';
return 'normal';
}

cristiand391 marked this conversation as resolved.
Show resolved Hide resolved
private displayFailures(): void {
if (this.result.response.status === RequestStatus.Succeeded) return;

Expand Down
35 changes: 11 additions & 24 deletions src/formatters/deployResultFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ import * as os from 'os';
import * as path from 'path';
import * as fs from 'fs';
import { ux } from '@oclif/core';
import { dim, underline, bold } from 'chalk';
import { dim, underline } from 'chalk';
import {
CodeCoverageWarnings,
DeployResult,
Failures,
FileResponse,
FileResponseFailure,
RequestStatus,
Successes,
} from '@salesforce/source-deploy-retrieve';
import { Org, SfError } from '@salesforce/core';
import { ensureArray } from '@salesforce/kit';
Expand All @@ -27,7 +25,6 @@ import {
JUnitReporter,
TestResult,
} from '@salesforce/apex-node';
import { StandardColors } from '@salesforce/sf-plugins-core';
import { DeployResultJson, isSdrFailure, isSdrSuccess, TestLevel, Verbosity, Formatter } from '../utils/types';
import {
generateCoveredLines,
Expand All @@ -36,7 +33,16 @@ import {
transformCoverageToApexCoverage,
coverageOutput,
} from '../utils/coverage';
import { sortFileResponses, asRelativePaths, tableHeader, getFileResponseSuccessProps } from '../utils/output';
import {
sortFileResponses,
asRelativePaths,
tableHeader,
getFileResponseSuccessProps,
sortTestResults,
error,
success,
check,
} from '../utils/output';

export class DeployResultFormatter implements Formatter<DeployResultJson> {
private relativeFiles: FileResponse[];
Expand Down Expand Up @@ -380,22 +386,3 @@ export class DeployResultFormatter implements Formatter<DeployResultJson> {
return 'normal';
}
}

function sortTestResults<T extends Failures | Successes>(results: T[]): T[] {
return results.sort((a, b) => {
if (a.methodName === b.methodName) {
return a.name.localeCompare(b.name);
}
return a.methodName.localeCompare(b.methodName);
});
}

function error(message: string): string {
return StandardColors.error(bold(message));
}

function success(message: string): string {
return StandardColors.success(bold(message));
}

const check = StandardColors.success('✓');
1 change: 1 addition & 0 deletions src/utils/flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export const zipFileFlag = Flags.custom<string>({
export const testsFlag = Flags.string({
char: 't',
multiple: true,
dependsOn: ['test-level'],
cristiand391 marked this conversation as resolved.
Show resolved Hide resolved
summary: commonFlagMessages.getMessage('flags.tests.summary'),
description: commonFlagMessages.getMessage('flags.tests.description'),
// the old version allowed comma separated values, and the change is confusing enough to deserve a warning
Expand Down
Loading