commit 0d90ccc2ec221acab2620662a19e0da6765d4e05
author Danny Tuppeny <danny@tuppeny.com> Fri Jan 24 09:26:38 2025 -0800
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Jan 24 09:26:38 2025 -0800
tree f30086b4b21aca0b4fe0980a5a812d5dd07f6514
parent 4fce27b1f7ee6e8436e064dba8314d77995d5fae

[analysis_server] Use unique request IDs in tests to avoid flaky tests

Some of the LSP-over-Legacy tests were intermittently failing because the test did not correctly wait for the `setClientCapabilities` request to complete.

The reason for this was that it used the ID `0` (from `_newRequestId`) but so did `setAnalysisRoots`. If the timing was right, the second request would appear to complete when the first did (because the response had id=0) and the test would start before the capabilities had actually been set (and since the test reads them directly off the server, they would be incorrect).

This moves the `_nextRequestId` field down into a base class and updates the helpers for requests like `setAnalysisRoots` to use it too.

Change-Id: Ice44191aad39c7ea81c40e501827557fe563e3d5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405760
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>

pkg/analysis_server/test/analysis_server_base.dart

diff --git a/pkg/analysis_server/test/analysis_server_base.dart b/pkg/analysis_server/test/analysis_server_base.dart
index e035286..16d2867 100644
--- a/pkg/analysis_server/test/analysis_server_base.dart
+++ b/pkg/analysis_server/test/analysis_server_base.dart
@@ -88,6 +88,9 @@
   /// implementation are still fully verified.
   static final MemoryByteStore _sharedByteStore = MemoryByteStore();
 
+  /// The next ID to use in a request to the server.
+  var nextRequestId = 0;
+
   MemoryByteStore _byteStore = _sharedByteStore;
 
   final TestPluginManager pluginManager = TestPluginManager();
@@ -148,7 +151,10 @@
     handleSuccessfulRequest(
       AnalysisSetPriorityFilesParams(
         files.map((e) => e.path).toList(),
-      ).toRequest('0', clientUriConverter: server.uriConverter),
+      ).toRequest(
+        '${nextRequestId++}',
+        clientUriConverter: server.uriConverter,
+      ),
     );
   }
 
@@ -156,7 +162,10 @@
     await handleSuccessfulRequest(
       AnalysisSetPriorityFilesParams(
         files.map((e) => e.path).toList(),
-      ).toRequest('0', clientUriConverter: server.uriConverter),
+      ).toRequest(
+        '${nextRequestId++}',
+        clientUriConverter: server.uriConverter,
+      ),
     );
   }
 
@@ -171,7 +180,10 @@
         includedConverted,
         excludedConverted,
         packageRoots: {},
-      ).toRequest('0', clientUriConverter: server.uriConverter),
+      ).toRequest(
+        '${nextRequestId++}',
+        clientUriConverter: server.uriConverter,
+      ),
     );
   }
 
@@ -214,9 +226,10 @@
 
   Future<void> _setGeneralAnalysisSubscriptions() async {
     await handleSuccessfulRequest(
-      AnalysisSetGeneralSubscriptionsParams(
-        _analysisGeneralServices,
-      ).toRequest('0', clientUriConverter: server.uriConverter),
+      AnalysisSetGeneralSubscriptionsParams(_analysisGeneralServices).toRequest(
+        '${nextRequestId++}',
+        clientUriConverter: server.uriConverter,
+      ),
     );
   }
 }
@@ -264,9 +277,10 @@
   ) async {
     (_analysisFileSubscriptions[service] ??= []).add(file.path);
     await handleSuccessfulRequest(
-      AnalysisSetSubscriptionsParams(
-        _analysisFileSubscriptions,
-      ).toRequest('0', clientUriConverter: server.uriConverter),
+      AnalysisSetSubscriptionsParams(_analysisFileSubscriptions).toRequest(
+        '${nextRequestId++}',
+        clientUriConverter: server.uriConverter,
+      ),
     );
   }
 

pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart

diff --git a/pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart b/pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart
index 7c770e3..c25f95a 100644
--- a/pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart
+++ b/pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart
@@ -150,9 +150,6 @@
         LspEditHelpersMixin,
         LspVerifyEditHelpersMixin,
         ClientCapabilitiesHelperMixin {
-  /// The next ID to use a request to the server.
-  var _nextRequestId = 0;
-
   /// The last ID that was used for a legacy request.
   late String lastSentLegacyRequestId;
 
@@ -207,7 +204,7 @@
       AnalysisUpdateContentParams({
         convertPath(filePath): AddContentOverlay(content),
       }).toRequest(
-        '${_nextRequestId++}',
+        '${nextRequestId++}',
         clientUriConverter: server.uriConverter,
       ),
     );
@@ -241,7 +238,7 @@
   /// Creates a legacy request with an auto-assigned ID.
   Request createLegacyRequest(RequestParams params) {
     return params.toRequest(
-      '${_nextRequestId++}',
+      '${nextRequestId++}',
       clientUriConverter: server.uriConverter,
     );
   }
@@ -323,7 +320,7 @@
       AnalysisUpdateContentParams({
         convertPath(filePath): RemoveContentOverlay(),
       }).toRequest(
-        '${_nextRequestId++}',
+        '${nextRequestId++}',
         clientUriConverter: server.uriConverter,
       ),
     );
@@ -341,7 +338,7 @@
     var request = ServerSetClientCapabilitiesParams(
       [],
       lspCapabilities: clientCapabilities,
-    ).toRequest('${_nextRequestId++}', clientUriConverter: server.uriConverter);
+    ).toRequest('${nextRequestId++}', clientUriConverter: server.uriConverter);
 
     await handleSuccessfulRequest(request);
   }
@@ -377,7 +374,7 @@
       AnalysisUpdateContentParams({
         convertPath(filePath): ChangeContentOverlay([edit]),
       }).toRequest(
-        '${_nextRequestId++}',
+        '${nextRequestId++}',
         clientUriConverter: server.uriConverter,
       ),
     );