Skip to content

Commit

Permalink
test(compiler-cli): improve testing harness for incremental compilati…
Browse files Browse the repository at this point in the history 
…on (#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
  • Loading branch information
@IgorMinar @matsko
IgorMinar authored and matsko committed Aug 28, 2018
1 parent ab32ac6 commit 317d40d
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 81 deletions.
3 changes: 0 additions & 3 deletions packages/compiler-cli/src/transformers/compiler_host.ts
View file
Expand Up @@ -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[] = [];
Expand Down
47 changes: 28 additions & 19 deletions packages/compiler-cli/test/test_support.ts
View file
Expand Up @@ -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<string>([]) 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) {
Expand All @@ -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) {
Expand Down
120 changes: 61 additions & 59 deletions packages/compiler-cli/test/transformers/program_spec.ts
View file
Expand Up @@ -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')];
Expand All @@ -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 {
Expand All @@ -85,17 +85,16 @@ describe('ng program', () => {
const originalGetSourceFile = host.getSourceFile;
const cache = new Map<string, ts.SourceFile>();
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;
Expand Down Expand Up @@ -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);
});

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': `
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 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);
});

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': `
});
// 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': `
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);
});
});

});

Expand Down

0 comments on commit 317d40d

Please sign in to comment.