-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Do not report config file errors if the file opened isn't from configured project and that project doesn't have the config errors #18959
Conversation
…red project and that project doesnt have the config errors Fixes #16635
@mhegazy or @andy-ms can you please take a look |
src/harness/unittests/telemetry.ts
Outdated
@@ -255,6 +255,17 @@ namespace ts.projectSystem { | |||
return events; | |||
} | |||
|
|||
getEventsWithName<T extends server.ProjectServiceEvent>(eventName: T["eventName"]): ReadonlyArray<T> { | |||
let events: T[]; | |||
removeWhere(this.events, event => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use filterMutate
instead of removeWhere
(#19082). But does this function need to mutate this.events
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes getEvent was written such that the already queried events would be removed. (and was written such that there will be only expected events present) getEventsWithName does same thing just ensures that we filter the list for only events asked
serverEventManager.checkEventCountOfType("configFileDiag", 1); | ||
for (const event of serverEventManager.events) { | ||
if (event.eventName === "configFileDiag") { | ||
assert.equal(event.data.configFileName, configFile.path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a bit confusion for the API users. it would make more sense to not send this error event at all, unless this file is part of the project.
…esnot belong to the configured project
07f3cd3
to
bb4abbd
Compare
} | ||
|
||
hasZeroEvent<T extends server.ProjectServiceEvent>(eventName: T["eventName"]) { | ||
const eventCount = countWhere(this.events, event => event.eventName === eventName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of getting a count, just loop through all events and assert that event.eventName !== eventName
.
ping @mhegazy and/or @andy-ms |
Fixes #16635