[analysis_server] Tidy up LSP tests failTestOnAnyErrorNotification to be implied when using expectErrorNotification

This is a small tidy up that changes `failTestOnAnyErrorNotification` to be automatically set when using `expectErrorNotification` so there's slightly less noise in tests.

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

pkg/analysis_server/test/lsp/code_actions_abstract.dart

@@ -23,15 +23,12 @@ abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
     CodeActionTriggerKind? triggerKind,
     String? filePath,
     bool openTargetFile = false,
-    bool failTestOnAnyErrorNotification = true,
   }) async {
     filePath ??= mainFilePath;
     var code = TestCode.parse(content);
     newFile(filePath, code.code);
 
-    await initialize(
-      failTestOnAnyErrorNotification: failTestOnAnyErrorNotification,
-    );
+    await initialize();
 
     var fileUri = uriConverter.toClientUri(filePath);
     if (openTargetFile) {

pkg/analysis_server/test/lsp/code_actions_refactor_test.dart

@@ -502,7 +502,8 @@ void doFoo(void Function() a) => a();
     );
 
     expect(response.valid, isFalse);
-    expect(response.message, contains('Cannot extract the closure as a method'));
+    expect(
+        response.message, contains('Cannot extract the closure as a method'));
   }
 
   /// Test if the client does not call refactor.validate it still gets a
@@ -521,10 +522,7 @@ void doFoo(void Function() a) => a();
     final codeAction = await expectAction(
       content,
       command: Commands.performRefactor,
-
       title: extractMethodTitle,
-      // We expect an error notification so don't fail on it.
-      failTestOnAnyErrorNotification: false,
     );
     final command = codeAction.command!;
 

pkg/analysis_server/test/lsp/file_modification_test.dart

@@ -18,10 +18,7 @@ void main() {
 class FileModificationTest extends AbstractLspAnalysisServerTest {
   Future<void> test_change_badPosition() async {
     final contents = '';
-    await initialize(
-      // Error is expected and checked below.
-      failTestOnAnyErrorNotification: false,
-    );
+    await initialize();
     await openFile(mainFileUri, contents);
 
     // Since this is a notification and not a request, the server cannot
@@ -86,10 +83,7 @@ class FileModificationTest extends AbstractLspAnalysisServerTest {
           end: Position(line: 1, character: 1)),
       text: 'test',
     ));
-    await initialize(
-      // Error is expected and checked below.
-      failTestOnAnyErrorNotification: false,
-    );
+    await initialize();
     final notificationParams = await expectErrorNotification(
       () => changeFile(222, mainFileUri, [simpleEdit]),
     );
@@ -134,10 +128,7 @@ class FileModificationTest extends AbstractLspAnalysisServerTest {
   }
 
   Future<void> test_open_invalidPath() async {
-    await initialize(
-      // Error is expected and checked below.
-      failTestOnAnyErrorNotification: false,
-    );
+    await initialize();
 
     final notificationParams = await expectErrorNotification(
       () => openFile(Uri.http('localhost', 'not-a-file'), ''),

pkg/analysis_server/test/lsp/initialization_test.dart

@@ -934,14 +934,13 @@ class InitializationTest extends AbstractLspAnalysisServerTest {
   }
 
   Future<void> test_nonFileScheme_rootUri() async {
+    // We expect an error notification about the invalid file we try to open.
+    failTestOnAnyErrorNotification = false;
+
     final rootUri = Uri.parse('vsls://');
     final fileUri = rootUri.replace(path: '/file1.dart');
 
-    await initialize(
-      rootUri: rootUri,
-      // We expect an error notification about the invalid file we try to open.
-      failTestOnAnyErrorNotification: false,
-    );
+    await initialize(rootUri: rootUri);
     expect(server.contextManager.includedPaths, equals([]));
 
     // Also open a non-file file to ensure it doesn't cause the root to be added.
@@ -954,6 +953,8 @@ class InitializationTest extends AbstractLspAnalysisServerTest {
   /// Related tests for didChangeWorkspaceFolders are in
   /// [ChangeWorkspaceFoldersTest].
   Future<void> test_nonFileScheme_workspaceFolders() async {
+    // We expect an error notification about the invalid file we try to open.
+    failTestOnAnyErrorNotification = false;
     newPubspecYamlFile(projectFolderPath, '');
 
     final rootUri = Uri.parse('vsls://');
@@ -964,8 +965,6 @@ class InitializationTest extends AbstractLspAnalysisServerTest {
         rootUri,
         pathContext.toUri(projectFolderPath),
       ],
-      // We expect an error notification about the invalid file we try to open.
-      failTestOnAnyErrorNotification: false,
     );
     expect(server.contextManager.includedPaths, equals([projectFolderPath]));
 

pkg/analysis_server/test/lsp/server_abstract.dart

@@ -818,6 +818,12 @@ mixin LspAnalysisServerTestMixin
   /// list.
   final diagnostics = <Uri, List<Diagnostic>>{};
 
+  /// Whether to fail tests if any error notifications are received from the
+  /// server.
+  ///
+  /// This does not need to be set when using [expectErrorNotification].
+  bool failTestOnAnyErrorNotification = true;
+
   /// A stream of [NotificationMessage]s from the server that may be errors.
   Stream<NotificationMessage> get errorNotificationsFromServer {
     return notificationsFromServer.where(_isErrorNotification);
@@ -955,10 +961,14 @@ mixin LspAnalysisServerTestMixin
     Duration timeout = const Duration(seconds: 5),
   }) async {
     final firstError = errorNotificationsFromServer.first;
-    await f();
 
+    failTestOnAnyErrorNotification = false;
+
+    await f();
     final notificationFromServer = await firstError.timeout(timeout);
 
+    failTestOnAnyErrorNotification = true;
+
     expect(notificationFromServer, isNotNull);
     return ShowMessageParams.fromJson(
         notificationFromServer.params as Map<String, Object?>);
@@ -1069,17 +1079,18 @@ mixin LspAnalysisServerTestMixin
     Map<String, Object?>? initializationOptions,
     bool throwOnFailure = true,
     bool allowEmptyRootUri = false,
-    bool failTestOnAnyErrorNotification = true,
     bool includeClientRequestTime = false,
     void Function()? immediatelyAfterInitialized,
   }) async {
     this.includeClientRequestTime = includeClientRequestTime;
 
-    if (failTestOnAnyErrorNotification) {
-      errorNotificationsFromServer.listen((NotificationMessage error) {
+    errorNotificationsFromServer.listen((NotificationMessage error) {
+      // Always subscribe to this and check the flag here so it can be toggled
+      // during tests (for example automatically by expectErrorNotification).
+      if (failTestOnAnyErrorNotification) {
         fail('${error.toJson()}');
-      });
-    }
+      }
+    });
 
     final clientCapabilities = ClientCapabilities(
       workspace: workspaceCapabilities,

pkg/analysis_server/test/lsp/server_test.dart

@@ -151,10 +151,7 @@ class ServerTest extends AbstractLspAnalysisServerTest {
   }
 
   Future<void> test_inconsistentStateError() async {
-    await initialize(
-      // Error is expected and checked below.
-      failTestOnAnyErrorNotification: false,
-    );
+    await initialize();
     await openFile(mainFileUri, '');
     // Attempt to make an illegal modification to the file. This indicates the
     // client and server are out of sync and we expect the server to shut down.
@@ -260,10 +257,7 @@ class ServerTest extends AbstractLspAnalysisServerTest {
   }
 
   Future<void> test_unknownNotifications_logError() async {
-    await initialize(
-      // Error is expected and checked below.
-      failTestOnAnyErrorNotification: false,
-    );
+    await initialize();
 
     final notification =
         makeNotification(Method.fromJson(r'some/randomNotification'), null);