[flutter_tools] remove globals from proxy validator (#59175)

Remove global Platfrom from proxy validator. move tests to new file, and update asserts to cover message contents.

packages/flutter_tools/lib/src/doctor.dart

@@ -76,6 +76,7 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
       ...IntelliJValidator.installedValidators,
       ...VsCodeValidator.installedValidators,
     ];
+    final ProxyValidator proxyValidator = ProxyValidator(platform: globals.platform);
     _validators = <DoctorValidator>[
       FlutterValidator(),
       if (androidWorkflow.appliesToHostPlatform)
@@ -105,8 +106,8 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
         ...ideValidators
       else
         NoIdeValidator(),
-      if (ProxyValidator.shouldShow)
-        ProxyValidator(),
+      if (proxyValidator.shouldShow)
+        proxyValidator,
       if (deviceManager.canListAnything)
         DeviceValidator(),
     ];

packages/flutter_tools/lib/src/proxy_validator.dart

@@ -4,48 +4,59 @@
 
 import 'dart:async';
 
+import 'package:meta/meta.dart';
+
+import 'base/platform.dart';
 import 'doctor.dart';
-import 'globals.dart' as globals;
 
+/// A validator that displays configured HTTP_PROXY environment variables.
+///
+/// if the `HTTP_PROXY` environment variable is non-empty, the contents are
+/// validated along with `NO_PROXY`.
 class ProxyValidator extends DoctorValidator {
-  ProxyValidator() : super('Proxy Configuration');
-
-  static bool get shouldShow => _getEnv('HTTP_PROXY').isNotEmpty;
+  ProxyValidator({
+    @required Platform platform,
+  })  : shouldShow = _getEnv('HTTP_PROXY', platform).isNotEmpty,
+        _httpProxy = _getEnv('HTTP_PROXY', platform),
+        _noProxy = _getEnv('NO_PROXY', platform),
+        super('Proxy Configuration');
 
-  final String _httpProxy = _getEnv('HTTP_PROXY');
-  final String _noProxy = _getEnv('NO_PROXY');
+  final bool shouldShow;
+  final String _httpProxy;
+  final String _noProxy;
 
   /// Gets a trimmed, non-null environment variable. If the variable is not set
   /// an empty string will be returned. Checks for the lowercase version of the
   /// environment variable first, then uppercase to match Dart's HTTP implementation.
-  static String _getEnv(String key) =>
-      globals.platform.environment[key.toLowerCase()]?.trim() ??
-      globals.platform.environment[key.toUpperCase()]?.trim() ??
-      '';
+  static String _getEnv(String key, Platform platform) =>
+    platform.environment[key.toLowerCase()]?.trim() ??
+    platform.environment[key.toUpperCase()]?.trim() ??
+    '';
 
   @override
   Future<ValidationResult> validate() async {
-    final List<ValidationMessage> messages = <ValidationMessage>[];
-
-    if (_httpProxy.isNotEmpty) {
-      messages.add(const ValidationMessage('HTTP_PROXY is set'));
-
-      if (_noProxy.isEmpty) {
-        messages.add(const ValidationMessage.hint('NO_PROXY is not set'));
-      } else {
-        messages.add(ValidationMessage('NO_PROXY is $_noProxy'));
-        for (final String host in const <String>['127.0.0.1', 'localhost']) {
-          final ValidationMessage msg = _noProxy.contains(host)
-              ? ValidationMessage('NO_PROXY contains $host')
-              : ValidationMessage.hint('NO_PROXY does not contain $host');
-
-          messages.add(msg);
-        }
-      }
+    if (_httpProxy.isEmpty) {
+      return const ValidationResult(
+          ValidationType.installed, <ValidationMessage>[]);
     }
 
-    final bool hasIssues =
-        messages.any((ValidationMessage msg) => msg.isHint || msg.isHint);
+    final List<ValidationMessage> messages = <ValidationMessage>[
+      const ValidationMessage('HTTP_PROXY is set'),
+      if (_noProxy.isEmpty)
+        const ValidationMessage.hint('NO_PROXY is not set')
+      else
+        ...<ValidationMessage>[
+          ValidationMessage('NO_PROXY is $_noProxy'),
+          for (String host in const <String>['127.0.0.1', 'localhost'])
+            if (_noProxy.contains(host))
+              ValidationMessage('NO_PROXY contains $host')
+            else
+              ValidationMessage.hint('NO_PROXY does not contain $host'),
+        ],
+    ];
+
+    final bool hasIssues = messages.any(
+      (ValidationMessage msg) => msg.isHint || msg.isError);
 
     return ValidationResult(
       hasIssues ? ValidationType.partial : ValidationType.installed,

packages/flutter_tools/test/commands.shard/hermetic/doctor_test.dart

@@ -20,7 +20,6 @@ import 'package:flutter_tools/src/doctor.dart';
 import 'package:flutter_tools/src/features.dart';
 import 'package:flutter_tools/src/globals.dart' as globals;
 import 'package:flutter_tools/src/ios/plist_parser.dart';
-import 'package:flutter_tools/src/proxy_validator.dart';
 import 'package:flutter_tools/src/reporting/reporting.dart';
 import 'package:flutter_tools/src/version.dart';
 import 'package:flutter_tools/src/vscode/vscode.dart';
@@ -170,91 +169,6 @@ void main() {
     }, overrides: noColorTerminalOverride);
   });
 
-  group('proxy validator', () {
-    testUsingContext('does not show if HTTP_PROXY is not set', () {
-      expect(ProxyValidator.shouldShow, isFalse);
-    }, overrides: <Type, Generator>{
-      Platform: () => FakePlatform()..environment = <String, String>{},
-    });
-
-    testUsingContext('does not show if HTTP_PROXY is only whitespace', () {
-      expect(ProxyValidator.shouldShow, isFalse);
-    }, overrides: <Type, Generator>{
-      Platform: () =>
-          FakePlatform()..environment = <String, String>{'HTTP_PROXY': ' '},
-    });
-
-    testUsingContext('shows when HTTP_PROXY is set', () {
-      expect(ProxyValidator.shouldShow, isTrue);
-    }, overrides: <Type, Generator>{
-      Platform: () => FakePlatform()
-        ..environment = <String, String>{'HTTP_PROXY': 'fakeproxy.local'},
-    });
-
-    testUsingContext('shows when http_proxy is set', () {
-      expect(ProxyValidator.shouldShow, isTrue);
-    }, overrides: <Type, Generator>{
-      Platform: () => FakePlatform()
-        ..environment = <String, String>{'http_proxy': 'fakeproxy.local'},
-    });
-
-    testUsingContext('reports success when NO_PROXY is configured correctly', () async {
-      final ValidationResult results = await ProxyValidator().validate();
-      final List<ValidationMessage> issues = results.messages
-          .where((ValidationMessage msg) => msg.isError || msg.isHint)
-          .toList();
-      expect(issues, hasLength(0));
-    }, overrides: <Type, Generator>{
-      Platform: () => FakePlatform()
-        ..environment = <String, String>{
-          'HTTP_PROXY': 'fakeproxy.local',
-          'NO_PROXY': 'localhost,127.0.0.1',
-        },
-    });
-
-    testUsingContext('reports success when no_proxy is configured correctly', () async {
-      final ValidationResult results = await ProxyValidator().validate();
-      final List<ValidationMessage> issues = results.messages
-          .where((ValidationMessage msg) => msg.isError || msg.isHint)
-          .toList();
-      expect(issues, hasLength(0));
-    }, overrides: <Type, Generator>{
-      Platform: () => FakePlatform()
-        ..environment = <String, String>{
-          'http_proxy': 'fakeproxy.local',
-          'no_proxy': 'localhost,127.0.0.1',
-        },
-    });
-
-    testUsingContext('reports issues when NO_PROXY is missing localhost', () async {
-      final ValidationResult results = await ProxyValidator().validate();
-      final List<ValidationMessage> issues = results.messages
-          .where((ValidationMessage msg) => msg.isError || msg.isHint)
-          .toList();
-      expect(issues, isNot(hasLength(0)));
-    }, overrides: <Type, Generator>{
-      Platform: () => FakePlatform()
-        ..environment = <String, String>{
-          'HTTP_PROXY': 'fakeproxy.local',
-          'NO_PROXY': '127.0.0.1',
-        },
-    });
-
-    testUsingContext('reports issues when NO_PROXY is missing 127.0.0.1', () async {
-      final ValidationResult results = await ProxyValidator().validate();
-      final List<ValidationMessage> issues = results.messages
-          .where((ValidationMessage msg) => msg.isError || msg.isHint)
-          .toList();
-      expect(issues, isNot(hasLength(0)));
-    }, overrides: <Type, Generator>{
-      Platform: () => FakePlatform()
-        ..environment = <String, String>{
-          'HTTP_PROXY': 'fakeproxy.local',
-          'NO_PROXY': 'localhost',
-        },
-    });
-  });
-
   group('doctor with overridden validators', () {
     testUsingContext('validate non-verbose output format for run without issues', () async {
       final Doctor doctor = Doctor(logger: logger);

packages/flutter_tools/test/commands.shard/hermetic/proxy_validator_test.dart

+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/doctor.dart';
+import 'package:flutter_tools/src/proxy_validator.dart';
+
+import '../../src/common.dart';
+
+void main() {
+  testWithoutContext('ProxyValidator does not show if HTTP_PROXY is not set', () {
+    final Platform platform = FakePlatform(environment: <String, String>{});
+
+    expect(ProxyValidator(platform: platform).shouldShow, isFalse);
+  });
+
+  testWithoutContext('ProxyValidator does not show if HTTP_PROXY is only whitespace', () {
+    final Platform platform = FakePlatform(environment: <String, String>{'HTTP_PROXY': ' '});
+
+    expect(ProxyValidator(platform: platform).shouldShow, isFalse);
+  });
+
+  testWithoutContext('ProxyValidator shows when HTTP_PROXY is set', () {
+    final Platform platform = FakePlatform(environment: <String, String>{'HTTP_PROXY': 'fakeproxy.local'});
+
+    expect(ProxyValidator(platform: platform).shouldShow, isTrue);
+  });
+
+  testWithoutContext('ProxyValidator shows when http_proxy is set', () {
+    final Platform platform = FakePlatform(environment: <String, String>{'http_proxy': 'fakeproxy.local'});
+
+    expect(ProxyValidator(platform: platform).shouldShow, isTrue);
+  });
+
+  testWithoutContext('ProxyValidator reports success when NO_PROXY is configured correctly', () async {
+    final Platform platform = FakePlatform(
+      environment: <String, String>{
+        'HTTP_PROXY': 'fakeproxy.local',
+        'NO_PROXY': 'localhost,127.0.0.1',
+      },
+    );
+    final ValidationResult results = await ProxyValidator(platform: platform).validate();
+
+    expect(results.messages, const <ValidationMessage>[
+      ValidationMessage('HTTP_PROXY is set'),
+      ValidationMessage('NO_PROXY is localhost,127.0.0.1'),
+      ValidationMessage('NO_PROXY contains 127.0.0.1'),
+      ValidationMessage('NO_PROXY contains localhost'),
+    ]);
+  });
+
+  testWithoutContext('ProxyValidator reports success when no_proxy is configured correctly', () async {
+    final Platform platform = FakePlatform(
+      environment: <String, String>{
+        'http_proxy': 'fakeproxy.local',
+        'no_proxy': 'localhost,127.0.0.1',
+      },
+    );
+    final ValidationResult results = await ProxyValidator(platform: platform).validate();
+
+    expect(results.messages, const <ValidationMessage>[
+      ValidationMessage('HTTP_PROXY is set'),
+      ValidationMessage('NO_PROXY is localhost,127.0.0.1'),
+      ValidationMessage('NO_PROXY contains 127.0.0.1'),
+      ValidationMessage('NO_PROXY contains localhost'),
+    ]);
+  });
+
+  testWithoutContext('ProxyValidator reports issues when NO_PROXY is missing localhost', () async {
+    final Platform platform = FakePlatform(
+      environment: <String, String>{
+        'HTTP_PROXY': 'fakeproxy.local',
+        'NO_PROXY': '127.0.0.1',
+      },
+    );
+    final ValidationResult results = await ProxyValidator(platform: platform).validate();
+
+    expect(results.messages, const <ValidationMessage>[
+      ValidationMessage('HTTP_PROXY is set'),
+      ValidationMessage('NO_PROXY is 127.0.0.1'),
+      ValidationMessage('NO_PROXY contains 127.0.0.1'),
+      ValidationMessage.hint('NO_PROXY does not contain localhost'),
+    ]);
+  });
+
+  testWithoutContext('ProxyValidator reports issues when NO_PROXY is missing 127.0.0.1', () async {
+    final Platform platform =  FakePlatform(environment: <String, String>{
+      'HTTP_PROXY': 'fakeproxy.local',
+      'NO_PROXY': 'localhost',
+    });
+    final ValidationResult results = await ProxyValidator(platform: platform).validate();
+
+    expect(results.messages, const <ValidationMessage>[
+      ValidationMessage('HTTP_PROXY is set'),
+      ValidationMessage('NO_PROXY is localhost'),
+      ValidationMessage.hint('NO_PROXY does not contain 127.0.0.1'),
+      ValidationMessage('NO_PROXY contains localhost'),
+    ]);
+  });
+}