From 49f27e31ed8c9b563b92a193a1a9eed366e069de Mon Sep 17 00:00:00 2001 From: JoostK Date: Fri, 26 Jun 2020 23:06:36 +0200 Subject: [PATCH] test(compiler-cli): re-enable dynamic value diagnostic tests on Windows CI (#37782) This commit re-enables some tests that were temporarily disabled on Windows, as they failed on native Windows CI. The Windows filesystem emulation has been corrected in an earlier commit, such that the original failure would now also occur during emulation on Linux CI. PR Close #37782 --- .../annotations/test/diagnostics_spec.ts | 157 +++++++++--------- .../ngtsc/typecheck/test/type_checker_spec.ts | 87 +++++----- 2 files changed, 116 insertions(+), 128 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/diagnostics_spec.ts index 86e77709e9..390c6ee933 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/diagnostics_spec.ts @@ -6,99 +6,92 @@ * found in the LICENSE file at https://angular.io/license */ -import {platform} from 'os'; import * as ts from 'typescript'; -import {FatalDiagnosticError} from '../../diagnostics'; -import {absoluteFrom as _} from '../../file_system'; +import {absoluteFrom as _, getSourceFileOrError} from '../../file_system'; import {runInEachFileSystem, TestFile} from '../../file_system/testing'; import {PartialEvaluator} from '../../partial_evaluator'; import {TypeScriptReflectionHost} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing'; import {createValueHasWrongTypeError} from '../src/diagnostics'; -runInEachFileSystem(os => { +runInEachFileSystem(() => { describe('ngtsc annotation diagnostics', () => { - // These tests are currently disabled when running in Windows mode as the assertions involving - // the filename attached to the diagnostic are suffering from a case-sensitivity issue. - // - // TODO(JoostK): re-enable on Windows once the case issue has been solved. - if (os !== 'Windows' && platform() !== 'win32') { - describe('createValueError()', () => { - it('should include a trace for dynamic values', () => { - const error = createError('', 'nonexistent', 'Error message'); + describe('createValueError()', () => { + it('should include a trace for dynamic values', () => { + const {error, program} = createError('', 'nonexistent', 'Error message'); + const entrySf = getSourceFileOrError(program, _('/entry.ts')); - if (typeof error.message === 'string') { - return fail('Created error must have a message chain'); - } - expect(error.message.messageText).toBe('Error message'); - expect(error.message.next!.length).toBe(1); - expect(error.message.next![0].messageText) - .toBe(`Value could not be determined statically.`); + if (typeof error.message === 'string') { + return fail('Created error must have a message chain'); + } + expect(error.message.messageText).toBe('Error message'); + expect(error.message.next!.length).toBe(1); + expect(error.message.next![0].messageText) + .toBe(`Value could not be determined statically.`); - expect(error.relatedInformation).toBeDefined(); - expect(error.relatedInformation!.length).toBe(1); + expect(error.relatedInformation).toBeDefined(); + expect(error.relatedInformation!.length).toBe(1); - expect(error.relatedInformation![0].messageText).toBe('Unknown reference.'); - expect(error.relatedInformation![0].file!.fileName).toBe(_('/entry.ts')); - expect(getSourceCode(error.relatedInformation![0])).toBe('nonexistent'); - }); - - it('should include a pointer for a reference to a named declaration', () => { - const error = createError( - `import {Foo} from './foo';`, 'Foo', 'Error message', - [{name: _('/foo.ts'), contents: 'export class Foo {}'}]); - - if (typeof error.message === 'string') { - return fail('Created error must have a message chain'); - } - expect(error.message.messageText).toBe('Error message'); - expect(error.message.next!.length).toBe(1); - expect(error.message.next![0].messageText).toBe(`Value is a reference to 'Foo'.`); - - expect(error.relatedInformation).toBeDefined(); - expect(error.relatedInformation!.length).toBe(1); - expect(error.relatedInformation![0].messageText).toBe('Reference is declared here.'); - expect(error.relatedInformation![0].file!.fileName).toBe(_('/foo.ts')); - expect(getSourceCode(error.relatedInformation![0])).toBe('Foo'); - }); - - it('should include a pointer for a reference to an anonymous declaration', () => { - const error = createError( - `import Foo from './foo';`, 'Foo', 'Error message', - [{name: _('/foo.ts'), contents: 'export default class {}'}]); - - if (typeof error.message === 'string') { - return fail('Created error must have a message chain'); - } - expect(error.message.messageText).toBe('Error message'); - expect(error.message.next!.length).toBe(1); - expect(error.message.next![0].messageText) - .toBe(`Value is a reference to an anonymous declaration.`); - - expect(error.relatedInformation).toBeDefined(); - expect(error.relatedInformation!.length).toBe(1); - expect(error.relatedInformation![0].messageText).toBe('Reference is declared here.'); - expect(error.relatedInformation![0].file!.fileName).toBe(_('/foo.ts')); - expect(getSourceCode(error.relatedInformation![0])).toBe('export default class {}'); - }); - - it('should include a representation of the value\'s type', () => { - const error = createError('', '{a: 2}', 'Error message'); - - if (typeof error.message === 'string') { - return fail('Created error must have a message chain'); - } - expect(error.message.messageText).toBe('Error message'); - expect(error.message.next!.length).toBe(1); - expect(error.message.next![0].messageText).toBe(`Value is of type '{ a: number }'.`); - - expect(error.relatedInformation).not.toBeDefined(); - }); + expect(error.relatedInformation![0].messageText).toBe('Unknown reference.'); + expect(error.relatedInformation![0].file!.fileName).toBe(entrySf.fileName); + expect(getSourceCode(error.relatedInformation![0])).toBe('nonexistent'); }); - } - it('should not be empty', () => {}); + it('should include a pointer for a reference to a named declaration', () => { + const {error, program} = createError( + `import {Foo} from './foo';`, 'Foo', 'Error message', + [{name: _('/foo.ts'), contents: 'export class Foo {}'}]); + const fooSf = getSourceFileOrError(program, _('/foo.ts')); + + if (typeof error.message === 'string') { + return fail('Created error must have a message chain'); + } + expect(error.message.messageText).toBe('Error message'); + expect(error.message.next!.length).toBe(1); + expect(error.message.next![0].messageText).toBe(`Value is a reference to 'Foo'.`); + + expect(error.relatedInformation).toBeDefined(); + expect(error.relatedInformation!.length).toBe(1); + expect(error.relatedInformation![0].messageText).toBe('Reference is declared here.'); + expect(error.relatedInformation![0].file!.fileName).toBe(fooSf.fileName); + expect(getSourceCode(error.relatedInformation![0])).toBe('Foo'); + }); + + it('should include a pointer for a reference to an anonymous declaration', () => { + const {error, program} = createError( + `import Foo from './foo';`, 'Foo', 'Error message', + [{name: _('/foo.ts'), contents: 'export default class {}'}]); + const fooSf = getSourceFileOrError(program, _('/foo.ts')); + + if (typeof error.message === 'string') { + return fail('Created error must have a message chain'); + } + expect(error.message.messageText).toBe('Error message'); + expect(error.message.next!.length).toBe(1); + expect(error.message.next![0].messageText) + .toBe(`Value is a reference to an anonymous declaration.`); + + expect(error.relatedInformation).toBeDefined(); + expect(error.relatedInformation!.length).toBe(1); + expect(error.relatedInformation![0].messageText).toBe('Reference is declared here.'); + expect(error.relatedInformation![0].file!.fileName).toBe(fooSf.fileName); + expect(getSourceCode(error.relatedInformation![0])).toBe('export default class {}'); + }); + + it('should include a representation of the value\'s type', () => { + const {error} = createError('', '{a: 2}', 'Error message'); + + if (typeof error.message === 'string') { + return fail('Created error must have a message chain'); + } + expect(error.message.messageText).toBe('Error message'); + expect(error.message.next!.length).toBe(1); + expect(error.message.next![0].messageText).toBe(`Value is of type '{ a: number }'.`); + + expect(error.relatedInformation).not.toBeDefined(); + }); + }); }); }); @@ -108,8 +101,7 @@ function getSourceCode(diag: ts.DiagnosticRelatedInformation): string { } function createError( - code: string, expr: string, messageText: string, - supportingFiles: TestFile[] = []): FatalDiagnosticError { + code: string, expr: string, messageText: string, supportingFiles: TestFile[] = []) { const {program} = makeProgram( [{name: _('/entry.ts'), contents: `${code}; const target$ = ${expr}`}, ...supportingFiles], /* options */ undefined, /* host */ undefined, /* checkForErrors */ false); @@ -121,5 +113,6 @@ function createError( const evaluator = new PartialEvaluator(reflectionHost, checker, /* dependencyTracker */ null); const value = evaluator.evaluate(valueExpr); - return createValueHasWrongTypeError(valueExpr, value, messageText); + const error = createValueHasWrongTypeError(valueExpr, value, messageText); + return {error, program}; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts index b4d95d1b5a..dd60007982 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts @@ -6,8 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {platform} from 'os'; - import {ErrorCode, ngErrorCode} from '../../diagnostics'; import {absoluteFrom, absoluteFromSourceFile, getSourceFileOrError} from '../../file_system'; import {runInEachFileSystem} from '../../file_system/testing'; @@ -15,7 +13,7 @@ import {OptimizeFor} from '../api'; import {getClass, setup, TestDeclaration} from './test_utils'; -runInEachFileSystem(os => { +runInEachFileSystem(() => { describe('TemplateTypeChecker', () => { it('should batch diagnostic operations when requested in WholeProgram mode', () => { const file1 = absoluteFrom('/file1.ts'); @@ -172,50 +170,47 @@ runInEachFileSystem(os => { expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INLINE_TCB_REQUIRED)); }); - // These tests are currently disabled when running in Windows mode as the assertions involving - // the filename attached to the diagnostic are suffering from a case-sensitivity issue. - if (os !== 'Windows' && platform() !== 'win32') { - it('should produce errors for components that require type constructor inlining', () => { - const fileName = absoluteFrom('/main.ts'); - const dirFile = absoluteFrom('/dir.ts'); - const {program, templateTypeChecker} = setup( - [ - { - fileName, - source: `export class Cmp {}`, - templates: {'Cmp': '
'}, - declarations: [{ - name: 'TestDir', - selector: '[dir]', - file: dirFile, - type: 'directive', - isGeneric: true, - }] - }, - { - fileName: dirFile, - source: ` - // A non-exported interface used as a type bound for a generic directive causes - // an inline type constructor to be required. - interface NotExported {} - export abstract class TestDir {}`, - templates: {}, - } - ], - {inlining: false}); - const sf = getSourceFileOrError(program, fileName); - const diags = templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram); - expect(diags.length).toBe(1); - expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INLINE_TYPE_CTOR_REQUIRED)); + it('should produce errors for components that require type constructor inlining', () => { + const fileName = absoluteFrom('/main.ts'); + const dirFile = absoluteFrom('/dir.ts'); + const {program, templateTypeChecker} = setup( + [ + { + fileName, + source: `export class Cmp {}`, + templates: {'Cmp': '
'}, + declarations: [{ + name: 'TestDir', + selector: '[dir]', + file: dirFile, + type: 'directive', + isGeneric: true, + }] + }, + { + fileName: dirFile, + source: ` + // A non-exported interface used as a type bound for a generic directive causes + // an inline type constructor to be required. + interface NotExported {} + export class TestDir {}`, + templates: {}, + } + ], + {inlining: false}); + const sf = getSourceFileOrError(program, fileName); + const diags = templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram); + expect(diags.length).toBe(1); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INLINE_TYPE_CTOR_REQUIRED)); - // The relatedInformation of the diagnostic should point to the directive which required - // the inline type constructor. - expect(diags[0].relatedInformation).not.toBeUndefined(); - expect(diags[0].relatedInformation!.length).toBe(1); - expect(diags[0].relatedInformation![0].file).not.toBeUndefined(); - expect(absoluteFromSourceFile(diags[0].relatedInformation![0].file!)).toBe(dirFile); - }); - } + // The relatedInformation of the diagnostic should point to the directive which required + // the inline type constructor. + const dirSf = getSourceFileOrError(program, dirFile); + expect(diags[0].relatedInformation).not.toBeUndefined(); + expect(diags[0].relatedInformation!.length).toBe(1); + expect(diags[0].relatedInformation![0].file).not.toBeUndefined(); + expect(absoluteFromSourceFile(diags[0].relatedInformation![0].file!)).toBe(dirSf.fileName); + }); }); describe('template overrides', () => {