Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): lint non human readable formatter…
Browse files Browse the repository at this point in the history
…s produces invalid output

fixes #12674
  • Loading branch information
alan-agius4 authored and alexeagle committed Dec 6, 2018
1 parent bc76d0c commit 45b6df5
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 10 deletions.
8 changes: 6 additions & 2 deletions packages/angular/cli/models/architect-command.ts
Expand Up @@ -7,6 +7,7 @@
*/
import {
Architect,
BuilderContext,
TargetSpecifier,
} from '@angular-devkit/architect';
import { experimental, json, schema, tags } from '@angular-devkit/core';
Expand Down Expand Up @@ -185,8 +186,11 @@ export abstract class ArchitectCommand<
return 1;
}
const realBuilderConf = this._architect.getBuilderConfiguration({ ...targetSpec, overrides });

const result = await this._architect.run(realBuilderConf, { logger: this.logger }).toPromise();
const builderContext: Partial<BuilderContext> = {
logger: this.logger,
targetSpecifier: targetSpec,
};
const result = await this._architect.run(realBuilderConf, builderContext).toPromise();

return result.success ? 0 : 1;
}
Expand Down
1 change: 1 addition & 0 deletions packages/angular_devkit/architect/src/architect.ts
Expand Up @@ -64,6 +64,7 @@ export interface BuilderContext {
host: virtualFs.Host<{}>;
workspace: experimental.workspace.Workspace;
architect: Architect;
targetSpecifier?: TargetSpecifier;
}

// TODO: use Build Event Protocol
Expand Down
8 changes: 6 additions & 2 deletions packages/angular_devkit/architect/testing/run-target-spec.ts
Expand Up @@ -9,7 +9,7 @@
import { experimental, logging, normalize } from '@angular-devkit/core';
import { Observable, merge, throwError, timer } from 'rxjs';
import { concatMap, concatMapTo, finalize, takeUntil } from 'rxjs/operators';
import { Architect, BuildEvent, TargetSpecifier } from '../src';
import { Architect, BuildEvent, BuilderContext, TargetSpecifier } from '../src';
import { TestProjectHost } from './test-project-host';

export const DefaultTimeout = 45000;
Expand All @@ -33,9 +33,13 @@ export function runTargetSpec(
});

// Load the workspace from the root of the host, then run a target.
const builderContext: Partial<BuilderContext> = {
logger,
targetSpecifier: targetSpec,
};
const runArchitect$ = workspace.loadWorkspaceFromHost(workspaceFile).pipe(
concatMap(ws => new Architect(ws).loadArchitect()),
concatMap(arch => arch.run(arch.getBuilderConfiguration(targetSpec), { logger })),
concatMap(arch => arch.run(arch.getBuilderConfiguration(targetSpec), builderContext)),
finalize(() => finalizeCB()),
);

Expand Down
16 changes: 11 additions & 5 deletions packages/angular_devkit/build_angular/src/tslint/index.ts
Expand Up @@ -60,6 +60,17 @@ export default class TslintBuilder implements Builder<TslintBuilderOptions> {
const root = this.context.workspace.root;
const systemRoot = getSystemPath(root);
const options = builderConfig.options;
const targetSpecifier = this.context.targetSpecifier;
const projectName = targetSpecifier && targetSpecifier.project || '';

// Print formatter output directly for non human-readable formats.
if (!['prose', 'verbose', 'stylish'].includes(options.format)) {
options.silent = true;
}

if (!options.silent) {
this.context.logger.info(`Linting ${JSON.stringify(projectName)}...`);
}

if (!options.tsConfig && options.typeCheck) {
throw new Error('A "project" must be specified to enable type checking.');
Expand Down Expand Up @@ -116,11 +127,6 @@ export default class TslintBuilder implements Builder<TslintBuilderOptions> {
}
}

// Print formatter output directly for non human-readable formats.
if (['prose', 'verbose', 'stylish'].indexOf(options.format) == -1) {
options.silent = true;
}

if (result.warningCount > 0 && !options.silent) {
this.context.logger.warn('Lint warnings found in the listed files.');
}
Expand Down
Expand Up @@ -12,7 +12,7 @@ import { tap } from 'rxjs/operators';
import { TslintBuilderOptions } from '../../src';
import { host, tslintTargetSpec } from '../utils';


// tslint:disable-next-line:no-big-function
describe('Tslint Target', () => {
const filesWithErrors = { 'src/foo.ts': 'const foo = "";\n' };

Expand All @@ -25,6 +25,28 @@ describe('Tslint Target', () => {
).toPromise().then(done, done.fail);
}, 30000);

it(`should show project name when formatter is human readable`, (done) => {
const logger = new TestLogger('lint-info');

runTargetSpec(host, tslintTargetSpec, undefined, undefined, logger).pipe(
tap((buildEvent) => expect(buildEvent.success).toBe(true)),
tap(() => {
expect(logger.includes('Linting "app"...')).toBe(true);
}),
).toPromise().then(done, done.fail);
}, 30000);

it(`should not show project name when formatter is non human readable`, (done) => {
const logger = new TestLogger('lint-info');

runTargetSpec(host, tslintTargetSpec, { format: 'checkstyle' }, undefined, logger).pipe(
tap((buildEvent) => expect(buildEvent.success).toBe(true)),
tap(() => {
expect(logger.includes('Linting "app"...')).toBe(false);
}),
).toPromise().then(done, done.fail);
}, 30000);

it('should report lint error once', (done) => {
host.writeMultipleFiles({'src/app/app.component.ts': 'const foo = "";\n' });
const logger = new TestLogger('lint-error');
Expand Down

0 comments on commit 45b6df5

Please sign in to comment.