From 249725d4b72a0dcdd5524457c813330002ed07a3 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 4 Oct 2017 13:25:56 -0700 Subject: [PATCH 1/5] Do not report config file errors if the file opened isnt from configured project and that project doesnt have the config errors Fixes #16635 --- src/harness/unittests/telemetry.ts | 34 +++++++--- .../unittests/tsserverProjectSystem.ts | 67 ++++++++++++++++++- src/server/editorServices.ts | 28 +++++--- src/server/project.ts | 6 +- src/server/session.ts | 8 +-- .../reference/api/tsserverlibrary.d.ts | 1 + 6 files changed, 116 insertions(+), 28 deletions(-) diff --git a/src/harness/unittests/telemetry.ts b/src/harness/unittests/telemetry.ts index 7c09806632180..d2a54fdc1bb96 100644 --- a/src/harness/unittests/telemetry.ts +++ b/src/harness/unittests/telemetry.ts @@ -7,7 +7,7 @@ namespace ts.projectSystem { const file = makeFile("/a.js"); const et = new EventTracker([file]); et.service.openClientFile(file.path); - assert.equal(et.getEvents().length, 0); + assert.equal(et.getEventsWithName(ts.server.ProjectInfoTelemetryEvent).length, 0); }); it("only sends an event once", () => { @@ -25,12 +25,12 @@ namespace ts.projectSystem { et.service.openClientFile(file2.path); checkNumberOfProjects(et.service, { inferredProjects: 1 }); - assert.equal(et.getEvents().length, 0); + assert.equal(et.getEventsWithName(ts.server.ProjectInfoTelemetryEvent).length, 0); et.service.openClientFile(file.path); checkNumberOfProjects(et.service, { configuredProjects: 1, inferredProjects: 1 }); - assert.equal(et.getEvents().length, 0); + assert.equal(et.getEventsWithName(ts.server.ProjectInfoTelemetryEvent).length, 0); }); it("counts files by extension", () => { @@ -219,7 +219,7 @@ namespace ts.projectSystem { const et = new EventTracker([tsconfig, file]); et.host.getFileSize = () => server.maxProgramSizeForNonTsFiles + 1; et.service.openClientFile(file.path); - et.getEvent(server.ProjectLanguageServiceStateEvent, /*mayBeMore*/ true); + et.getEvent(server.ProjectLanguageServiceStateEvent); et.assertProjectInfoTelemetryEvent({ projectId: Harness.mockHash("/jsconfig.json"), fileStats: fileStats({ js: 1 }), @@ -255,6 +255,17 @@ namespace ts.projectSystem { return events; } + getEventsWithName(eventName: T["eventName"]): ReadonlyArray { + let events: T[]; + removeWhere(this.events, event => { + if (event.eventName === eventName) { + (events || (events = [])).push(event as T); + return true; + } + }); + return events || emptyArray; + } + assertProjectInfoTelemetryEvent(partial: Partial, configFile?: string): void { assert.deepEqual(this.getEvent(ts.server.ProjectInfoTelemetryEvent), { projectId: Harness.mockHash(configFile || "/tsconfig.json"), @@ -278,10 +289,17 @@ namespace ts.projectSystem { }); } - getEvent(eventName: T["eventName"], mayBeMore = false): T["data"] { - if (mayBeMore) { assert(this.events.length !== 0); } - else { assert.equal(this.events.length, 1); } - const event = this.events.shift(); + getEvent(eventName: T["eventName"]): T["data"] { + let event: server.ProjectServiceEvent; + removeWhere(this.events, e => { + if (e.eventName === eventName) { + if (event) { + assert(false, "more than one event found"); + } + event = e; + return true; + } + }); assert.equal(event.eventName, eventName); return event.data; } diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 06e2beeaaa6cf..e8b18ea3b5498 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -463,7 +463,7 @@ namespace ts.projectSystem { const { configFileName, configFileErrors } = projectService.openClientFile(file1.path); assert(configFileName, "should find config file"); - assert.isTrue(!configFileErrors, `expect no errors in config file, got ${JSON.stringify(configFileErrors)}`); + assert.isTrue(!configFileErrors || configFileErrors.length === 0, `expect no errors in config file, got ${JSON.stringify(configFileErrors)}`); checkNumberOfInferredProjects(projectService, 0); checkNumberOfConfiguredProjects(projectService, 1); @@ -503,7 +503,7 @@ namespace ts.projectSystem { const { configFileName, configFileErrors } = projectService.openClientFile(file1.path); assert(configFileName, "should find config file"); - assert.isTrue(!configFileErrors, `expect no errors in config file, got ${JSON.stringify(configFileErrors)}`); + assert.isTrue(!configFileErrors || configFileErrors.length === 0, `expect no errors in config file, got ${JSON.stringify(configFileErrors)}`); checkNumberOfInferredProjects(projectService, 0); checkNumberOfConfiguredProjects(projectService, 1); @@ -3169,6 +3169,69 @@ namespace ts.projectSystem { host.runQueuedTimeoutCallbacks(); serverEventManager.checkEventCountOfType("configFileDiag", 3); }); + + it("are generated when the config file doesnot include file opened but has errors", () => { + const serverEventManager = new TestServerEventManager(); + const file = { + path: "/a/b/app.ts", + content: "let x = 10" + }; + const file2 = { + path: "/a/b/test.ts", + content: "let x = 10" + }; + const configFile = { + path: "/a/b/tsconfig.json", + content: `{ + "compilerOptions": { + "foo": "bar", + "allowJS": true + }, + "files": ["app.ts"] + }` + }; + + const host = createServerHost([file, file2, libFile, configFile]); + const session = createSession(host, { + canUseEvents: true, + eventHandler: serverEventManager.handler + }); + openFilesForSession([file2], session); + serverEventManager.checkEventCountOfType("configFileDiag", 1); + for (const event of serverEventManager.events) { + if (event.eventName === "configFileDiag") { + assert.equal(event.data.configFileName, configFile.path); + assert.equal(event.data.triggerFile, file2.path); + return; + } + } + }); + + it("are not generated when the config file doesnot include file opened and doesnt contain any errors", () => { + const serverEventManager = new TestServerEventManager(); + const file = { + path: "/a/b/app.ts", + content: "let x = 10" + }; + const file2 = { + path: "/a/b/test.ts", + content: "let x = 10" + }; + const configFile = { + path: "/a/b/tsconfig.json", + content: `{ + "files": ["app.ts"] + }` + }; + + const host = createServerHost([file, file2, libFile, configFile]); + const session = createSession(host, { + canUseEvents: true, + eventHandler: serverEventManager.handler + }); + openFilesForSession([file2], session); + serverEventManager.checkEventCountOfType("configFileDiag", 0); + }); }); describe("skipLibCheck", () => { diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 25a285929bef2..32358ba3c4c3d 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1561,14 +1561,17 @@ namespace ts.server { project.watchWildcards(projectOptions.wildcardDirectories); } this.updateNonInferredProject(project, projectOptions.files, fileNamePropertyReader, projectOptions.compilerOptions, projectOptions.typeAcquisition, projectOptions.compileOnSave); + this.sendConfigFileDiagEvent(project, configFileName); + } + private sendConfigFileDiagEvent(project: ConfiguredProject, triggerFile: NormalizedPath) { if (!this.eventHandler) { return; } this.eventHandler({ eventName: ConfigFileDiagEvent, - data: { configFileName, diagnostics: project.getGlobalProjectErrors() || [], triggerFile: configFileName } + data: { configFileName: project.getConfigFilePath(), diagnostics: project.getAllProjectErrors(), triggerFile } }); } @@ -1888,6 +1891,7 @@ namespace ts.server { openClientFileWithNormalizedPath(fileName: NormalizedPath, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean, projectRootPath?: NormalizedPath): OpenConfiguredProjectResult { let configFileName: NormalizedPath; + let sendConfigFileDiagEvent = false; let configFileErrors: ReadonlyArray; const info = this.getOrCreateScriptInfoOpenedByClientForNormalizedPath(fileName, fileContent, scriptKind, hasMixedContent); @@ -1898,14 +1902,8 @@ namespace ts.server { project = this.findConfiguredProjectByProjectName(configFileName); if (!project) { project = this.createConfiguredProject(configFileName); - - // even if opening config file was successful, it could still - // contain errors that were tolerated. - const errors = project.getGlobalProjectErrors(); - if (errors && errors.length > 0) { - // set configFileErrors only when the errors array is non-empty - configFileErrors = errors; - } + // Send the event only if the project got created as part of this open request + sendConfigFileDiagEvent = true; } } } @@ -1919,10 +1917,21 @@ namespace ts.server { // At this point if file is part of any any configured or external project, then it would be present in the containing projects // So if it still doesnt have any containing projects, it needs to be part of inferred project if (info.isOrphan()) { + // Since the file isnt part of configured project, + // report config file and its error only if config file found had errors (and hence may be didnt include the file) + if (sendConfigFileDiagEvent && !project.getAllProjectErrors().length) { + configFileName = undefined; + sendConfigFileDiagEvent = false; + } this.assignOrphanScriptInfoToInferredProject(info, projectRootPath); } this.addToListOfOpenFiles(info); + if (sendConfigFileDiagEvent) { + configFileErrors = project.getAllProjectErrors(); + this.sendConfigFileDiagEvent(project as ConfiguredProject, fileName); + } + // Remove the configured projects that have zero references from open files. // This was postponed from closeOpenFile to after opening next file, // so that we can reuse the project if we need to right away @@ -1938,6 +1947,7 @@ namespace ts.server { // the file from that old project is reopened because of opening file from here. this.deleteOrphanScriptInfoNotInAnyProject(); this.printProjects(); + return { configFileName, configFileErrors }; } diff --git a/src/server/project.ts b/src/server/project.ts index 655f3ed0cfcaf..203355ad9db16 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1271,14 +1271,14 @@ namespace ts.server { * Get the errors that dont have any file name associated */ getGlobalProjectErrors(): ReadonlyArray { - return filter(this.projectErrors, diagnostic => !diagnostic.file); + return filter(this.projectErrors, diagnostic => !diagnostic.file) || emptyArray; } /** * Get all the project errors */ getAllProjectErrors(): ReadonlyArray { - return this.projectErrors; + return this.projectErrors || emptyArray; } setProjectErrors(projectErrors: Diagnostic[]) { @@ -1335,6 +1335,8 @@ namespace ts.server { } this.stopWatchingWildCards(); + this.projectErrors = undefined; + this.configFileSpecs = undefined; } addOpenRef() { diff --git a/src/server/session.ts b/src/server/session.ts index 57dac7ec799a0..59762f8191b81 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -969,13 +969,7 @@ namespace ts.server { * @param fileContent is a version of the file content that is known to be more up to date than the one on disk */ private openClientFile(fileName: NormalizedPath, fileContent?: string, scriptKind?: ScriptKind, projectRootPath?: NormalizedPath) { - const { configFileName, configFileErrors } = this.projectService.openClientFileWithNormalizedPath(fileName, fileContent, scriptKind, /*hasMixedContent*/ false, projectRootPath); - if (this.eventHandler) { - this.eventHandler({ - eventName: "configFileDiag", - data: { triggerFile: fileName, configFileName, diagnostics: configFileErrors || emptyArray } - }); - } + this.projectService.openClientFileWithNormalizedPath(fileName, fileContent, scriptKind, /*hasMixedContent*/ false, projectRootPath); } private getPosition(args: protocol.FileLocationRequestArgs, scriptInfo: ScriptInfo): number { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 1feb6b7d073f6..b9b72dec922af 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -7517,6 +7517,7 @@ declare namespace ts.server { private createConfiguredProject(configFileName); private updateNonInferredProjectFiles(project, files, propertyReader); private updateNonInferredProject(project, newUncheckedFiles, propertyReader, newOptions, newTypeAcquisition, compileOnSave); + private sendConfigFileDiagEvent(project, triggerFile); private getOrCreateInferredProjectForProjectRootPathIfEnabled(info, projectRootPath); private getOrCreateSingleInferredProjectIfEnabled(); private createInferredProject(rootDirectoryForResolution, isSingleInferredProject?, projectRootPath?); From c5b4f5e7e72516f2cb946a189e1b06fed17ef199 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 10 Oct 2017 16:28:44 -0700 Subject: [PATCH 2/5] Use filterMutate instead of removeWhere --- src/harness/unittests/telemetry.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/harness/unittests/telemetry.ts b/src/harness/unittests/telemetry.ts index d2a54fdc1bb96..25120af45c1d2 100644 --- a/src/harness/unittests/telemetry.ts +++ b/src/harness/unittests/telemetry.ts @@ -257,11 +257,12 @@ namespace ts.projectSystem { getEventsWithName(eventName: T["eventName"]): ReadonlyArray { let events: T[]; - removeWhere(this.events, event => { + filterMutate(this.events, event => { if (event.eventName === eventName) { (events || (events = [])).push(event as T); - return true; + return false; } + return true; }); return events || emptyArray; } @@ -291,14 +292,15 @@ namespace ts.projectSystem { getEvent(eventName: T["eventName"]): T["data"] { let event: server.ProjectServiceEvent; - removeWhere(this.events, e => { + filterMutate(this.events, e => { if (e.eventName === eventName) { if (event) { assert(false, "more than one event found"); } event = e; - return true; + return false; } + return true; }); assert.equal(event.eventName, eventName); return event.data; From bb4abbd95ecb20ba3e7e4ca12dfe41b1c80533c5 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 10 Oct 2017 17:37:02 -0700 Subject: [PATCH 3/5] Do not generate config file diagnostics event when the file opened doesnot belong to the configured project --- src/harness/unittests/tsserverProjectSystem.ts | 11 ++--------- src/server/editorServices.ts | 10 ++++------ 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 3d244fd30f8ba..04b848e7cdf71 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3168,7 +3168,7 @@ namespace ts.projectSystem { serverEventManager.checkEventCountOfType("configFileDiag", 3); }); - it("are generated when the config file doesnot include file opened but has errors", () => { + it("are not generated when the config file doesnot include file opened and config file has errors", () => { const serverEventManager = new TestServerEventManager(); const file = { path: "/a/b/app.ts", @@ -3195,14 +3195,7 @@ namespace ts.projectSystem { eventHandler: serverEventManager.handler }); openFilesForSession([file2], session); - serverEventManager.checkEventCountOfType("configFileDiag", 1); - for (const event of serverEventManager.events) { - if (event.eventName === "configFileDiag") { - assert.equal(event.data.configFileName, configFile.path); - assert.equal(event.data.triggerFile, file2.path); - return; - } - } + serverEventManager.checkEventCountOfType("configFileDiag", 0); }); it("are not generated when the config file doesnot include file opened and doesnt contain any errors", () => { diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index fd2297473908a..765a31884437d 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1933,12 +1933,10 @@ namespace ts.server { // At this point if file is part of any any configured or external project, then it would be present in the containing projects // So if it still doesnt have any containing projects, it needs to be part of inferred project if (info.isOrphan()) { - // Since the file isnt part of configured project, - // report config file and its error only if config file found had errors (and hence may be didnt include the file) - if (sendConfigFileDiagEvent && !project.getAllProjectErrors().length) { - configFileName = undefined; - sendConfigFileDiagEvent = false; - } + // Since the file isnt part of configured project, do not send config file event + configFileName = undefined; + sendConfigFileDiagEvent = false; + this.assignOrphanScriptInfoToInferredProject(info, projectRootPath); } this.addToListOfOpenFiles(info); From 0e2eb3a2b88628680ca3ef56703d3679a8439f80 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 10 Oct 2017 18:25:26 -0700 Subject: [PATCH 4/5] Combine the event manager testing --- src/harness/unittests/telemetry.ts | 98 ++--------- .../unittests/tsserverProjectSystem.ts | 154 ++++++++++-------- 2 files changed, 101 insertions(+), 151 deletions(-) diff --git a/src/harness/unittests/telemetry.ts b/src/harness/unittests/telemetry.ts index 25120af45c1d2..9bb2db738013d 100644 --- a/src/harness/unittests/telemetry.ts +++ b/src/harness/unittests/telemetry.ts @@ -5,9 +5,9 @@ namespace ts.projectSystem { describe("project telemetry", () => { it("does nothing for inferred project", () => { const file = makeFile("/a.js"); - const et = new EventTracker([file]); + const et = new TestServerEventManager([file]); et.service.openClientFile(file.path); - assert.equal(et.getEventsWithName(ts.server.ProjectInfoTelemetryEvent).length, 0); + et.hasZeroEvent(ts.server.ProjectInfoTelemetryEvent); }); it("only sends an event once", () => { @@ -15,7 +15,7 @@ namespace ts.projectSystem { const file2 = makeFile("/b.ts"); const tsconfig = makeFile("/a/tsconfig.json", {}); - const et = new EventTracker([file, file2, tsconfig]); + const et = new TestServerEventManager([file, file2, tsconfig]); et.service.openClientFile(file.path); et.assertProjectInfoTelemetryEvent({}, tsconfig.path); @@ -25,12 +25,12 @@ namespace ts.projectSystem { et.service.openClientFile(file2.path); checkNumberOfProjects(et.service, { inferredProjects: 1 }); - assert.equal(et.getEventsWithName(ts.server.ProjectInfoTelemetryEvent).length, 0); + et.hasZeroEvent(ts.server.ProjectInfoTelemetryEvent); et.service.openClientFile(file.path); checkNumberOfProjects(et.service, { configuredProjects: 1, inferredProjects: 1 }); - assert.equal(et.getEventsWithName(ts.server.ProjectInfoTelemetryEvent).length, 0); + et.hasZeroEvent(ts.server.ProjectInfoTelemetryEvent); }); it("counts files by extension", () => { @@ -39,7 +39,7 @@ namespace ts.projectSystem { const compilerOptions: ts.CompilerOptions = { allowJs: true }; const tsconfig = makeFile("/tsconfig.json", { compilerOptions, include: ["src"] }); - const et = new EventTracker([...files, notIncludedFile, tsconfig]); + const et = new TestServerEventManager([...files, notIncludedFile, tsconfig]); et.service.openClientFile(files[0].path); et.assertProjectInfoTelemetryEvent({ fileStats: { ts: 2, tsx: 1, js: 1, jsx: 1, dts: 1 }, @@ -50,7 +50,7 @@ namespace ts.projectSystem { it("works with external project", () => { const file1 = makeFile("/a.ts"); - const et = new EventTracker([file1]); + const et = new TestServerEventManager([file1]); const compilerOptions: ts.server.protocol.CompilerOptions = { strict: true }; const projectFileName = "/hunter2/foo.csproj"; @@ -148,7 +148,7 @@ namespace ts.projectSystem { (compilerOptions as any).unknownCompilerOption = "hunter2"; // These are always ignored. const tsconfig = makeFile("/tsconfig.json", { compilerOptions, files: ["/a.ts"] }); - const et = new EventTracker([file, tsconfig]); + const et = new TestServerEventManager([file, tsconfig]); et.service.openClientFile(file.path); et.assertProjectInfoTelemetryEvent({ @@ -168,7 +168,7 @@ namespace ts.projectSystem { compileOnSave: true, }); - const et = new EventTracker([tsconfig, file]); + const et = new TestServerEventManager([tsconfig, file]); et.service.openClientFile(file.path); et.assertProjectInfoTelemetryEvent({ extends: true, @@ -198,7 +198,7 @@ namespace ts.projectSystem { exclude: [], }, }); - const et = new EventTracker([jsconfig, file]); + const et = new TestServerEventManager([jsconfig, file]); et.service.openClientFile(file.path); et.assertProjectInfoTelemetryEvent({ projectId: Harness.mockHash("/jsconfig.json"), @@ -216,7 +216,7 @@ namespace ts.projectSystem { it("detects whether language service was disabled", () => { const file = makeFile("/a.js"); const tsconfig = makeFile("/jsconfig.json", {}); - const et = new EventTracker([tsconfig, file]); + const et = new TestServerEventManager([tsconfig, file]); et.host.getFileSize = () => server.maxProgramSizeForNonTsFiles + 1; et.service.openClientFile(file.path); et.getEvent(server.ProjectLanguageServiceStateEvent); @@ -235,83 +235,7 @@ namespace ts.projectSystem { }); }); - class EventTracker { - private events: server.ProjectServiceEvent[] = []; - readonly service: TestProjectService; - readonly host: projectSystem.TestServerHost; - - constructor(files: projectSystem.FileOrFolder[]) { - this.host = createServerHost(files); - this.service = createProjectService(this.host, { - eventHandler: event => { - this.events.push(event); - }, - }); - } - - getEvents(): ReadonlyArray { - const events = this.events; - this.events = []; - return events; - } - - getEventsWithName(eventName: T["eventName"]): ReadonlyArray { - let events: T[]; - filterMutate(this.events, event => { - if (event.eventName === eventName) { - (events || (events = [])).push(event as T); - return false; - } - return true; - }); - return events || emptyArray; - } - - assertProjectInfoTelemetryEvent(partial: Partial, configFile?: string): void { - assert.deepEqual(this.getEvent(ts.server.ProjectInfoTelemetryEvent), { - projectId: Harness.mockHash(configFile || "/tsconfig.json"), - fileStats: fileStats({ ts: 1 }), - compilerOptions: {}, - extends: false, - files: false, - include: false, - exclude: false, - compileOnSave: false, - typeAcquisition: { - enable: false, - exclude: false, - include: false, - }, - configFileName: "tsconfig.json", - projectType: "configured", - languageServiceEnabled: true, - version: ts.version, - ...partial, - }); - } - - getEvent(eventName: T["eventName"]): T["data"] { - let event: server.ProjectServiceEvent; - filterMutate(this.events, e => { - if (e.eventName === eventName) { - if (event) { - assert(false, "more than one event found"); - } - event = e; - return false; - } - return true; - }); - assert.equal(event.eventName, eventName); - return event.data; - } - } - function makeFile(path: string, content: {} = ""): projectSystem.FileOrFolder { return { path, content: isString(content) ? "" : JSON.stringify(content) }; } - - function fileStats(nonZeroStats: Partial): server.FileStats { - return { ts: 0, tsx: 0, dts: 0, js: 0, jsx: 0, ...nonZeroStats }; - } } diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 04b848e7cdf71..d5b9b9d62fcf4 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -132,16 +132,78 @@ namespace ts.projectSystem { return map(fileNames, toExternalFile); } - class TestServerEventManager { - public events: server.ProjectServiceEvent[] = []; + export function fileStats(nonZeroStats: Partial): server.FileStats { + return { ts: 0, tsx: 0, dts: 0, js: 0, jsx: 0, ...nonZeroStats }; + } + + export class TestServerEventManager { + private events: server.ProjectServiceEvent[] = []; + readonly session: TestSession; + readonly service: server.ProjectService; + readonly host: projectSystem.TestServerHost; + constructor(files: projectSystem.FileOrFolder[]) { + this.host = createServerHost(files); + this.session = createSession(this.host, { + canUseEvents: true, + eventHandler: event => this.events.push(event), + }); + this.service = this.session.getProjectService(); + } + + getEvents(): ReadonlyArray { + const events = this.events; + this.events = []; + return events; + } + + getEvent(eventName: T["eventName"]): T["data"] { + let eventData: T["data"]; + filterMutate(this.events, e => { + if (e.eventName === eventName) { + if (eventData !== undefined) { + assert(false, "more than one event found"); + } + eventData = e.data; + return false; + } + return true; + }); + assert.isDefined(eventData); + return eventData; + } + + hasZeroEvent(eventName: T["eventName"]) { + const eventCount = countWhere(this.events, event => event.eventName === eventName); + assert.equal(eventCount, 0); + } - handler: server.ProjectServiceEventHandler = (event: server.ProjectServiceEvent) => { - this.events.push(event); + checkSingleConfigFileDiagEvent(configFileName: string, triggerFile: string) { + const eventData = this.getEvent(server.ConfigFileDiagEvent); + assert.equal(eventData.configFileName, configFileName); + assert.equal(eventData.triggerFile, triggerFile); } - checkEventCountOfType(eventType: "configFileDiag", expectedCount: number) { - const eventsOfType = filter(this.events, e => e.eventName === eventType); - assert.equal(eventsOfType.length, expectedCount, `The actual event counts of type ${eventType} is ${eventsOfType.length}, while expected ${expectedCount}`); + assertProjectInfoTelemetryEvent(partial: Partial, configFile?: string): void { + assert.deepEqual(this.getEvent(ts.server.ProjectInfoTelemetryEvent), { + projectId: Harness.mockHash(configFile || "/tsconfig.json"), + fileStats: fileStats({ ts: 1 }), + compilerOptions: {}, + extends: false, + files: false, + include: false, + exclude: false, + compileOnSave: false, + typeAcquisition: { + enable: false, + exclude: false, + include: false, + }, + configFileName: "tsconfig.json", + projectType: "configured", + languageServiceEnabled: true, + version: ts.version, + ...partial, + }); } } @@ -3076,7 +3138,6 @@ namespace ts.projectSystem { describe("Configure file diagnostics events", () => { it("are generated when the config file has errors", () => { - const serverEventManager = new TestServerEventManager(); const file = { path: "/a/b/app.ts", content: "let x = 10" @@ -3090,26 +3151,12 @@ namespace ts.projectSystem { } }` }; - - const host = createServerHost([file, configFile]); - const session = createSession(host, { - canUseEvents: true, - eventHandler: serverEventManager.handler - }); - openFilesForSession([file], session); - serverEventManager.checkEventCountOfType("configFileDiag", 1); - - for (const event of serverEventManager.events) { - if (event.eventName === "configFileDiag") { - assert.equal(event.data.configFileName, configFile.path); - assert.equal(event.data.triggerFile, file.path); - return; - } - } + const serverEventManager = new TestServerEventManager([file, configFile]); + openFilesForSession([file], serverEventManager.session); + serverEventManager.checkSingleConfigFileDiagEvent(configFile.path, file.path); }); it("are generated when the config file doesn't have errors", () => { - const serverEventManager = new TestServerEventManager(); const file = { path: "/a/b/app.ts", content: "let x = 10" @@ -3120,18 +3167,12 @@ namespace ts.projectSystem { "compilerOptions": {} }` }; - - const host = createServerHost([file, configFile]); - const session = createSession(host, { - canUseEvents: true, - eventHandler: serverEventManager.handler - }); - openFilesForSession([file], session); - serverEventManager.checkEventCountOfType("configFileDiag", 1); + const serverEventManager = new TestServerEventManager([file, configFile]); + openFilesForSession([file], serverEventManager.session); + serverEventManager.checkSingleConfigFileDiagEvent(configFile.path, file.path); }); it("are generated when the config file changes", () => { - const serverEventManager = new TestServerEventManager(); const file = { path: "/a/b/app.ts", content: "let x = 10" @@ -3143,33 +3184,28 @@ namespace ts.projectSystem { }` }; - const host = createServerHost([file, configFile]); - const session = createSession(host, { - canUseEvents: true, - eventHandler: serverEventManager.handler - }); - openFilesForSession([file], session); - serverEventManager.checkEventCountOfType("configFileDiag", 1); + const serverEventManager = new TestServerEventManager([file, configFile]); + openFilesForSession([file], serverEventManager.session); + serverEventManager.checkSingleConfigFileDiagEvent(configFile.path, file.path); configFile.content = `{ "compilerOptions": { "haha": 123 } }`; - host.reloadFS([file, configFile]); - host.runQueuedTimeoutCallbacks(); - serverEventManager.checkEventCountOfType("configFileDiag", 2); + serverEventManager.host.reloadFS([file, configFile]); + serverEventManager.host.runQueuedTimeoutCallbacks(); + serverEventManager.checkSingleConfigFileDiagEvent(configFile.path, configFile.path); configFile.content = `{ "compilerOptions": {} }`; - host.reloadFS([file, configFile]); - host.runQueuedTimeoutCallbacks(); - serverEventManager.checkEventCountOfType("configFileDiag", 3); + serverEventManager.host.reloadFS([file, configFile]); + serverEventManager.host.runQueuedTimeoutCallbacks(); + serverEventManager.checkSingleConfigFileDiagEvent(configFile.path, configFile.path); }); it("are not generated when the config file doesnot include file opened and config file has errors", () => { - const serverEventManager = new TestServerEventManager(); const file = { path: "/a/b/app.ts", content: "let x = 10" @@ -3188,18 +3224,12 @@ namespace ts.projectSystem { "files": ["app.ts"] }` }; - - const host = createServerHost([file, file2, libFile, configFile]); - const session = createSession(host, { - canUseEvents: true, - eventHandler: serverEventManager.handler - }); - openFilesForSession([file2], session); - serverEventManager.checkEventCountOfType("configFileDiag", 0); + const serverEventManager = new TestServerEventManager([file, file2, libFile, configFile]); + openFilesForSession([file2], serverEventManager.session); + serverEventManager.hasZeroEvent("configFileDiag"); }); it("are not generated when the config file doesnot include file opened and doesnt contain any errors", () => { - const serverEventManager = new TestServerEventManager(); const file = { path: "/a/b/app.ts", content: "let x = 10" @@ -3215,13 +3245,9 @@ namespace ts.projectSystem { }` }; - const host = createServerHost([file, file2, libFile, configFile]); - const session = createSession(host, { - canUseEvents: true, - eventHandler: serverEventManager.handler - }); - openFilesForSession([file2], session); - serverEventManager.checkEventCountOfType("configFileDiag", 0); + const serverEventManager = new TestServerEventManager([file, file2, libFile, configFile]); + openFilesForSession([file2], serverEventManager.session); + serverEventManager.hasZeroEvent("configFileDiag"); }); }); From cf9b83accc62833a109e48f03463bb1c02a5f767 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 10 Oct 2017 21:15:20 -0700 Subject: [PATCH 5/5] Instead of counting events with name, verify each event to not equal event name --- src/harness/unittests/tsserverProjectSystem.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index d5b9b9d62fcf4..af85e21260aea 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -173,8 +173,7 @@ namespace ts.projectSystem { } hasZeroEvent(eventName: T["eventName"]) { - const eventCount = countWhere(this.events, event => event.eventName === eventName); - assert.equal(eventCount, 0); + this.events.forEach(event => assert.notEqual(event.eventName, eventName)); } checkSingleConfigFileDiagEvent(configFileName: string, triggerFile: string) {