Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a reusable in-process LSP test client (factored out of the fourslash harness) and adds an opt-in “replay” test to execute message scripts captured from fuzzing, plus a VS Code launch template entry to run that replay test.
Changes:
- Add
internal/testutil/lsptestutil.LSPClientto share LSP request/response plumbing across tests. - Refactor fourslash to use the shared LSP client helper.
- Add
cmd/tsgoreplay test (skipped unless-replayflag is provided) and a corresponding VS Code launch template.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/testutil/lsptestutil/lspclient.go | New shared LSP test client with async routing + typed request/notification helpers. |
| internal/fourslash/fourslash.go | Replace embedded fourslash LSP client plumbing with lsptestutil.LSPClient. |
| internal/fourslash/statebaseline.go | Update state baselining to use the refactored client/server access paths. |
| cmd/tsgo/replay_test.go | New opt-in replay test that reads a replay log and replays LSP messages against an in-process server. |
| .vscode/launch.template.json | Add a launch config for running the replay test. |
| scanner := bufio.NewScanner(f) | ||
|
|
||
| if !scanner.Scan() { | ||
| t.Fatalf("replay file is empty") | ||
| } | ||
|
|
||
| rootDirPlaceholder := "@PROJECT_ROOT@" | ||
| rootDirUriPlaceholder := "@PROJECT_ROOT_URI@" | ||
| firstLine := scanner.Bytes() | ||
| var initObj initialArguments | ||
| err = json.Unmarshal(firstLine, &initObj) | ||
| if err != nil { | ||
| t.Fatalf("failed to parse initial arguments: %v", err) | ||
| } | ||
|
|
||
| if initObj.RootDirPlaceholder != "" { | ||
| rootDirPlaceholder = initObj.RootDirPlaceholder | ||
| } | ||
| if initObj.RootDirUriPlaceholder != "" { | ||
| rootDirUriPlaceholder = initObj.RootDirUriPlaceholder | ||
| } | ||
|
|
||
| rootDirReplacer := strings.NewReplacer( | ||
| rootDirPlaceholder, *testDir, | ||
| rootDirUriPlaceholder, string(testDirUri), | ||
| ) | ||
|
|
||
| var messages []*rawMessage | ||
| var id int32 = 1 | ||
| for scanner.Scan() { | ||
| line := scanner.Text() | ||
| line = rootDirReplacer.Replace(line) | ||
| var rawMsg rawMessage | ||
| err := json.Unmarshal([]byte(line), &rawMsg) | ||
| if err != nil { | ||
| t.Fatalf("failed to parse message: %v", err) | ||
| } | ||
| messages = append(messages, &rawMsg) | ||
| } |
There was a problem hiding this comment.
The replay file is read using bufio.Scanner without increasing the buffer size or checking scanner.Err() after the scan loop. Scanner has a default ~64K token limit, so large JSON lines (common for LSP didChange payloads) will fail with ErrTooLong and silently truncate the replay. Set a larger buffer via scanner.Buffer(...) and check scanner.Err() after the loop to fail with a clear error.
Isn't it possible to run with I'm guessing the big issue is that |
Yes, but you also want the executable that has been built
You're right, I had forgotten about that, but another problem that was on my mind is that you want a binary that was compiled without optimizations, otherwise it's gonna be a bad time debugging it. In general it seemed simpler to me for the workflow to use a test in our repo, unless we think the fuzzer client is going to evolve into something complicated. |
Luckily, there is no optimization level - just differing build flags (e.g. |
|
Thinking about this some more, I think it's okay to have two replay scripts - one in the repo for ease of iteration, and one published on npm that's just JS/TS. |
There is, if you do |
|
I think we are saying the same thing - there's no |
|
The default build Go does is effectively |
|
This doesn't really have to be a test, though, since you can use a |
Making it not a test would slightly complicate reuse of the LSP client (which takes in a *testing.T). |
Yep, I wouldn't try and deal with that either. We'd have to introduce a new main package to do this, which is kind of icky, but not impossible within internal or something. |
3 participants
To debug a replay file obtained from the fuzzer:
launch.jsonwith a path to the replay file and the test directory (e.g./code/replay.txt,code/Project)This PR adds a test + launch task to run the commands present in a replay script obtained from the fuzzer. The test is skipped unless the
replayflag is provided.This is needed for us to be able to conveniently debug problems found by the fuzzer. With Strada, we used to be able to launch tsserver with
--inspect-brk=...and then attach to that process for debugging, but IIUC there isn't an equivalent thing in Go.I refactored the LSP client implementation out of fourslash for reuse in the replay test.