Make tsserver entrypoint work for ts-go + refactor#210
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Forgot to update the reporting logic, will fix that. |
|
PR seems to work, at least when triggered manually: https://typescript.visualstudio.com/TypeScript/_build/results?buildId=167572&view=results |
|
This is what you get when you run the bot command, right? Do you have triggerer update to allow us to start triggering it on PRs? |
It's what you get from the bot, yes. |
|
This one it should be able to do, if the code is sent to enable it. |
This reverts commit f68de46.
| if (method === "window/logMessage" && params?.type === 1) { | ||
| lastErrorLogMessage = params.message; | ||
| } | ||
| if (method !== "window/logMessage") { |
There was a problem hiding this comment.
This was making the logs too large to load.
| console.log(JSON.stringify({ method: "unknown", message: errorMessage })); | ||
| console.error("Server connection closed prematurely:", e); | ||
| const stderrOutput = stderrChunks.join(""); | ||
| const panicMsg = getPanicMessageFromStderr(stderrOutput); |
There was a problem hiding this comment.
This is needed to detect unrecovered panics.
|
|
||
| function sendRequestUntyped(method: string, params: object): Promise<unknown> { | ||
| function sendRequestUntyped(method: string, params: unknown): Promise<unknown> { | ||
| if (params === undefined) { |
There was a problem hiding this comment.
Fix needed otherwise the vscode library would send [null] as parameter if params is undefined, causing a server deserialization error.
| Raw error text: <code>${summary.rawErrorArtifactPath}</code> in the <a href="${artifactFolderUrlPlaceholder}">artifact folder</a> <br /> | ||
| Replay commands: <code>${summary.replayScriptArtifactPath}</code> in the <a href="${artifactFolderUrlPlaceholder}">artifact folder</a> | ||
| <h4>Last few requests</h4> | ||
| `; |
There was a problem hiding this comment.
When we report an error on the new server, we now say what happened with the old server too, to make it easier to reason about possibly flaky results.
iisaduan
left a comment
iisaduan
left a comment
There was a problem hiding this comment.
To be clear, we have
- scheduled
- tsc (diffs latest and latest stable)
- fuzzer (runs fuzzer on latest)
- manual
- tsc (diffs main and branch)
- tsserver (diffs main and branch)
(is that correct?)
Yeah, that's what we should have for now modulo possible bugs. |
4 participants
This PR makes ts-error-deltas work with entry point
tsserverand repotypescript-go.exerciseLspServer, except we follow the provided replay script.isGoparameter that is not really needed but makes it easier to track where the code changed to support tsgo. I think this will make it easier to delete the old Strada-supporting code once we have finished porting everything here.I also renamed a few things:
lspis nowfuzzerto avoid confusion withtsserver. We still need to have a separate entry point for fuzzing for now because it indicates we're not comparing two versions, just testing the most current one.githubanduserscripts/params toscheduledandtriggered. The old names were extremely confusing for me, because the names were not really meaningful, and both of those scripts could either test the "user tests" (a.k.a the repos in theuserTestsdirectory), or the "top N" github repos, depending on what's passed as repo list to the tests. The real distinction is that one script runs triggered by the bot for comparing a PR with main, and the other runs scheduled to compare two released versions.There's also a few fixes to
exerciseLspServer/lspHarness, and to tests' tsconfig.