From 317d40d879cc5eac1073a9182384423596fc098b Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Tue, 21 Aug 2018 16:27:44 -0700 Subject: [PATCH] test(compiler-cli): improve testing harness for incremental compilation (#25275) In tsc 3.0 the check that enables program structure reuse in tryReuseStructureFromOldProgram has changed and now uses identity comparison on arrays within CompilerOptions. Since we recreate the options on each incremental compilation, we now fail this check. After this change the default set of options is reused in between incremental compilations, but we still allow options to be overriden if needed. PR Close #25275 --- .../src/transformers/compiler_host.ts | 3 - packages/compiler-cli/test/test_support.ts | 47 ++++--- .../test/transformers/program_spec.ts | 116 +++++++++--------- 3 files changed, 87 insertions(+), 79 deletions(-) diff --git a/packages/compiler-cli/src/transformers/compiler_host.ts b/packages/compiler-cli/src/transformers/compiler_host.ts index e5fdd7147c..eb54ffd943 100644 --- a/packages/compiler-cli/src/transformers/compiler_host.ts +++ b/packages/compiler-cli/src/transformers/compiler_host.ts @@ -393,9 +393,6 @@ export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHos getSourceFile( fileName: string, languageVersion: ts.ScriptTarget, onError?: ((message: string) => void)|undefined): ts.SourceFile { - if (fileName.endsWith('@angular/core/src/di/injection_token.d.ts')) { - debugger; - } // Note: Don't exit early in this method to make sure // we always have up to date references on the file! let genFileNames: string[] = []; diff --git a/packages/compiler-cli/test/test_support.ts b/packages/compiler-cli/test/test_support.ts index 563550aca8..1dcb259018 100644 --- a/packages/compiler-cli/test/test_support.ts +++ b/packages/compiler-cli/test/test_support.ts @@ -49,6 +49,33 @@ export interface TestSupport { } function createTestSupportFor(basePath: string) { + // Typescript uses identity comparison on `paths` and other arrays in order to determine + // if program structure can be reused for incremental compilation, so we reuse the default + // values unless overriden, and freeze them so that they can't be accidentaly changed somewhere + // in tests. + const defaultCompilerOptions = { + basePath, + 'experimentalDecorators': true, + 'skipLibCheck': true, + 'strict': true, + 'strictPropertyInitialization': false, + 'types': Object.freeze([]) as string[], + 'outDir': path.resolve(basePath, 'built'), + 'rootDir': basePath, + 'baseUrl': basePath, + 'declaration': true, + 'target': ts.ScriptTarget.ES5, + 'module': ts.ModuleKind.ES2015, + 'moduleResolution': ts.ModuleResolutionKind.NodeJs, + 'lib': Object.freeze([ + path.resolve(basePath, 'node_modules/typescript/lib/lib.es6.d.ts'), + ]) as string[], + // clang-format off + 'paths': Object.freeze({'@angular/*': ['./node_modules/@angular/*']}) as {[index: string]: string[]} + // clang-format on + }; + + return {basePath, write, writeFiles, createCompilerOptions, shouldExist, shouldNotExist}; function write(fileName: string, content: string) { @@ -66,25 +93,7 @@ function createTestSupportFor(basePath: string) { } function createCompilerOptions(overrideOptions: ng.CompilerOptions = {}): ng.CompilerOptions { - return { - basePath, - 'experimentalDecorators': true, - 'skipLibCheck': true, - 'strict': true, - 'strictPropertyInitialization': false, - 'types': [], - 'outDir': path.resolve(basePath, 'built'), - 'rootDir': basePath, - 'baseUrl': basePath, - 'declaration': true, - 'target': ts.ScriptTarget.ES5, - 'module': ts.ModuleKind.ES2015, - 'moduleResolution': ts.ModuleResolutionKind.NodeJs, - 'lib': [ - path.resolve(basePath, 'node_modules/typescript/lib/lib.es6.d.ts'), - ], - 'paths': {'@angular/*': ['./node_modules/@angular/*']}, ...overrideOptions, - }; + return {...defaultCompilerOptions, ...overrideOptions}; } function shouldExist(fileName: string) { diff --git a/packages/compiler-cli/test/transformers/program_spec.ts b/packages/compiler-cli/test/transformers/program_spec.ts index e612cd7700..c5afa87a68 100644 --- a/packages/compiler-cli/test/transformers/program_spec.ts +++ b/packages/compiler-cli/test/transformers/program_spec.ts @@ -59,7 +59,7 @@ describe('ng program', () => { function compile( oldProgram?: ng.Program, overrideOptions?: ng.CompilerOptions, rootNames?: string[], - host?: CompilerHost): {program: ng.Program, emitResult: ts.EmitResult, host: ng.CompilerHost} { + host?: CompilerHost): {program: ng.Program, emitResult: ts.EmitResult} { const options = testSupport.createCompilerOptions(overrideOptions); if (!rootNames) { rootNames = [path.resolve(testSupport.basePath, 'src/index.ts')]; @@ -75,7 +75,7 @@ describe('ng program', () => { }); expectNoDiagnosticsInProgram(options, program); const emitResult = program.emit(); - return {emitResult, program, host}; + return {emitResult, program}; } function createWatchModeHost(): ng.CompilerHost { @@ -85,17 +85,16 @@ describe('ng program', () => { const originalGetSourceFile = host.getSourceFile; const cache = new Map(); host.getSourceFile = function(fileName: string): ts.SourceFile { - if (fileName.endsWith('@angular/core/src/di/injection_token.d.ts')) { - debugger; - } const sf = originalGetSourceFile.call(host, fileName) as ts.SourceFile; - if (sf && cache.has(sf.fileName)) { - const oldSf = cache.get(sf.fileName)!; - if (oldSf.getFullText() === sf.getFullText()) { - return oldSf; + if (sf) { + if (cache.has(sf.fileName)) { + const oldSf = cache.get(sf.fileName) !; + if (oldSf.getFullText() === sf.getFullText()) { + return oldSf; + } } + cache.set(sf.fileName, sf); } - sf && cache.set(sf.fileName, sf); return sf; }; return host; @@ -290,62 +289,65 @@ describe('ng program', () => { .toBe(false); }); - describe('reuse tests', () => { - it('should reuse the old ts program completely if nothing changed', () => { - testSupport.writeFiles({'src/index.ts': createModuleAndCompSource('main')}); - const host = createWatchModeHost(); - // Note: the second compile drops factories for library files, - // and therefore changes the structure again - const p1 = compile(undefined, undefined, undefined, host).program; - const p2 = compile(p1, undefined, undefined, host).program; - debugger; - compile(p2, undefined, undefined, host); - expect(tsStructureIsReused(p2.getTsProgram())).toBe(StructureIsReused.Completely); - }); + describe( + 'verify that program structure is reused within tsc in order to speed up incremental compilation', + () => { - it('should reuse the old ts program completely if a template or a ts file changed', () => { - const host = createWatchModeHost(); - testSupport.writeFiles({ - 'src/main.ts': createModuleAndCompSource('main', 'main.html'), - 'src/main.html': `Some template`, - 'src/util.ts': `export const x = 1`, - 'src/index.ts': ` + it('should reuse the old ts program completely if nothing changed', () => { + testSupport.writeFiles({'src/index.ts': createModuleAndCompSource('main')}); + const host = createWatchModeHost(); + // Note: the second compile drops factories for library files, + // and therefore changes the structure again + const p1 = compile(undefined, undefined, undefined, host).program; + const p2 = compile(p1, undefined, undefined, host).program; + compile(p2, undefined, undefined, host); + expect(tsStructureIsReused(p2.getTsProgram())).toBe(StructureIsReused.Completely); + }); + + it('should reuse the old ts program completely if a template or a ts file changed', + () => { + const host = createWatchModeHost(); + testSupport.writeFiles({ + 'src/main.ts': createModuleAndCompSource('main', 'main.html'), + 'src/main.html': `Some template`, + 'src/util.ts': `export const x = 1`, + 'src/index.ts': ` export * from './main'; export * from './util'; ` - }); - // Note: the second compile drops factories for library files, - // and therefore changes the structure again - const p1 = compile(undefined, undefined, undefined, host).program; - const p2 = compile(p1, undefined, undefined, host).program; - testSupport.writeFiles({ - 'src/main.html': `Another template`, - 'src/util.ts': `export const x = 2`, - }); - compile(p2, undefined, undefined, host); - expect(tsStructureIsReused(p2.getTsProgram())).toBe(StructureIsReused.Completely); - }); + }); + // Note: the second compile drops factories for library files, + // and therefore changes the structure again + const p1 = compile(undefined, undefined, undefined, host).program; + const p2 = compile(p1, undefined, undefined, host).program; + testSupport.writeFiles({ + 'src/main.html': `Another template`, + 'src/util.ts': `export const x = 2`, + }); + compile(p2, undefined, undefined, host); + expect(tsStructureIsReused(p2.getTsProgram())).toBe(StructureIsReused.Completely); + }); - it('should not reuse the old ts program if an import changed', () => { - const host = createWatchModeHost(); - testSupport.writeFiles({ - 'src/main.ts': createModuleAndCompSource('main'), - 'src/util.ts': `export const x = 1`, - 'src/index.ts': ` + it('should not reuse the old ts program if an import changed', () => { + const host = createWatchModeHost(); + testSupport.writeFiles({ + 'src/main.ts': createModuleAndCompSource('main'), + 'src/util.ts': `export const x = 1`, + 'src/index.ts': ` export * from './main'; export * from './util'; ` + }); + // Note: the second compile drops factories for library files, + // and therefore changes the structure again + const p1 = compile(undefined, undefined, undefined, host).program; + const p2 = compile(p1, undefined, undefined, host).program; + testSupport.writeFiles( + {'src/util.ts': `import {Injectable} from '@angular/core'; export const x = 1;`}); + compile(p2, undefined, undefined, host); + expect(tsStructureIsReused(p2.getTsProgram())).toBe(StructureIsReused.SafeModules); + }); }); - // Note: the second compile drops factories for library files, - // and therefore changes the structure again - const p1 = compile(undefined, undefined, undefined, host).program; - const p2 = compile(p1, undefined, undefined, host).program; - testSupport.writeFiles( - {'src/util.ts': `import {Injectable} from '@angular/core'; export const x = 1;`}); - compile(p2, undefined, undefined, host); - expect(tsStructureIsReused(p2.getTsProgram())).toBe(StructureIsReused.SafeModules); - }); - }); });