Skip to content

Misc LSP server / fourslash testing improvements #1591

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.

Sign up for GitHub

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Misc LSP server / fourslash testing improvements #1591

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2d65be3
Write fourslash stderr to t.Output
jakebailey Aug 15, 2025
e624abe
Use test context
jakebailey Aug 15, 2025
8762147
Coordinate LSP server exit error
jakebailey Aug 15, 2025
177d586
Move ctx up to the top where it should be
jakebailey Aug 15, 2025
d2e6291
Wow okay I guess this fixes a bunch of tests
jakebailey Aug 15, 2025
55847b6
Fix race
jakebailey Aug 15, 2025
87fa358
Merge branch 'main' into jabaile/fourslash-testing-improvements
jakebailey Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cmd/tsgo/lsp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"context"
"flag"
"fmt"
"os"
Expand All @@ -14,7 +15,7 @@ import (
"github.com/microsoft/typescript-go/internal/vfs/osvfs"
)

func runLSP(args []string) int {
func runLSP(ctx context.Context, args []string) int {
flag := flag.NewFlagSet("lsp", flag.ContinueOnError)
stdio := flag.Bool("stdio", false, "use stdio for communication")
pprofDir := flag.String("pprofDir", "", "Generate pprof CPU/memory profiles to the given directory.")
Expand Down Expand Up @@ -51,7 +52,7 @@ func runLSP(args []string) int {
TypingsLocation: typingsLocation,
})

if err := s.Run(); err != nil {
if err := s.Run(ctx); err != nil {
return 1
}
return 0
Expand Down
8 changes: 7 additions & 1 deletion cmd/tsgo/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package main

import (
"context"
"os"
"os/signal"
"syscall"

"github.com/microsoft/typescript-go/internal/execute"
)
Expand All @@ -11,11 +14,14 @@ func main() {
}

func runMain() int {
ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
defer cancel()

args := os.Args[1:]
if len(args) > 0 {
switch args[0] {
case "--lsp":
return runLSP(args[1:])
return runLSP(ctx, args[1:])
case "--api":
return runAPI(args[1:])
}
Expand Down
24 changes: 18 additions & 6 deletions internal/fourslash/fourslash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package fourslash

import (
"context"
"fmt"
"io"
"maps"
Expand Down Expand Up @@ -140,6 +141,8 @@ func NewFourslash(t *testing.T, capabilities *lsproto.ClientCapabilities, conten
// Just skip this for now.
t.Skip("bundled files are not embedded")
}
ctx := t.Context()

fileName := getFileNameFromTest(t)
testfs := make(map[string]string)
scriptInfos := make(map[string]*scriptInfo)
Expand All @@ -159,11 +162,10 @@ func NewFourslash(t *testing.T, capabilities *lsproto.ClientCapabilities, conten
outputReader, outputWriter := newLSPPipe()
fs := bundled.WrapFS(vfstest.FromMap(testfs, true /*useCaseSensitiveFileNames*/))

var err strings.Builder
server := lsp.NewServer(&lsp.ServerOptions{
In: inputReader,
Out: outputWriter,
Err: &err,
Err: t.Output(),

Cwd: "/",
FS: fs,
Expand All @@ -172,14 +174,14 @@ func NewFourslash(t *testing.T, capabilities *lsproto.ClientCapabilities, conten
ParsedFileCache: &parsedFileCache{},
})

lspCtx, lspCancel := context.WithCancel(ctx)
lspErrChan := make(chan error, 1)

go func() {
defer func() {
outputWriter.Close()
}()
err := server.Run()
if err != nil {
t.Error("server error:", err)
}
lspErrChan <- server.Run(lspCtx)
}()

converters := ls.NewConverters(lsproto.PositionEncodingKindUTF8, func(fileName string) *ls.LineMap {
Expand Down Expand Up @@ -210,7 +212,17 @@ func NewFourslash(t *testing.T, capabilities *lsproto.ClientCapabilities, conten
f.activeFilename = f.testData.Files[0].fileName

t.Cleanup(func() {
lspCancel()
inputWriter.Close()

select {
case <-ctx.Done():
// do nothing
case err := <-lspErrChan:
Copy link
Preview

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a potential race condition where the LSP server might complete before the test cleanup runs, causing the channel read to block indefinitely. Consider using a select with a timeout or making the channel read non-blocking with a default case.

Copilot uses AI. Check for mistakes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The channel is buffered above. And if the server never exits, that's a bug and the test should time out / report a deadlock.

if err != nil && lspCtx.Err() == nil {
t.Errorf("LSP server exited with error: %v", err)
}
}
})
return f
}
Expand Down
8 changes: 1 addition & 7 deletions internal/lsp/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@ import (
"errors"
"fmt"
"io"
"os"
"os/signal"
"runtime/debug"
"slices"
"sync"
"sync/atomic"
"syscall"

"github.com/go-json-experiment/json"
"github.com/microsoft/typescript-go/internal/collections"
Expand Down Expand Up @@ -249,10 +246,7 @@ func (s *Server) RefreshDiagnostics(ctx context.Context) error {
return nil
}

func (s *Server) Run() error {
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
defer stop()

func (s *Server) Run(ctx context.Context) error {
g, ctx := errgroup.WithContext(ctx)
g.Go(func() error { return s.dispatchLoop(ctx) })
g.Go(func() error { return s.writeLoop(ctx) })
Expand Down
Loading