Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(language-service): reset MockHost after every spec instead of creating new LS #33200

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/language-service/test/BUILD.bazel
Expand Up @@ -13,7 +13,6 @@ ts_library(
"//packages/compiler",
"//packages/compiler-cli/test:test_utils",
"//packages/language-service",
"@npm//reflect-metadata",
"@npm//typescript",
],
)
Expand Down
20 changes: 6 additions & 14 deletions packages/language-service/test/definitions_spec.ts
Expand Up @@ -9,24 +9,17 @@
import * as ts from 'typescript';

import {createLanguageService} from '../src/language_service';
import {LanguageService} from '../src/types';
import {TypeScriptServiceHost} from '../src/typescript_host';

import {MockTypescriptHost} from './test_utils';

describe('definitions', () => {
let mockHost: MockTypescriptHost;
let service: ts.LanguageService;
let ngHost: TypeScriptServiceHost;
let ngService: LanguageService;

beforeEach(() => {
// Create a new mockHost every time to reset any files that are overridden.
mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts']);
service = ts.createLanguageService(mockHost);
ngHost = new TypeScriptServiceHost(mockHost, service);
ngService = createLanguageService(ngHost);
});
const mockHost = new MockTypescriptHost(['/app/main.ts']);
const service = ts.createLanguageService(mockHost);
const ngHost = new TypeScriptServiceHost(mockHost, service);
const ngService = createLanguageService(ngHost);

beforeEach(() => { mockHost.reset(); });

it('should be able to find field in an interpolation', () => {
const fileName = mockHost.addCode(`
Expand Down Expand Up @@ -201,7 +194,6 @@ describe('definitions', () => {
});

it('should be able to find an input provider', () => {
// '/app/parsing-cases.ts', 'tcName',
const fileName = mockHost.addCode(`
@Component({
template: '<test-comp ~{start-my}[«tcName»]="name"~{end-my}></div>'
Expand Down
18 changes: 6 additions & 12 deletions packages/language-service/test/diagnostics_spec.ts
Expand Up @@ -8,7 +8,6 @@

import * as ts from 'typescript';
import {createLanguageService} from '../src/language_service';
import * as ng from '../src/types';
import {TypeScriptServiceHost} from '../src/typescript_host';
import {MockTypescriptHost} from './test_utils';

Expand All @@ -29,17 +28,12 @@ const NG_FOR_CASES = '/app/ng-for-cases.ts';
const NG_IF_CASES = '/app/ng-if-cases.ts';

describe('diagnostics', () => {
let mockHost: MockTypescriptHost;
let ngHost: TypeScriptServiceHost;
let tsLS: ts.LanguageService;
let ngLS: ng.LanguageService;

beforeEach(() => {
mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts']);
tsLS = ts.createLanguageService(mockHost);
ngHost = new TypeScriptServiceHost(mockHost, tsLS);
ngLS = createLanguageService(ngHost);
});
const mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts']);
const tsLS = ts.createLanguageService(mockHost);
const ngHost = new TypeScriptServiceHost(mockHost, tsLS);
const ngLS = createLanguageService(ngHost);

beforeEach(() => { mockHost.reset(); });

it('should produce no diagnostics for test.ng', () => {
// there should not be any errors on existing external template
Expand Down
36 changes: 14 additions & 22 deletions packages/language-service/test/hover_spec.ts
Expand Up @@ -9,23 +9,17 @@
import * as ts from 'typescript';

import {createLanguageService} from '../src/language_service';
import {LanguageService} from '../src/types';
import {TypeScriptServiceHost} from '../src/typescript_host';

import {MockTypescriptHost} from './test_utils';

describe('hover', () => {
let mockHost: MockTypescriptHost;
let tsLS: ts.LanguageService;
let ngLSHost: TypeScriptServiceHost;
let ngLS: LanguageService;

beforeEach(() => {
mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts']);
tsLS = ts.createLanguageService(mockHost);
ngLSHost = new TypeScriptServiceHost(mockHost, tsLS);
ngLS = createLanguageService(ngLSHost);
});
const mockHost = new MockTypescriptHost(['/app/main.ts']);
ayazhafiz marked this conversation as resolved.
Show resolved Hide resolved
const tsLS = ts.createLanguageService(mockHost);
const ngLSHost = new TypeScriptServiceHost(mockHost, tsLS);
const ngLS = createLanguageService(ngLSHost);

beforeEach(() => { mockHost.reset(); });

it('should be able to find field in an interpolation', () => {
const fileName = mockHost.addCode(`
Expand Down Expand Up @@ -184,18 +178,16 @@ describe('hover', () => {

it('should be able to find the NgModule of a directive', () => {
const fileName = '/app/parsing-cases.ts';
mockHost.override(fileName, `
import {Directive} from '@angular/core';

@Directive({
selector: '[string-model]',
})
export class «StringModel» {}`);
const marker = mockHost.getReferenceMarkerFor(fileName, 'StringModel');
const quickInfo = ngLS.getHoverAt(fileName, marker.start);
const content = mockHost.readFile(fileName) !;
const position = content.indexOf('StringModel');
expect(position).toBeGreaterThan(0);
const quickInfo = ngLS.getHoverAt(fileName, position);
expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker);
expect(textSpan).toEqual({
start: position,
length: 'StringModel'.length,
});
expect(toText(displayParts)).toBe('(directive) AppModule.StringModel: class');
});
});
Expand Down
18 changes: 6 additions & 12 deletions packages/language-service/test/template_references_spec.ts
Expand Up @@ -15,18 +15,12 @@ import {TypeScriptServiceHost} from '../src/typescript_host';
import {MockTypescriptHost} from './test_utils';

describe('references', () => {
let documentRegistry = ts.createDocumentRegistry();
let mockHost: MockTypescriptHost;
let service: ts.LanguageService;
let ngHost: TypeScriptServiceHost;
let ngService: LanguageService = createLanguageService(undefined !);

beforeEach(() => {
mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts']);
service = ts.createLanguageService(mockHost, documentRegistry);
ngHost = new TypeScriptServiceHost(mockHost, service);
ngService = createLanguageService(ngHost);
});
const mockHost = new MockTypescriptHost(['/app/main.ts']);
const service = ts.createLanguageService(mockHost);
const ngHost = new TypeScriptServiceHost(mockHost, service);
const ngService = createLanguageService(ngHost);

beforeEach(() => { mockHost.reset(); });

it('should be able to get template references',
() => { expect(() => ngService.getTemplateReferences()).not.toThrow(); });
Expand Down
20 changes: 16 additions & 4 deletions packages/language-service/test/test_utils.ts
Expand Up @@ -92,7 +92,7 @@ const COMPILER_OPTIONS: Readonly<ts.CompilerOptions> = {
};

export class MockTypescriptHost implements ts.LanguageServiceHost {
private angularPath?: string;
private readonly angularPath: string;
private readonly nodeModulesPath: string;
private readonly scriptVersion = new Map<string, number>();
private readonly overrides = new Map<string, string>();
Expand Down Expand Up @@ -127,14 +127,16 @@ export class MockTypescriptHost implements ts.LanguageServiceHost {
}

addScript(fileName: string, content: string) {
if (this.scriptVersion.has(fileName)) {
throw new Error(`${fileName} is already in the root files.`);
}
this.scriptVersion.set(fileName, 0);
this.projectVersion++;
this.overrides.set(fileName, content);
this.overrideDirectory.add(path.dirname(fileName));
this.scriptNames.push(fileName);
}

forgetAngular() { this.angularPath = undefined; }

overrideOptions(options: Partial<ts.CompilerOptions>) {
this.options = {...this.options, ...options};
this.projectVersion++;
Expand Down Expand Up @@ -185,6 +187,16 @@ export class MockTypescriptHost implements ts.LanguageServiceHost {
* Reset the project to its original state, effectively removing all overrides.
*/
reset() {
// project version and script version must be monotonically increasing,
// they must not be reset to zero.
this.projectVersion++;
for (const fileName of this.overrides.keys()) {
const version = this.scriptVersion.get(fileName);
if (version === undefined) {
throw new Error(`No prior version found for ${fileName}`);
}
this.scriptVersion.set(fileName, version + 1);
}
// Remove overrides from scriptNames
let length = 0;
for (let i = 0; i < this.scriptNames.length; ++i) {
Expand Down Expand Up @@ -251,7 +263,7 @@ export class MockTypescriptHost implements ts.LanguageServiceHost {
return result;
}
}
if (this.angularPath && name.startsWith('/' + node_modules + at_angular)) {
if (name.startsWith('/' + node_modules + at_angular)) {
return this.myPath.posix.join(
this.angularPath, name.substr(node_modules.length + at_angular.length + 1));
}
Expand Down
1 change: 0 additions & 1 deletion packages/language-service/test/typescript_host_spec.ts
Expand Up @@ -6,7 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/

import 'reflect-metadata';
import * as ts from 'typescript';

import {TypeScriptServiceHost} from '../src/typescript_host';
Expand Down