[analysis_server] Add a shared test interface to simplify sharing tests between LSP, LSP-over-Legacy

This is refactor extracted from an upcoming change (to make property editor tests run against both servers) to make that change smaller and easier to review.

There are some existing shared tests that run for both LSP and Legacy servers, but they currently do not touch much server API (one is for DTD and one tests reverse-requests). Migrating other tests (such as EditableArguments) requires some additional API be the same between the different test/server base classes.

This change adds an `abstract interface class SharedTestInterface` to serve as a common interface for methods that shared tests need to use that have different implementations between LSP and Legacy. For example, updating the overlays in an LSP-over-Legacy test needs to use the Legacy APIs for updating the overlay and not the LSP ones (so we can't just use the LSP methods like we would for calling something like getHover for LSP-over-Legacy).

It also:

- adds some new futures to the LSP test base to match the behaviour of the legacy one (wait for in-progress analysis)
- replaces the shared mixins with real base classes that implement the shared interface (for ex. `abstract class SharedLspOverLegacyTest extends LspOverLegacyTest implements SharedTestInterface`) to make it easier to create shared tests
- renames `sendLspRequest` to `sendLspRequestToClient` to make it clearer what direction this method is for

Change-Id: I070c2c005b11b9afd8a87aa22b04972a9dde2320
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404680
Reviewed-by: Phil Quitslund <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>

Author: DanTup

pkg/analysis_server/test/lsp/server_abstract.dart

@@ -33,6 +33,7 @@ import 'package:unified_analytics/unified_analytics.dart';
 
 import '../mocks.dart';
 import '../mocks_lsp.dart';
+import '../shared/shared_test_interface.dart';
 import '../support/configuration_files.dart';
 import '../test_macros.dart';
 import 'change_verifier.dart';
@@ -248,6 +249,10 @@ abstract class AbstractLspAnalysisServerTest
     _previousContextBuilds = server.contextBuilds;
   }
 
+  Future<ResponseMessage> sendLspRequestToClient(Method method, Object params) {
+    return server.sendRequest(method, params);
+  }
+
   @override
   Future<void> sendNotificationToServer(
     NotificationMessage notification,
@@ -882,9 +887,19 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin, LspEditHelpersMixin
   /// server.
   bool failTestOnErrorDiagnostic = true;
 
+  /// A completer for [initialAnalysis].
+  final Completer<void> _initialAnalysisCompleter = Completer<void>();
+
+  /// A completer for [currentAnalysis].
+  Completer<void> _currentAnalysisCompleter = Completer<void>()..complete();
+
   /// [analysisOptionsPath] as a 'file:///' [Uri].
   Uri get analysisOptionsUri => pathContext.toUri(analysisOptionsPath);
 
+  /// A [Future] that completes when the current analysis completes (or is
+  /// already completed if no analysis is in progress).
+  Future<void> get currentAnalysis => _currentAnalysisCompleter.future;
+
   /// A stream of [NotificationMessage]s from the server that may be errors.
   Stream<NotificationMessage> get errorNotificationsFromServer {
     return notificationsFromServer.where(_isErrorNotification);
@@ -895,8 +910,7 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin, LspEditHelpersMixin
       serverCapabilities.experimental as Map<String, Object?>? ?? {};
 
   /// A [Future] that completes with the first analysis after initialization.
-  Future<void> get initialAnalysis =>
-      initialized ? Future.value() : waitForAnalysisComplete();
+  Future<void> get initialAnalysis => _initialAnalysisCompleter.future;
 
   bool get initialized => _clientCapabilities != null;
 
@@ -1152,6 +1166,16 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin, LspEditHelpersMixin
     notificationsFromServer.listen((notification) async {
       if (notification.method == Method.progress) {
         await _handleProgress(notification);
+      } else if (notification.method == CustomMethods.analyzerStatus) {
+        var params = AnalyzerStatusParams.fromJson(
+          notification.params as Map<String, Object?>,
+        );
+
+        if (params.isAnalyzing) {
+          _handleAnalysisBegin();
+        } else {
+          _handleAnalysisEnd();
+        }
       }
     });
 
@@ -1592,6 +1616,19 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin, LspEditHelpersMixin
     return outlineParams.outline;
   }
 
+  void _handleAnalysisBegin() async {
+    assert(_currentAnalysisCompleter.isCompleted);
+    _currentAnalysisCompleter = Completer<void>();
+  }
+
+  void _handleAnalysisEnd() async {
+    if (!_initialAnalysisCompleter.isCompleted) {
+      _initialAnalysisCompleter.complete();
+    }
+    assert(!_currentAnalysisCompleter.isCompleted);
+    _currentAnalysisCompleter.complete();
+  }
+
   Future<void> _handleProgress(NotificationMessage request) async {
     var params = ProgressParams.fromJson(
       request.params as Map<String, Object?>,
@@ -1607,6 +1644,15 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin, LspEditHelpersMixin
     if (WorkDoneProgressEnd.canParse(params.value, nullLspJsonReporter)) {
       validProgressTokens.remove(params.token);
     }
+
+    if (params.token == analyzingProgressToken) {
+      if (WorkDoneProgressBegin.canParse(params.value, nullLspJsonReporter)) {
+        _handleAnalysisBegin();
+      }
+      if (WorkDoneProgressEnd.canParse(params.value, nullLspJsonReporter)) {
+        _handleAnalysisEnd();
+      }
+    }
   }
 
   Future<void> _handleWorkDoneProgressCreate(RequestMessage request) async {
@@ -1642,16 +1688,24 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin, LspEditHelpersMixin
   }
 }
 
-/// A mixin that provides a common interface for [AbstractLspAnalysisServerTest] classes
-/// to work with shared tests.
-mixin SharedLspAnalysisServerTestMixin on AbstractLspAnalysisServerTest {
+/// An [AbstractLspAnalysisServerTest] that provides an implementation of
+/// [SharedTestInterface] to allow tests to be shared between server/test kinds.
+abstract class SharedAbstractLspAnalysisServerTest
+    extends AbstractLspAnalysisServerTest
+    implements SharedTestInterface {
+  @override
   String get testFilePath => join(projectFolderPath, 'lib', 'test.dart');
 
+  @override
+  Uri get testFileUri => toUri(testFilePath);
+
+  @override
   void createFile(String path, String content) {
     newFile(path, content);
   }
 
-  Future<ResponseMessage> sendLspRequest(Method method, Object params) {
-    return server.sendRequest(method, params);
+  @override
+  Future<void> initializeServer() async {
+    await initialize();
   }
 }

pkg/analysis_server/test/lsp/workspace_apply_edit_test.dart

@@ -14,16 +14,15 @@ void main() {
 }
 
 @reflectiveTest
-class WorkspaceApplyEditTest extends AbstractLspAnalysisServerTest
+class WorkspaceApplyEditTest extends SharedAbstractLspAnalysisServerTest
     with
-        // Tests are defined in SharedLspAnalysisServerTestMixin because they
+        // Tests are defined in SharedWorkspaceApplyEditTests because they
         // are shared and run for both LSP and Legacy servers.
-        SharedLspAnalysisServerTestMixin,
         SharedWorkspaceApplyEditTests {
   @override
   Future<void> initializeServer() async {
     await initialize();
-    await initialAnalysis;
+    await currentAnalysis;
   }
 
   @override

pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart

@@ -24,6 +24,7 @@ import '../lsp/change_verifier.dart';
 import '../lsp/request_helpers_mixin.dart';
 import '../lsp/server_abstract.dart';
 import '../services/completion/dart/text_expectations.dart';
+import '../shared/shared_test_interface.dart';
 
 class EventsCollector {
   final ContextResolutionTest test;
@@ -258,12 +259,10 @@ abstract class LspOverLegacyTest extends PubPackageAnalysisServerTest
     var error = lspResponse.error;
     if (error != null) {
       throw error;
-    } else if (T == Null) {
+    } else {
       return lspResponse.result == null
           ? null as T
-          : throw 'Expected Null response but got ${lspResponse.result}';
-    } else {
-      return fromJson(lspResponse.result as R);
+          : fromJson(lspResponse.result as R);
     }
   }
 
@@ -311,6 +310,17 @@ abstract class LspOverLegacyTest extends PubPackageAnalysisServerTest
     }
   }
 
+  Future<void> removeOverlay(String filePath) {
+    return handleSuccessfulRequest(
+      AnalysisUpdateContentParams({
+        convertPath(filePath): RemoveContentOverlay(),
+      }).toRequest(
+        '${_nextRequestId++}',
+        clientUriConverter: server.uriConverter,
+      ),
+    );
+  }
+
   /// Send the configured LSP client capabilities to the server in a
   /// `server.setClientCapabilities` request.
   Future<void> sendClientCapabilities() async {
@@ -369,15 +379,42 @@ abstract class LspOverLegacyTest extends PubPackageAnalysisServerTest
   }
 }
 
-/// A mixin that provides a common interface for [LspOverLegacyTest] classes
-/// to work with shared tests.
-mixin SharedLspOverLegacyMixin on LspOverLegacyTest {
+/// A [LspOverLegacyTest] that provides an implementation of
+/// [SharedTestInterface] to allow tests to be shared between server/test kinds.
+abstract class SharedLspOverLegacyTest extends LspOverLegacyTest
+    implements SharedTestInterface {
+  // TODO(dantup): Support this for LSP-over-Legacy shared tests.
+  var failTestOnErrorDiagnostic = false;
+
+  @override
+  Future<void> get currentAnalysis => waitForTasksFinished();
+
+  @override
+  Future<void> closeFile(Uri uri) async {
+    await removeOverlay(fromUri(uri));
+  }
+
+  @override
   void createFile(String path, String content) {
     newFile(path, content);
   }
 
+  @override
+  Future<void> openFile(Uri uri, String content, {int version = 1}) async {
+    await addOverlay(fromUri(uri), content);
+  }
+
+  @override
+  Future<void> replaceFile(int newVersion, Uri uri, String content) async {
+    // For legacy, we can use addOverlay to replace the whole file.
+    await addOverlay(fromUri(uri), content);
+  }
+
   /// Wraps an LSP request up and sends it from the server to the client.
-  Future<ResponseMessage> sendLspRequest(Method method, Object params) async {
+  Future<ResponseMessage> sendLspRequestToClient(
+    Method method,
+    Object params,
+  ) async {
     var id = server.nextServerRequestId++;
     // Round-trip through JSON to ensure everything becomes basic types and we
     // don't have instances of classes like `Either2<>` in the JSON.

pkg/analysis_server/test/lsp_over_legacy/workspace_apply_edit_test.dart

@@ -14,11 +14,10 @@ void main() {
 }
 
 @reflectiveTest
-class WorkspaceApplyEditTest extends LspOverLegacyTest
+class WorkspaceApplyEditTest extends SharedLspOverLegacyTest
     with
-        // Tests are defined in SharedLspAnalysisServerTestMixin because they
+        // Tests are defined in SharedWorkspaceApplyEditTests because they
         // are shared and run for both LSP and Legacy servers.
-        SharedLspOverLegacyMixin,
         SharedWorkspaceApplyEditTests {
   @override
   Future<void> initializeServer() async {

pkg/analysis_server/test/shared/shared_test_interface.dart

@@ -0,0 +1,52 @@
+// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+/// A common interface that can be implemented by a set of base classes to
+/// allow tests to be written to run in different configurations
+/// (LSP and LSP-over-Legacy, and in-process and out-of-process).
+///
+/// Implementations should use the appropriate APIs for the given server, for
+/// example a test running against LSP will send a `textDocument/didOpen`
+/// notification from [openFile] whereas when using the legacy server (even for
+/// LSP-over-Legacy tests) will send an `analysis.updateContent` request.
+abstract interface class SharedTestInterface {
+  /// A future that completes when the current analysis completes.
+  ///
+  /// If there is no analysis in progress, completes immediately.
+  Future<void> get currentAnalysis;
+
+  /// Sets whether the test should fail if error diagnostics are generated.
+  ///
+  /// This is used to avoid accidentally including invalid code in tests but can
+  /// be overridden for tests that are deliberately testing invalid code.
+  set failTestOnErrorDiagnostic(bool value);
+
+  /// Gets the full normalized file path of a file named "test.dart" in the test
+  /// project.
+  String get testFilePath;
+
+  /// Gets a file:/// URI for [testFilePath];
+  Uri get testFileUri => Uri.file(testFilePath);
+
+  /// Tells the server that file with [uri] has been closed and any overlay
+  /// should be removed.
+  Future<void> closeFile(Uri uri);
+
+  /// Creates a file at [filePath] with the given [content].
+  void createFile(String filePath, String content);
+
+  /// Performs standard initialization of the server, including starting
+  /// the server (an external process for integration tests) and sending any
+  /// initialization/analysis roots, and waiting for initial analysis to
+  /// complete.
+  Future<void> initializeServer();
+
+  /// Tells the server that the file with [uri] has been opened and has the
+  /// given [content].
+  Future<void> openFile(Uri uri, String content, {int version = 1});
+
+  /// Tells the server that the file with [uri] has had it's content replaced
+  /// with [content].
+  Future<void> replaceFile(int newVersion, Uri uri, String content);
+}

pkg/analysis_server/test/shared/shared_workspace_apply_edit_tests.dart

@@ -31,7 +31,7 @@ mixin SharedWorkspaceApplyEditTests
 
   /// Overridden by test subclasses to send LSP requests from the server to
   /// the client.
-  Future<ResponseMessage> sendLspRequest(Method method, Object params);
+  Future<ResponseMessage> sendLspRequestToClient(Method method, Object params);
 
   test_applyEdit_existingFile() async {
     var code = TestCode.parse('''
@@ -147,7 +147,7 @@ inserted<<<<<<<<<<
     ApplyWorkspaceEditResult? receivedApplyEditResult;
 
     var verifier = await executeForEdits(() async {
-      var result = await sendLspRequest(
+      var result = await sendLspRequestToClient(
         Method.workspace_applyEdit,
         ApplyWorkspaceEditParams(edit: workspaceEdit),
       );