[analysis_server] Fix race condition in will_rename_files test

During this test, sometimes the refactor would complete before the overlay modification was processed, which means we don't get the expected failure response.

This isn't a server bug because the results are still consistent (and clients should discard any responses if a file was modified since the request was sent). This test was to ensure we handled the obvious cases of this in case the client did not handle it.

The fix is to artificially slow down the refactor to ensure the overlay change has time to execute before we check for consistency at the end.

Change-Id: I5697a73d7ca6dd7115dc6c0e87b8a93ba3cd533b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/331940
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

pkg/analysis_server/lib/src/lsp/handlers/handler_will_rename_files.dart

@@ -8,12 +8,19 @@ import 'package:analysis_server/src/lsp/handlers/handlers.dart';
 import 'package:analysis_server/src/lsp/mapping.dart';
 import 'package:analysis_server/src/lsp/registration/feature_registration.dart';
 import 'package:analysis_server/src/services/refactoring/legacy/move_file.dart';
+import 'package:meta/meta.dart';
 
 typedef StaticOptions = FileOperationRegistrationOptions?;
 
 class WillRenameFilesHandler
     extends SharedMessageHandler<RenameFilesParams, WorkspaceEdit?> {
+  /// A [Future] used by tests to allow inserting a delay during computation
+  /// to allow forcing inconsistent analysis.
+  @visibleForTesting
+  static Future<void>? delayDuringComputeForTests;
+
   WillRenameFilesHandler(super.server);
+
   @override
   Method get handlesMessage => Method.workspace_willRenameFiles;
 
@@ -67,6 +74,10 @@ class WillRenameFilesHandler
     }
 
     final change = await refactoring.createChange();
+    if (delayDuringComputeForTests != null) {
+      await delayDuringComputeForTests;
+    }
+
     if (token.isCancellationRequested) {
       return cancelled();
     }

pkg/analysis_server/test/lsp/completion_dart_test.dart

@@ -3941,25 +3941,29 @@ void f() {
     await openFile(mainFileUri, code.code);
     await initialAnalysis;
 
-    // User a Completer to control when the completion handler starts computing.
+    // Use a Completer to control when the completion handler starts computing.
     final completer = Completer<void>();
     CompletionHandler.delayAfterResolveForTests = completer.future;
+    try {
+      // Start the completion request but don't await it yet.
+      final completionRequest =
+          getCompletionList(mainFileUri, code.position.position);
+      // Modify the document to ensure the snippet requests will fail to build
+      // edits and then allow the handler to continue.
+      await replaceFile(222, mainFileUri, '');
+      completer.complete();
 
-    // Start the completion request but don't await it yet.
-    final completionRequest =
-        getCompletionList(mainFileUri, code.position.position);
-    // Modify the document to ensure the snippet requests will fail to build
-    // edits and then allow the handler to continue.
-    await replaceFile(222, mainFileUri, '');
-    completer.complete();
+      // Wait for the results.
+      final result = await completionRequest;
 
-    // Wait for the results.
-    final result = await completionRequest;
-
-    // Ensure we flagged that we did not return everything but we still got
-    // results.
-    expect(result.isIncomplete, isTrue);
-    expect(result.items, isNotEmpty);
+      // Ensure we flagged that we did not return everything but we still got
+      // results.
+      expect(result.isIncomplete, isTrue);
+      expect(result.items, isNotEmpty);
+    } finally {
+      // Ensure we never leave an incomplete future if anything above throws.
+      CompletionHandler.delayAfterResolveForTests = null;
+    }
   }
 
   Future<void>

pkg/analysis_server/test/lsp_over_legacy/will_rename_files_test.dart

@@ -2,7 +2,10 @@
 // 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.
 
+import 'dart:async';
+
 import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/lsp/handlers/handler_will_rename_files.dart';
 import 'package:analysis_server/src/protocol_server.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -43,17 +46,28 @@ class WillRenameFilesTest extends LspOverLegacyTest {
   Future<void> test_inconsistentAnalysis() async {
     final testFileNewPath = join(testPackageLibPath, 'test_new.dart');
 
-    await addOverlay(testFilePath, 'original');
-    // Don't await, need to send modification.
-    final editFuture = onWillRename([
-      FileRename(
-        oldUri: toUri(testFilePath).toString(),
-        newUri: toUri(testFileNewPath).toString(),
-      ),
-    ]);
-    await updateOverlay(testFilePath, SourceEdit(0, 0, 'inserted'));
+    // Use a Completer to control when the refactor finishes computing so that
+    // we can ensure the overlay modification had time to be applied and trigger
+    // creation of new sessions.
+    final completer = Completer<void>();
+    WillRenameFilesHandler.delayDuringComputeForTests = completer.future;
+    try {
+      await addOverlay(testFilePath, 'original');
+      // Don't await, need to send modification.
+      final editFuture = onWillRename([
+        FileRename(
+          oldUri: toUri(testFilePath).toString(),
+          newUri: toUri(testFileNewPath).toString(),
+        ),
+      ]);
+      await updateOverlay(testFilePath, SourceEdit(0, 0, 'inserted'));
+      await pumpEventQueue(times: 50000).then(completer.complete);
 
-    expect(editFuture, throwsA(isResponseError(ErrorCodes.ContentModified)));
+      expect(editFuture, throwsA(isResponseError(ErrorCodes.ContentModified)));
+    } finally {
+      // Ensure we never leave an incomplete future if anything above throws.
+      WillRenameFilesHandler.delayDuringComputeForTests = null;
+    }
   }
 
   /// Test moving multiple items at once. Both files reference each other