Skip to content

Commit

Permalink
feat: improve snyk fix spinner output
Browse files Browse the repository at this point in the history
  • Loading branch information
lili2311 committed Mar 29, 2021
1 parent 4ef96f5 commit 23bd100
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 74 deletions.
2 changes: 0 additions & 2 deletions packages/snyk-fix/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import stripAnsi = require('strip-ansi');
import * as outputFormatter from './lib/output-formatters/show-results-summary';
import { loadPlugin } from './plugins/load-plugin';
import { FixHandlerResultByPlugin } from './plugins/types';
export { EntityToFix } from './types';

import { EntityToFix, ErrorsByEcoSystem, FixedMeta, FixOptions } from './types';
import { convertErrorToUserMessage } from './lib/errors/error-to-user-message';
Expand Down Expand Up @@ -62,7 +61,6 @@ export async function fix(
text: 'Done',
symbol: meta.fixed === 0 ? chalk.red('✖') : chalk.green('✔'),
});

return {
results: resultsByPlugin,
exceptions: exceptionsByScanType,
Expand Down
2 changes: 1 addition & 1 deletion packages/snyk-fix/test/unit/__snapshots__/fix.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ Object {
"fixSummary": "Successful fixes:
requirements.txt
Pinned django from 1.6.1 to 2.0.1
Upgraded django from 1.6.1 to 2.0.1
Unresolved items:
Expand Down
13 changes: 13 additions & 0 deletions src/cli/commands/fix/get-display-path.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as pathLib from 'path';

import { isLocalFolder } from '../../../lib/detect';

export function getDisplayPath(path: string): string {
if (!isLocalFolder(path)) {
return path;
}
if (path === process.cwd()) {
return '.';
}
return pathLib.relative(process.cwd(), path);
}
11 changes: 5 additions & 6 deletions src/cli/commands/fix/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ export = fix;

import * as Debug from 'debug';
import * as snykFix from '@snyk/fix';
import * as pathLib from 'path';
import * as ora from 'ora';

import { MethodArgs } from '../../args';
Expand All @@ -17,6 +16,7 @@ import { validateTestOptions } from '../test/validate-test-options';
import { setDefaultTestOptions } from '../test/set-default-test-options';
import { validateFixCommandIsSupported } from './validate-fix-command-is-supported';
import { Options, TestOptions } from '../../../lib/types';
import { getDisplayPath } from './get-display-path';

const debug = Debug('snyk-fix');
const snykFixFeatureFlag = 'cliSnykFix';
Expand Down Expand Up @@ -70,11 +70,10 @@ async function runSnykTestLegacy(
stdOutSpinner.start();

for (const path of paths) {
let relativePath = path;
let displayPath = path;
try {
const { dir } = pathLib.parse(path);
relativePath = pathLib.relative(process.cwd(), dir);
stdOutSpinner.info(`Running \`snyk test\` for ${relativePath}`);
displayPath = getDisplayPath(path);
stdOutSpinner.info(`Running \`snyk test\` for ${displayPath}`);
// Create a copy of the options so a specific test can
// modify them i.e. add `options.file` etc. We'll need
// these options later.
Expand All @@ -99,7 +98,7 @@ async function runSnykTestLegacy(
results.push(...newRes);
} catch (error) {
const testError = formatTestError(error);
const userMessage = `Test for ${relativePath} failed with error: ${testError.message}.\nRun \`snyk test ${relativePath} -d\` for more information.`;
const userMessage = `Test for ${displayPath} failed with error: ${testError.message}.\nRun \`snyk test ${displayPath} -d\` for more information.`;
stdErrSpinner.fail(userMessage);
debug(userMessage);
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/plugins/get-deps-from-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export async function getDepsFromPlugin(
root,
);

if (!options.json && userWarningMessage) {
if (!options.json && !options.quiet && userWarningMessage) {
console.warn(chalk.bold.red(userWarningMessage));
}
return inspectRes;
Expand Down
4 changes: 3 additions & 1 deletion src/lib/snyk-test/assemble-payloads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ export async function assembleEcosystemPayloads(
path.relative('..', '.') + ' project dir');

spinner.clear<void>(spinnerLbl)();
await spinner(spinnerLbl);
if (!options.quiet) {
await spinner(spinnerLbl);
}

try {
const plugin = getPlugin(ecosystem);
Expand Down
14 changes: 10 additions & 4 deletions src/lib/snyk-test/run-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,9 @@ async function sendAndParseResults(
const iacResults: Promise<TestResult>[] = [];

await spinner.clear<void>(spinnerLbl)();
await spinner(spinnerLbl);
if (!options.quiet) {
await spinner(spinnerLbl);
}
for (const payload of payloads) {
iacResults.push(
queue.add(async () => {
Expand All @@ -266,7 +268,9 @@ async function sendAndParseResults(
const results: TestResult[] = [];
for (const payload of payloads) {
await spinner.clear<void>(spinnerLbl)();
await spinner(spinnerLbl);
if (!options.quiet) {
await spinner(spinnerLbl);
}
/** sendTestPayload() deletes the request.body from the payload once completed. */
const payloadCopy = Object.assign({}, payload);
const res = await sendTestPayload(payload);
Expand Down Expand Up @@ -556,15 +560,17 @@ async function assembleLocalPayloads(
try {
const payloads: Payload[] = [];
await spinner.clear<void>(spinnerLbl)();
await spinner(spinnerLbl);
if (!options.quiet) {
await spinner(spinnerLbl);
}
if (options.iac) {
return assembleIacLocalPayloads(root, options);
}
const deps = await getDepsFromPlugin(root, options);
const failedResults = (deps as MultiProjectResultCustom).failedResults;
if (failedResults?.length) {
await spinner.clear<void>(spinnerLbl)();
if (!options.json) {
if (!options.json && !options.quiet) {
console.warn(
chalk.bold.red(
`✗ ${failedResults.length}/${failedResults.length +
Expand Down
1 change: 1 addition & 0 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export interface Options {
debug?: boolean;
sarif?: boolean;
'group-issues'?: boolean;
quiet?: boolean;
}

// TODO(kyegupov): catch accessing ['undefined-properties'] via noImplicitAny
Expand Down
37 changes: 0 additions & 37 deletions test/__snapshots__/cli-fix.system.spec.ts.snap

This file was deleted.

23 changes: 1 addition & 22 deletions test/cli-fix.system.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ describe('snyk fix (system tests)', () => {
throw new Error('Test expected to return an error');
}
expect(stderr).toBe('');
expect(stripAnsi(stdout)).toMatchSnapshot();
expect(stripAnsi(stdout)).toMatch('No successful fixes');
expect(err.message).toMatch('Command failed');
expect(err.code).toBe(2);
done();
Expand All @@ -226,25 +226,4 @@ describe('snyk fix (system tests)', () => {
},
testTimeout,
);
it(
'`shows expected response when Python project was skipped because of missing remediation data --all-projects`',
(done) => {
exec(
`node ${main} fix ${noVulnsProjectPath}`,
{
env: {
PATH: process.env.PATH,
SNYK_TOKEN: apiKey,
SNYK_API,
SNYK_HOST,
},
},
(err, stdout) => {
expect(stripAnsi(stdout)).toMatchSnapshot();
done();
},
);
},
testTimeout,
);
});
23 changes: 23 additions & 0 deletions test/get-display-path.functional.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import * as pathLib from 'path';
import { getDisplayPath } from '../src/cli/commands/fix/get-display-path';

describe('getDisplayPath', () => {
it('paths that do not exist on disk returned as is', () => {
const displayPath = getDisplayPath('semver@2.3.1');
expect(displayPath).toEqual('semver@2.3.1');
});
it('current path is displayed as .', () => {
const displayPath = getDisplayPath(process.cwd());
expect(displayPath).toEqual('.');
});
it('a local path is returned as relative path to current dir', () => {
const displayPath = getDisplayPath(`test${pathLib.sep}fixtures`);
expect(displayPath).toEqual(`test${pathLib.sep}fixtures`);
});
it('a local full path is returned as relative path to current dir', () => {
const displayPath = getDisplayPath(
pathLib.resolve(process.cwd(), 'test', 'fixtures'),
);
expect(displayPath).toEqual(`test${pathLib.sep}fixtures`);
});
});

0 comments on commit 23bd100

Please sign in to comment.