Skip to content
This repository has been archived by the owner on Jan 5, 2022. It is now read-only.

Commit

Permalink
Warnings in success msg (#43)
Browse files Browse the repository at this point in the history
* Post all warnings at the end of the command

* Fix tests

* Fix test

* Add tests for warnings.ts

* Adapt success message text
  • Loading branch information
Florian Richter authored Jan 8, 2020
1 parent a2f080a commit 369694f
Show file tree
Hide file tree
Showing 13 changed files with 105 additions and 56 deletions.
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,11 @@
},
"repository": "https://github.com/SAP/cloud-sdk-cli",
"scripts": {
"postpack": "rm -f oclif.manifest.json",
"pretest": "tslint -p . -t stylish && tslint -p test -t stylish && prettier -c \"**/*.ts\" && tsc -b",
"lint": "tslint -p . -t stylish && tslint -p test -t stylish && prettier -c \"**/*.ts\"",
"fix:formatting": "tslint -p . -t stylish --fix && tslint -p test -t stylish --fix && prettier --write \"**/*.ts\"",
"prepack": "rm -rf lib && tsc -b && cp -R src/templates lib/ && oclif-dev manifest && oclif-dev readme",
"postpack": "rm -f oclif.manifest.json",
"pretest": "rm -rf test/test-output/*-spec/ && npm run lint && tsc -b",
"test": "jest",
"test:debug": "node --inspect-brk node_modules/.bin/jest --runInBand",
"test:watch-debug": "node --inspect-brk node_modules/.bin/jest --watch --runInBand",
Expand Down
3 changes: 1 addition & 2 deletions src/commands/add-approuter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
import { Command, flags } from '@oclif/command';
import cli from 'cli-ux';
import * as Listr from 'listr';
import { getProjectNameFromManifest } from '../utils/manifest-yaml';
import { copyFiles, findConflicts, getCopyDescriptors, getTemplatePaths } from '../utils/templates';
import { copyFiles, findConflicts, getCopyDescriptors, getProjectNameFromManifest, getTemplatePaths } from '../utils/';

export default class AddApprouter extends Command {
static description = 'Setup your Cloud Foundry app to authenticate through the app router';
Expand Down
3 changes: 1 addition & 2 deletions src/commands/add-cds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import { Command, flags } from '@oclif/command';
import cli from 'cli-ux';
import * as Listr from 'listr';
import { installDependencies, modifyGitIgnore, modifyPackageJson } from '../utils';
import { getProjectNameFromManifest } from '../utils/manifest-yaml';
import { copyFiles, findConflicts, getCopyDescriptors, getTemplatePaths } from '../utils/templates';
import { copyFiles, findConflicts, getCopyDescriptors, getProjectNameFromManifest, getTemplatePaths } from '../utils/';

export default class AddCds extends Command {
static description = 'Setup your Cloud Foundry app to use a CDS service';
Expand Down
15 changes: 8 additions & 7 deletions src/commands/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import cli from 'cli-ux';
import * as fs from 'fs';
import * as Listr from 'listr';
import * as path from 'path';
import { boxMessage, getWarnings } from '../utils';
import {
buildScaffold,
copyFiles,
Expand All @@ -22,7 +23,6 @@ import {
shouldBuildScaffold,
usageAnalytics
} from '../utils/';
import { boxMessage } from '../utils/message-formatter';

export default class Init extends Command {
static description = 'Initializes your project for the SAP Cloud SDK, SAP Cloud Platform Cloud Foundry and CI/CD using the SAP Cloud SDK toolkit';
Expand Down Expand Up @@ -169,19 +169,20 @@ export default class Init extends Command {
}

private printSuccessMessage(isScaffold: boolean, addCds: boolean) {
const message = [
'✅ Init finished successfully.',
'',
const warnings = getWarnings();
const body = [
'🚀 Next steps:',
...this.getNextSteps(isScaffold, addCds),
'',
'🔨 Consider setting up Jenkins to continuously build your app.',
'Use `sap-cloud-sdk add-cx-server` to create the setup script.'
];

message.push();

this.log(boxMessage(message));
if (warnings) {
this.log(boxMessage(['⚠️ Init finished with warnings:', ...warnings, '', ...body]));
} else {
this.log(boxMessage(['✅ Init finished successfully.', '', ...body]));
}
}

private getNextSteps(isScaffold: boolean, addCds: boolean): string[] {
Expand Down
21 changes: 14 additions & 7 deletions src/utils/git-ignore.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
/*!
* Copyright (c) 2020 SAP SE or an SAP affiliate company. All rights reserved.
*/
import cli from 'cli-ux';

import * as fs from 'fs';
import * as path from 'path';
import { recordWarning } from '../utils/';

export function modifyGitIgnore(projectDir: string, addCds: boolean) {
const pathToGitignore = path.resolve(projectDir, '.gitignore');
Expand All @@ -21,13 +22,19 @@ export function modifyGitIgnore(projectDir: string, addCds: boolean) {

fs.writeFileSync(pathToGitignore, newFileContent, 'utf8');
} catch (error) {
cli.warn('There was a problem writing to the .gitignore.');
cli.log('If your project is using a different version control system, please make sure the following paths are not tracked:');
pathsToIgnore.forEach(path => cli.log(' ' + path));
recordWarning(
'There was a problem writing to the .gitignore.',
'If your project is using a different version control system,',
'please make sure the following paths are not tracked:',
...pathsToIgnore.map(path => ' ' + path)
);
}
} else {
cli.warn('No .gitignore file found!');
cli.log('If your project is using a different version control system, please make sure the following paths are not tracked:');
pathsToIgnore.forEach(path => cli.log(' ' + path));
recordWarning(
'No .gitignore file found!',
'If your project is using a different version control system,',
'please make sure the following paths are not tracked:',
...pathsToIgnore.map(path => ' ' + path)
);
}
}
4 changes: 4 additions & 0 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@
*/

export * from './copy-list';
export * from './generate-vdm-util';
export * from './git-ignore';
export * from './jest-config';
export * from './manifest-yaml';
export * from './message-formatter';
export * from './package-json';
export * from './scaffold';
export * from './templates';
export * from './usage-analytics';
export * from './warnings';
9 changes: 5 additions & 4 deletions src/utils/jest-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
* Copyright (c) 2020 SAP SE or an SAP affiliate company. All rights reserved.
*/

import cli from 'cli-ux';
import * as fs from 'fs';
import { recordWarning } from '../utils/';

export function getJestConfig(isUnitTests: boolean) {
return {
Expand Down Expand Up @@ -37,9 +37,10 @@ export function modifyJestConfig(jestConfigPath: string, data: any) {

fs.writeFileSync(jestConfigPath, JSON.stringify(adjustedJestConfig, null, 2));
} catch (error) {
cli.warn(error);
return cli.warn(
`Could not edit your Jest config at "${jestConfigPath}". Please verify if the location is correct and consider opening a bug ticket at github.com/SAP/cloud-sdk-cli`
recordWarning(
`Could not edit your Jest config at "${jestConfigPath}".`,
'Please verify if the location is correct and consider opening a bug ticket',
'at https://github.com/SAP/cloud-sdk-cli'
);
}
}
4 changes: 3 additions & 1 deletion src/utils/package-json.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
/*!
* Copyright (c) 2020 SAP SE or an SAP affiliate company. All rights reserved.
*/

import cli from 'cli-ux';
import * as execa from 'execa';
import * as fs from 'fs';
import * as path from 'path';
import { recordWarning } from '../utils/';
import { getJestConfig } from './jest-config';

interface PackageJsonChange {
Expand Down Expand Up @@ -157,7 +159,7 @@ async function getVersionOfDependency(dependency: string): Promise<string> {

return `^${(await version).stdout}`;
} catch (e) {
cli.warn(`Error in finding version for dependency ${dependency} - use LATEST as fallback.`);
recordWarning(`Could not find version information for dependency ${dependency}.`, 'Instead `LATEST` was used as fallback.');
return 'latest';
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/utils/scaffold.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as execa from 'execa';
import * as fs from 'fs';
import * as path from 'path';
import * as rm from 'rimraf';
import { recordWarning } from '../utils/';

export async function shouldBuildScaffold(projectDir: string, buildScaffold: boolean, force: boolean = false): Promise<boolean> {
if (buildScaffold) {
Expand Down Expand Up @@ -60,8 +61,9 @@ function modifyMainTs(pathToMainTs: string) {
const modifiedMainTs = mainTs.replace('.listen(3000)', modifiedListen);

if (!modifiedMainTs.includes(modifiedListen)) {
cli.warn('Could not adjust listening port to `process.env.PORT`. Please adjust manually.');
recordWarning('Could not set listening port to `process.env.PORT`', 'in file `app.module.ts`. Please adjust manually.');
} else {
recordWarning('Could not set listening port to `process.env.PORT`', 'in file `app.module.ts`. Please adjust manually.');
fs.writeFileSync(pathToMainTs, modifiedMainTs);
}
}
Expand All @@ -75,7 +77,7 @@ export function addCatalogueModule(pathToAppModuleTs: string) {
.replace('imports: []', `imports: [${moduleName}]`);

if (!modifiedAppModuleTs.includes(`imports: [${moduleName}]`)) {
cli.warn(`Could not add module ${moduleName} to app.module. Please add manually.`);
recordWarning(`Could not add module ${moduleName} to \`app.module.ts\`. Please add manually.`);
} else {
fs.writeFileSync(pathToAppModuleTs, modifiedAppModuleTs);
}
Expand Down
17 changes: 17 additions & 0 deletions src/utils/warnings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*!
* Copyright (c) 2020 SAP SE or an SAP affiliate company. All rights reserved.
*/

let warnings: string[][] = [];

export function recordWarning(...warn: string[]) {
warnings.push(warn);
}

export function getWarnings(): string[] | undefined {
if (warnings.length > 0) {
const result = warnings.map(warn => `- ${warn.join('\n ')}`);
warnings = [];
return result;
}
}
22 changes: 12 additions & 10 deletions test/init.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,24 @@
* Copyright (c) 2020 SAP SE or an SAP affiliate company. All rights reserved.
*/
const error = jest.fn();
const warn = jest.fn(message => console.log('MOCKED WARNING: ', message));
jest.mock('@oclif/command', () => {
const command = jest.requireActual('@oclif/command');
command.Command.prototype.warn = warn;
return command;
});

jest.mock('cli-ux', () => {
// Mocking needs to happen before the command is imported
const cli = jest.requireActual('cli-ux');
return {
...cli,
default: {
...cli.default,
error,
warn
error
}
};
});
jest.mock('../src/utils/warnings');

import execa = require('execa');
import * as fs from 'fs-extra';
import * as path from 'path';
import Init from '../src/commands/init';
import { getWarnings, recordWarning } from '../src/utils/warnings';
import { getCleanProjectDir, getTestOutputDir } from './test-utils';

const testOutputDir = getTestOutputDir(__filename);
Expand Down Expand Up @@ -138,7 +132,15 @@ describe('Init', () => {

await Init.run([projectDir, '--projectName=testingApp', '--startCommand="npm start"', '--skipInstall', '--no-analytics']);

expect(warn).toHaveBeenCalledWith('No .gitignore file found!');
expect(recordWarning).toHaveBeenCalledWith(
'No .gitignore file found!',
'If your project is using a different version control system,',
'please make sure the following paths are not tracked:',
' credentials.json',
' /s4hana_pipeline',
' /deployment'
);
expect(getWarnings).toHaveBeenCalled();
}, 10000);

it('should add our scripts and dependencies to the package.json', async () => {
Expand Down
29 changes: 10 additions & 19 deletions test/utils/git-ignore.spec.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,10 @@
/*!
* Copyright (c) 2020 SAP SE or an SAP affiliate company. All rights reserved.
*/
const log = jest.fn();
const warn = jest.fn(message => console.log('MOCKED WARNING: ', message));

jest.mock('cli-ux', () => {
// Mocking needs to happen before the command is imported
const cli = jest.requireActual('cli-ux');
return {
...cli,
default: {
...cli.default,
log,
warn
}
};
});

jest.mock('../../src/utils/warnings');
import * as fs from 'fs';
import * as rm from 'rimraf';
import { modifyGitIgnore } from '../../src/utils';
import { modifyGitIgnore, recordWarning } from '../../src/utils';
import { getCleanProjectDir, getTestOutputDir } from '../test-utils';

const testOutputDir = getTestOutputDir(__filename);
Expand Down Expand Up @@ -74,7 +59,13 @@ describe('Git Ignore Utils', () => {

modifyGitIgnore(projectDir, false);

expect(warn).toHaveBeenCalledWith('No .gitignore file found!');
expect(log).toHaveBeenCalledTimes(4);
expect(recordWarning).toHaveBeenCalledWith(
'No .gitignore file found!',
'If your project is using a different version control system,',
'please make sure the following paths are not tracked:',
' credentials.json',
' /s4hana_pipeline',
' /deployment'
);
});
});
23 changes: 23 additions & 0 deletions test/utils/warnings.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*!
* Copyright (c) 2020 SAP SE or an SAP affiliate company. All rights reserved.
*/

import { getWarnings, recordWarning } from '../../src/utils/warnings';

describe('warnings', () => {
it('should record and return warnings', () => {
recordWarning('test warning');
expect(getWarnings()).toEqual(['- test warning']);
});

it('should record and return multiple warnings', () => {
recordWarning('test1');
recordWarning('test2');
expect(getWarnings()).toEqual(['- test1', '- test2']);
});

it('should record and return multi-line warnings', () => {
recordWarning('line1', 'line2', 'line3');
expect(getWarnings()).toEqual(['- line1\n line2\n line3']);
});
});

0 comments on commit 369694f

Please sign in to comment.