fix(@angular-devkit/architect): unsubscribe on timeout in runTargetSpec

This way cleanup logic runs on timeout.

It also simplifies timeout definition for runTargetSpec and Jasmine.

Author: filipesilva

packages/angular_devkit/architect/testing/run-target-spec.ts

@@ -6,25 +6,47 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import { Path, experimental, logging, normalize } from '@angular-devkit/core';
-import { Observable } from 'rxjs';
-import { concatMap } from 'rxjs/operators';
+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 { TestProjectHost } from './test-project-host';
 
+export const DefaultTimeout = 45000;
 
 export function runTargetSpec(
   host: TestProjectHost,
   targetSpec: TargetSpecifier,
   overrides = {},
+  timeout = DefaultTimeout,
   logger: logging.Logger = new logging.NullLogger(),
 ): Observable<BuildEvent> {
   targetSpec = { ...targetSpec, overrides };
   const workspaceFile = normalize('angular.json');
   const workspace = new experimental.workspace.Workspace(host.root(), host);
 
-  return workspace.loadWorkspaceFromHost(workspaceFile).pipe(
+  // Emit when runArchitect$ completes or errors.
+  // TODO: There must be a better way of doing this...
+  let finalizeCB = () => { };
+  const runArchitectFinalize$ = new Observable(obs => {
+    finalizeCB = () => obs.next();
+  });
+
+  // Load the workspace from the root of the host, then run a target.
+  const runArchitect$ = workspace.loadWorkspaceFromHost(workspaceFile).pipe(
     concatMap(ws => new Architect(ws).loadArchitect()),
     concatMap(arch => arch.run(arch.getBuilderConfiguration(targetSpec), { logger })),
+    finalize(() => finalizeCB()),
+  );
+
+  // Error out after the timeout if runArchitect$ hasn't finalized.
+  const timeout$ = timer(timeout).pipe(
+    takeUntil(runArchitectFinalize$),
+    concatMapTo(throwError(`runTargetSpec timeout (${timeout}) reached.`)),
+  );
+
+  return merge(
+    timeout$,
+    runArchitect$,
   );
 }

packages/angular_devkit/build_angular/test/app-shell/app-shell_spec_large.ts

@@ -5,10 +5,10 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import { runTargetSpec } from '@angular-devkit/architect/testing';
+import { DefaultTimeout, runTargetSpec } from '@angular-devkit/architect/testing';
 import { normalize, virtualFs } from '@angular-devkit/core';
 import { tap } from 'rxjs/operators';
-import { Timeout, host } from '../utils';
+import { host } from '../utils';
 
 
 describe('AppShell Builder', () => {
@@ -38,14 +38,13 @@ describe('AppShell Builder', () => {
       `,
     });
 
-    runTargetSpec(host, { project: 'app', target: 'app-shell' }).pipe(
+    runTargetSpec(host, { project: 'app', target: 'app-shell' }, DefaultTimeout * 2).pipe(
       tap((buildEvent) => expect(buildEvent.success).toBe(true)),
       tap(() => {
         const fileName = 'dist/index.html';
         const content = virtualFs.fileBufferToString(host.scopedSync().read(normalize(fileName)));
         expect(content).toMatch(/Welcome to app!/);
       }),
     ).toPromise().then(done, done.fail);
-
-  }, Timeout.Complex);
+  });
 });

packages/angular_devkit/build_angular/test/browser/allow-js_spec_large.ts

@@ -8,7 +8,7 @@
 
 import { runTargetSpec } from '@angular-devkit/architect/testing';
 import { tap } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder allow js', () => {
@@ -27,7 +27,7 @@ describe('Browser Builder allow js', () => {
     runTargetSpec(host, browserTargetSpec).pipe(
       tap((buildEvent) => expect(buildEvent.success).toBe(true)),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
   it('works with aot', (done) => {
     host.writeMultipleFiles({
@@ -40,5 +40,5 @@ describe('Browser Builder allow js', () => {
     runTargetSpec(host, browserTargetSpec, overrides).pipe(
       tap((buildEvent) => expect(buildEvent.success).toBe(true)),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 });

packages/angular_devkit/build_angular/test/browser/aot_spec_large.ts

@@ -9,7 +9,7 @@
 import { runTargetSpec } from '@angular-devkit/architect/testing';
 import { join, normalize, virtualFs } from '@angular-devkit/core';
 import { tap } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder AOT', () => {
@@ -29,5 +29,5 @@ describe('Browser Builder AOT', () => {
         expect(content).toMatch(/platformBrowser.*bootstrapModuleFactory.*AppModuleNgFactory/);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Standard);
+  });
 });

packages/angular_devkit/build_angular/test/browser/assets_spec_large.ts

@@ -9,7 +9,7 @@
 import { runTargetSpec } from '@angular-devkit/architect/testing';
 import { normalize, virtualFs } from '@angular-devkit/core';
 import { tap, toArray } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder assets', () => {
@@ -57,7 +57,7 @@ describe('Browser Builder assets', () => {
         expect(host.scopedSync().exists(normalize('./dist/folder/.gitkeep'))).toBe(false);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
   it('fails with non-absolute output path', (done) => {
     const assets: { [path: string]: string } = {
@@ -76,7 +76,7 @@ describe('Browser Builder assets', () => {
     // The node_modules folder must be deleted, otherwise code that tries to find the
     // node_modules folder will hit this one and can fail.
     host.scopedSync().delete(normalize('./node_modules'));
-  }, Timeout.Basic);
+  });
 
   it('fails with non-source root input path', (done) => {
     const assets: { [path: string]: string } = {
@@ -93,7 +93,7 @@ describe('Browser Builder assets', () => {
     // The node_modules folder must be deleted, otherwise code that tries to find the
     // node_modules folder will hit this one and can fail.
     host.scopedSync().delete(normalize('./node_modules'));
-  }, Timeout.Basic);
+  });
 
   it('still builds with empty asset array', (done) => {
     const overrides = {
@@ -104,5 +104,5 @@ describe('Browser Builder assets', () => {
       toArray(),
       tap((buildEvents) => expect(buildEvents.length).toBe(1)),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 });

packages/angular_devkit/build_angular/test/browser/base-href_spec_large.ts

@@ -9,7 +9,7 @@
 import { runTargetSpec } from '@angular-devkit/architect/testing';
 import { join, normalize, virtualFs } from '@angular-devkit/core';
 import { tap } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder base href', () => {
@@ -34,5 +34,5 @@ describe('Browser Builder base href', () => {
         expect(content).toMatch(/<base href="\/myUrl">/);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 });

packages/angular_devkit/build_angular/test/browser/build-optimizer_spec_large.ts

@@ -9,7 +9,7 @@
 import { runTargetSpec } from '@angular-devkit/architect/testing';
 import { join, normalize, virtualFs } from '@angular-devkit/core';
 import { tap } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder build optimizer', () => {
@@ -28,5 +28,5 @@ describe('Browser Builder build optimizer', () => {
         expect(content).not.toMatch(/\.decorators =/);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 });

packages/angular_devkit/build_angular/test/browser/bundle-budgets_spec_large.ts

@@ -6,9 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import { TestLogger, runTargetSpec } from '@angular-devkit/architect/testing';
+import { DefaultTimeout, TestLogger, runTargetSpec } from '@angular-devkit/architect/testing';
 import { tap } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder bundle budgets', () => {
@@ -24,22 +24,22 @@ describe('Browser Builder bundle budgets', () => {
 
     const logger = new TestLogger('rebuild-type-errors');
 
-    runTargetSpec(host, browserTargetSpec, overrides, logger).pipe(
+    runTargetSpec(host, browserTargetSpec, overrides, DefaultTimeout * 2, logger).pipe(
       tap((buildEvent) => expect(buildEvent.success).toBe(true)),
       tap(() => expect(logger.includes('WARNING')).toBe(false)),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Complex);
+  });
 
   it('shows errors', (done) => {
     const overrides = {
       optimization: true,
       budgets: [{ type: 'all', maximumError: '100b' }],
     };
 
-    runTargetSpec(host, browserTargetSpec, overrides).pipe(
+    runTargetSpec(host, browserTargetSpec, overrides, DefaultTimeout * 2).pipe(
       tap((buildEvent) => expect(buildEvent.success).toBe(false)),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Complex);
+  });
 
   it('shows warnings', (done) => {
     const overrides = {
@@ -49,9 +49,9 @@ describe('Browser Builder bundle budgets', () => {
 
     const logger = new TestLogger('rebuild-type-errors');
 
-    runTargetSpec(host, browserTargetSpec, overrides, logger).pipe(
+    runTargetSpec(host, browserTargetSpec, overrides, DefaultTimeout * 2, logger).pipe(
       tap((buildEvent) => expect(buildEvent.success).toBe(true)),
       tap(() => expect(logger.includes('WARNING')).toBe(true)),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Complex);
+  });
 });

packages/angular_devkit/build_angular/test/browser/circular-dependency_spec_large.ts

@@ -6,9 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import { TestLogger, runTargetSpec } from '@angular-devkit/architect/testing';
+import { DefaultTimeout, TestLogger, runTargetSpec } from '@angular-devkit/architect/testing';
 import { tap } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder circular dependency detection', () => {
@@ -22,9 +22,9 @@ describe('Browser Builder circular dependency detection', () => {
     const overrides = { baseHref: '/myUrl' };
     const logger = new TestLogger('circular-dependencies');
 
-    runTargetSpec(host, browserTargetSpec, overrides, logger).pipe(
+    runTargetSpec(host, browserTargetSpec, overrides, DefaultTimeout, logger).pipe(
       tap((buildEvent) => expect(buildEvent.success).toBe(true)),
       tap(() => expect(logger.includes('Circular dependency detected')).toBe(true)),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 });

packages/angular_devkit/build_angular/test/browser/deploy-url_spec_large.ts

@@ -9,7 +9,7 @@
 import { runTargetSpec } from '@angular-devkit/architect/testing';
 import { join, normalize, virtualFs } from '@angular-devkit/core';
 import { concatMap, tap } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder deploy url', () => {
@@ -37,7 +37,7 @@ describe('Browser Builder deploy url', () => {
         expect(content).toContain('http://example.com/some/path/main.js');
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
   it('uses deploy url for in webpack runtime', (done) => {
     const overrides = { deployUrl: 'deployUrl/' };
@@ -50,6 +50,6 @@ describe('Browser Builder deploy url', () => {
         expect(content).toContain('deployUrl/');
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
 });

packages/angular_devkit/build_angular/test/browser/errors_spec_large.ts

@@ -6,9 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import { TestLogger, runTargetSpec } from '@angular-devkit/architect/testing';
+import { DefaultTimeout, TestLogger, runTargetSpec } from '@angular-devkit/architect/testing';
 import { tap } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder errors', () => {
@@ -22,36 +22,36 @@ describe('Browser Builder errors', () => {
     `);
     const logger = new TestLogger('errors-compilation');
 
-    runTargetSpec(host, browserTargetSpec, undefined, logger).pipe(
+    runTargetSpec(host, browserTargetSpec, {}, DefaultTimeout, logger).pipe(
       tap((buildEvent) => {
         expect(buildEvent.success).toBe(false);
         expect(logger.includes('polyfills.ts is missing from the TypeScript')).toBe(true);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
   it('shows TS syntax errors', (done) => {
     host.appendToFile('src/app/app.component.ts', ']]]');
     const logger = new TestLogger('errors-syntax');
 
-    runTargetSpec(host, browserTargetSpec, undefined, logger).pipe(
+    runTargetSpec(host, browserTargetSpec, {}, DefaultTimeout, logger).pipe(
       tap((buildEvent) => {
         expect(buildEvent.success).toBe(false);
         expect(logger.includes('Declaration or statement expected.')).toBe(true);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
   it('shows static analysis errors', (done) => {
     host.replaceInFile('src/app/app.component.ts', `'app-root'`, `(() => 'app-root')()`);
     const logger = new TestLogger('errors-static');
 
-    runTargetSpec(host, browserTargetSpec, { aot: true }, logger).pipe(
+    runTargetSpec(host, browserTargetSpec, { aot: true }, DefaultTimeout, logger).pipe(
       tap((buildEvent) => {
         expect(buildEvent.success).toBe(false);
         expect(logger.includes('Function expressions are not supported in')).toBe(true);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
 });

packages/angular_devkit/build_angular/test/browser/i18n_spec_large.ts

@@ -9,7 +9,7 @@
 import { runTargetSpec } from '@angular-devkit/architect/testing';
 import { join, normalize, virtualFs } from '@angular-devkit/core';
 import { tap } from 'rxjs/operators';
-import { Timeout, browserTargetSpec, host } from '../utils';
+import { browserTargetSpec, host } from '../utils';
 
 
 describe('Browser Builder i18n', () => {
@@ -62,7 +62,7 @@ describe('Browser Builder i18n', () => {
         expect(content).toMatch(/Bonjour i18n!/);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
   it('ignores missing translations', (done) => {
     const overrides = {
@@ -84,7 +84,7 @@ describe('Browser Builder i18n', () => {
         expect(content).toMatch(/Other content/);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
   it('reports errors for missing translations', (done) => {
     const overrides = {
@@ -101,7 +101,7 @@ describe('Browser Builder i18n', () => {
     runTargetSpec(host, browserTargetSpec, overrides).pipe(
       tap((buildEvent) => expect(buildEvent.success).toBe(false)),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 
   it('register locales', (done) => {
     const overrides = { aot: true, i18nLocale: 'fr_FR' };
@@ -115,5 +115,5 @@ describe('Browser Builder i18n', () => {
         expect(content).toMatch(/angular_common_locales_fr/);
       }),
     ).toPromise().then(done, done.fail);
-  }, Timeout.Basic);
+  });
 });