[ty] Implement mock language server for testing #19391

dhruvmanila · 2025-07-17T04:26:42Z

Summary

This PR implements an initial version of a mock language server that can be used to write e2e tests using the real server running in the background.

The way it works is that you'd use the TestServerBuilder to help construct the TestServer with the setup data. This could be the workspace folders, populating the file and it's content in the memory file system, setting the right client capabilities to make the server respond correctly, etc. This can be expanded as we write more test cases.

There are still a few things to follow-up on:

~~In the Drop implementation, we should assert that there are no pending notification, request and responses from the server that the test code hasn't handled yet~~ Implemented in afd1f82 (#19391)
Reduce the setup boilerplate in any way we can
Improve the final assertion, currently I'm just snapshotting the final output

Test Plan

Written a few test cases.

github-actions · 2025-07-17T04:30:14Z

`mypy_primer` results

No ecosystem changes detected ✅
No memory usage changes detected ✅

crates/ty_server/src/server.rs

crates/ty_server/src/logging.rs

github-actions · 2025-07-17T08:32:53Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

crates/ty_server/src/server.rs

MichaReiser

Nice, this is awesome!

I've a few suggestions on how the error handling and asserting some basic constraints of the LSP protocol can be improved. But I don't mind if you decide to defer those to a follow up PR.

crates/ty_server/src/lib.rs

crates/ty_server/src/logging.rs

crates/ty_server/src/server.rs

crates/ty_server/src/test.rs

MichaReiser · 2025-07-18T07:46:33Z

crates/ty_server/src/test.rs

+
+impl Drop for TestServer {
+    fn drop(&mut self) {
+        self.drain_messages();


Should we move the drain to after the shutdown handling because the server could still be sending requests to the client during the shutdown sequence if it wnated to.

I think we should do this first because if we reached here then the test case didn't handle some messages.

We might then need to do it twice?

Oh, I think I might have misunderstood what it does. It simply drops all messages. Shouldn't it be an error if a test doesn't handle all messages? We could add a method that drops all messages but I would sort of expect that all messages are handled and any unhandled message is an error (because it didn't fully test the flow?)

It doesn't drop all the message, it basically keeps asking the server for the messages via the self.receive call until we stop receiving any messages from the server which records them in self.requests, self.notifications and self.responses and at the end there's a self.assert_no_pending_messages call that asserts that these data structures are empty.

This split is necessary because we should only panic after the server thread is joined to avoid unnecessary panic messages.

MichaReiser · 2025-07-18T08:12:29Z

crates/ty_server/src/test.rs

+        R: Request,
+    {
+        let id = self.next_request_id();
+        let request = lsp_server::Request::new(id.clone(), R::METHOD.to_string(), params);


This is more of an extension but it would be nice if the server tracked the pending_requests (FxHashMap<id, &'static str> where the value is the method).

get_response should panic if the server receives a response for a request not in pending_requests (that means the response was sent twice, e.g. after cancellation)

Drop should probably panic if there are any pending requests? I'm not entirely sure about that as it isn't a LSP spec violation. We might at least want to warn using tracing

crates/ty_server/src/test.rs

crates/ty_server/src/server.rs

MichaReiser · 2025-07-22T14:18:54Z

I'm taking a look at the failing windows test

MichaReiser · 2025-07-22T14:27:38Z

This works for me:

Subject: [PATCH] Add Micro Benchmark

This PR adds a microbenchmark for the Ruff linter. Adding microbenchmark for other tools
should be straightforward.
---
Index: crates/ty_server/src/test.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/crates/ty_server/src/test.rs b/crates/ty_server/src/test.rs
--- a/crates/ty_server/src/test.rs	(revision 07e2747a21e17da81ff3c9433e27721bb8a9d61e)
+++ b/crates/ty_server/src/test.rs	(date 1753194432043)
@@ -13,7 +13,7 @@
 use std::time::Duration;
 use std::{fmt, fs};
 
-use anyhow::{Context, Result};
+use anyhow::{Context, Result, anyhow};
 use crossbeam::channel::RecvTimeoutError;
 use insta::internals::SettingsBindDropGuard;
 use lsp_server::{Connection, Message, RequestId, Response, ResponseError};
@@ -836,15 +836,16 @@
         })?;
 
         let mut settings = insta::Settings::clone_current();
-        settings.add_filter(&tempdir_filter(&project_dir), "<temp_dir>/");
+        settings.add_filter(&tempdir_filter(project_dir.as_str()), "<temp_dir>/");
         // TODO: Is this required?
-        // settings.add_filter(
-        //     &tempdir_filter(
-        //         Url::from_file_path(project_dir.as_std_path())
-        //             .context("Failed to convert project path to URL")?,
-        //     ),
-        //     "file://<temp_dir>/",
-        // );
+        settings.add_filter(
+            &tempdir_filter(
+                Url::from_file_path(project_dir.as_std_path())
+                    .map_err(|()| anyhow!("Failed to convert root directory to url"))?
+                    .path(),
+            ),
+            "/<temp_dir>/",
+        );
         settings.add_filter(r#"\\(\w\w|\s|\.|")"#, "/$1");
         settings.add_filter(
             r#"The system cannot find the file specified."#,
@@ -865,6 +866,6 @@
     }
 }
 
-fn tempdir_filter(path: &SystemPath) -> String {
-    format!(r"{}\\?/?", regex::escape(path.as_str()))
+fn tempdir_filter(path: impl AsRef<str>) -> String {
+    format!(r"{}\\?/?", regex::escape(path.as_ref()))
 }

MichaReiser

This is great. I've a few more inline suggestions

MichaReiser · 2025-07-21T11:12:35Z

crates/ty_server/src/test.rs

+    /// Perform LSP initialization handshake
+    fn initialize(
+        mut self,
+        workspace_folders: Vec<WorkspaceFolder>,


Nit: The initialize function can compute workspace_folders from iterating over workspace_configurations (move line 133 into `initialize)

crates/ty_server/src/test.rs

MichaReiser · 2025-07-22T14:43:13Z

crates/ty_server/src/server.rs

+#[cfg(test)]
+mod tests {
+    use anyhow::Result;
+    use lsp_types::notification::PublishDiagnostics;
+    use ruff_db::system::SystemPath;
+
+    use crate::session::ClientOptions;
+    use crate::test::TestServerBuilder;
+
+    #[test]
+    fn initialization() -> Result<()> {
+        let server = TestServerBuilder::new()?
+            .build()?


I think I'd prefer if the tests were written as integration tests in the tests folder. See the ty/cli/main.rs for how to share code between integration tests. Unless this forces us to make many more types public that otherwise wouldn't have to be.

My main thinking here is that these are integration tests and the snapshots folder is a bit distracting in the middle of rust modules (this could also be solved by specifying another path in the insta settings)

Yeah, I wanted this to be integration tests but it will require making ClientOptions and everything inside it as publicly available so that it can be used to specify workspace options in the test server.

I see. I don't think I would mind this overly much but maybe worth a separate PR so that it's clear what we need to make public

crates/ty_server/src/test.rs

* main: (28 commits) [ty] highlight the argument in `static_assert` error messages (astral-sh#19426) [ty] Infer single-valuedness for enums based on `int`/`str` (astral-sh#19510) [ty] Restructure submodule query around `File` dependency [ty] Make `Module` a Salsa ingredient [ty] Reachability analysis for `isinstance(…)` branches (astral-sh#19503) [ty] Normalize single-member enums to their instance type (astral-sh#19502) [ty] Invert `ty_ide` and `ty_project` dependency (astral-sh#19501) [ty] Implement mock language server for testing (astral-sh#19391) [ty] Detect enums if metaclass is a subtype of EnumType/EnumMeta (astral-sh#19481) [ty] perform type narrowing for places marked `global` too (astral-sh#19381) [ty] Use `ThinVec` for sub segments in `PlaceExpr` (astral-sh#19470) [ty] Splat variadic arguments into parameter list (astral-sh#18996) [`flake8-pyi`] Skip fix if all `Union` members are `None` (`PYI016`) (astral-sh#19416) Skip notebook with errors in ecosystem check (astral-sh#19491) [ty] Consistent use of American english (in rules) (astral-sh#19488) [ty] Support iterating over enums (astral-sh#19486) Fix panic for illegal `Literal[…]` annotations with inner subscript expressions (astral-sh#19489) Move fix suggestion to subdiagnostic (astral-sh#19464) [ty] Implement non-stdlib stub mapping for classes and functions (astral-sh#19471) [ty] Disallow illegal uses of `ClassVar` (astral-sh#19483) ... # Conflicts: # crates/ty_ide/src/goto.rs

* main: [ty] Fix narrowing and reachability of class patterns with arguments (#19512) [ty] Implemented partial support for "find references" language server feature. (#19475) [`flake8-use-pathlib`] Add autofix for `PTH101`, `PTH104`, `PTH105`, `PTH121` (#19404) [`perflint`] Parenthesize generator expressions (`PERF401`) (#19325) [`pep8-naming`] Fix `N802` false positives for `CGIHTTPRequestHandler` and `SimpleHTTPRequestHandler` (#19432) [`pylint`] Handle empty comments after line continuation (`PLR2044`) (#19405) Move concise diagnostic rendering to `ruff_db` (#19398) [ty] highlight the argument in `static_assert` error messages (#19426) [ty] Infer single-valuedness for enums based on `int`/`str` (#19510) [ty] Restructure submodule query around `File` dependency [ty] Make `Module` a Salsa ingredient [ty] Reachability analysis for `isinstance(…)` branches (#19503) [ty] Normalize single-member enums to their instance type (#19502) [ty] Invert `ty_ide` and `ty_project` dependency (#19501) [ty] Implement mock language server for testing (#19391) [ty] Detect enums if metaclass is a subtype of EnumType/EnumMeta (#19481) [ty] perform type narrowing for places marked `global` too (#19381)

## Summary Reference: #19391 (comment)

## Summary Closes: astral-sh/ty#88 This PR implements an initial version of a mock language server that can be used to write e2e tests using the real server running in the background. The way it works is that you'd use the `TestServerBuilder` to help construct the `TestServer` with the setup data. This could be the workspace folders, populating the file and it's content in the memory file system, setting the right client capabilities to make the server respond correctly, etc. This can be expanded as we write more test cases. There are still a few things to follow-up on: - ~In the `Drop` implementation, we should assert that there are no pending notification, request and responses from the server that the test code hasn't handled yet~ Implemented in [`afd1f82` (#19391)](afd1f82) - Reduce the setup boilerplate in any way we can - Improve the final assertion, currently I'm just snapshotting the final output ## Test Plan Written a few test cases.

## Summary Reference: #19391 (comment)

dhruvmanila added server Related to the LSP server testing Related to testing Ruff itself ty Multi-file analysis & type inference labels Jul 17, 2025

MichaReiser reviewed Jul 17, 2025

View reviewed changes

crates/ty_server/src/server.rs Outdated Show resolved Hide resolved

dhruvmanila force-pushed the dhruv/mock-server branch from 290d685 to a28f5c2 Compare July 17, 2025 07:12

dhruvmanila commented Jul 17, 2025

View reviewed changes

crates/ty_server/src/logging.rs Outdated Show resolved Hide resolved

dhruvmanila force-pushed the dhruv/mock-server branch from a28f5c2 to 4352643 Compare July 17, 2025 08:19

dhruvmanila force-pushed the dhruv/mock-server branch 2 times, most recently from c229255 to 332c1cc Compare July 17, 2025 14:40

dhruvmanila commented Jul 17, 2025

View reviewed changes

crates/ty_server/src/server.rs Outdated Show resolved Hide resolved

dhruvmanila marked this pull request as ready for review July 17, 2025 14:50

dhruvmanila requested review from AlexWaygood, carljm, dcreager and sharkdp as code owners July 17, 2025 14:51

dhruvmanila force-pushed the dhruv/mock-server branch 2 times, most recently from afd1f82 to 964b413 Compare July 18, 2025 05:54

MichaReiser approved these changes Jul 18, 2025

View reviewed changes

dhruvmanila added 3 commits July 21, 2025 14:15

[ty] Implement mock language server for testing

3e82dc1

Test publish and pull diagnostics

626c585

Assert no pending messages before we drop the test server

c9be67b

dhruvmanila force-pushed the dhruv/mock-server branch 2 times, most recently from 79a42e2 to 98f4b46 Compare July 21, 2025 09:06

Address review feedback

a7aada5

dhruvmanila force-pushed the dhruv/mock-server branch from 98f4b46 to a7aada5 Compare July 21, 2025 09:08

Avoid assertion if test server is panicking

6d9466e

AlexWaygood removed their request for review July 21, 2025 13:42

Use tempdir instead of memory file system

a1068f6

dhruvmanila added 3 commits July 22, 2025 13:43

Fix tempdir handling

c5ad83d

Move "initializing logging once" to be done only during tests

80cf850

Use temp dir implementation from cli tests

ce493be

MichaReiser approved these changes Jul 22, 2025

View reviewed changes

MichaReiser reviewed Jul 23, 2025

View reviewed changes

crates/ty_server/src/test.rs Show resolved Hide resolved

Address review comments

20b61af

dhruvmanila force-pushed the dhruv/mock-server branch from 07e2747 to 20b61af Compare July 23, 2025 06:23

dhruvmanila added 3 commits July 23, 2025 12:13

Box the Response to reduce error size

22d5ad2

Avoid leading slash in temp dir replacement

bce74ca

Collect all messages during receive

1940ea2

dhruvmanila merged commit 53d795d into main Jul 23, 2025
36 checks passed

dhruvmanila deleted the dhruv/mock-server branch July 23, 2025 06:56

dhruvmanila mentioned this pull request Jul 24, 2025

[ty] Move server tests as integration tests #19522

Merged

dhruvmanila added a commit that referenced this pull request Jul 24, 2025

[ty] Move server tests as integration tests (#19522)

f9091ea

## Summary Reference: #19391 (comment)

AlexWaygood pushed a commit that referenced this pull request Jul 25, 2025

[ty] Move server tests as integration tests (#19522)

80a2469

## Summary Reference: #19391 (comment)

[ty] Implement mock language server for testing #19391

[ty] Implement mock language server for testing #19391

Uh oh!

Conversation

dhruvmanila commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser commented Jul 22, 2025

Uh oh!

MichaReiser commented Jul 22, 2025

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

dhruvmanila commented Jul 17, 2025 •

edited

Loading

github-actions bot commented Jul 17, 2025 •

edited

Loading

`mypy_primer` results

github-actions bot commented Jul 17, 2025 •

edited

Loading

`ruff-ecosystem` results

dhruvmanila Jul 18, 2025 •

edited

Loading