From 618b94d35767e7968f1d24ac874cde81ee407c35 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 7 Sep 2022 12:29:48 -0400 Subject: [PATCH 1/5] removing ProcessException catch since doctor validator handles it already --- .../lib/src/windows/windows_version_validator.dart | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/packages/flutter_tools/lib/src/windows/windows_version_validator.dart b/packages/flutter_tools/lib/src/windows/windows_version_validator.dart index 27f6cd243dc75..a03dd73d5e04a 100644 --- a/packages/flutter_tools/lib/src/windows/windows_version_validator.dart +++ b/packages/flutter_tools/lib/src/windows/windows_version_validator.dart @@ -37,17 +37,7 @@ class WindowsVersionValidator extends DoctorValidator { @override Future validate() async { - final ProcessResult result; - try { - result = await _processManager.run(['systeminfo']); - } on ProcessException { - return const ValidationResult( - ValidationType.missing, - [], - statusInfo: - 'Unable to run Windows version check using built-in `systeminfo`', - ); - } + final ProcessResult result = await _processManager.run(['systeminfo']); if (result.exitCode != 0) { return const ValidationResult( From b650586471a6713c82b96849c7c0850c64570fbf Mon Sep 17 00:00:00 2001 From: Elias Yishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 7 Sep 2022 14:17:22 -0400 Subject: [PATCH 2/5] Apply Jonah suggestions from code review Co-authored-by: Jonah Williams --- packages/flutter_tools/lib/src/doctor.dart | 7 ++++--- .../lib/src/windows/windows_version_validator.dart | 8 ++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/flutter_tools/lib/src/doctor.dart b/packages/flutter_tools/lib/src/doctor.dart index 6c55b4599a805..7099f1b8c1ae0 100644 --- a/packages/flutter_tools/lib/src/doctor.dart +++ b/packages/flutter_tools/lib/src/doctor.dart @@ -130,9 +130,10 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider { flutterRoot: () => Cache.flutterRoot!, operatingSystemUtils: globals.os, ), - if (platform.isWindows) WindowsVersionValidator( - processManager: globals.processManager, - ), + if (platform.isWindows) + WindowsVersionValidator( + processManager: globals.processManager, + ), if (androidWorkflow!.appliesToHostPlatform) GroupedValidator([androidValidator!, androidLicenseValidator!]), if (globals.iosWorkflow!.appliesToHostPlatform || macOSWorkflow.appliesToHostPlatform) diff --git a/packages/flutter_tools/lib/src/windows/windows_version_validator.dart b/packages/flutter_tools/lib/src/windows/windows_version_validator.dart index a03dd73d5e04a..47440f44c485b 100644 --- a/packages/flutter_tools/lib/src/windows/windows_version_validator.dart +++ b/packages/flutter_tools/lib/src/windows/windows_version_validator.dart @@ -7,14 +7,14 @@ import 'package:process/process.dart'; import '../base/io.dart'; import '../doctor_validator.dart'; -const List unsupportedVersions = [ +/// Flutter only supports development on Windows host machines version 10 and greater. +const List kUnsupportedVersions = [ '6', '7', '8', ]; -/// Validator to be run with `flutter doctor` to check -/// Windows host machines if they are running supported versions. +/// Validator for supported Windows host machine operating system version. class WindowsVersionValidator extends DoctorValidator { const WindowsVersionValidator({required ProcessManager processManager}) : _processManager = processManager, @@ -58,7 +58,7 @@ class WindowsVersionValidator extends DoctorValidator { // Use the string split method to extract the major version // and check against the [unsupportedVersions] list final ValidationType windowsVersionStatus; - String statusInfo; + final String statusInfo; if (matches.length == 1 && !unsupportedVersions .contains(matches.elementAt(0).group(2)?.split('.').elementAt(0))) { From 7f690ae3b725e0563efb94105f6dc3cfe7fbd35b Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 7 Sep 2022 14:22:38 -0400 Subject: [PATCH 3/5] renaming variable --- .../lib/src/windows/windows_version_validator.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/flutter_tools/lib/src/windows/windows_version_validator.dart b/packages/flutter_tools/lib/src/windows/windows_version_validator.dart index 47440f44c485b..e5d63423372d4 100644 --- a/packages/flutter_tools/lib/src/windows/windows_version_validator.dart +++ b/packages/flutter_tools/lib/src/windows/windows_version_validator.dart @@ -56,11 +56,11 @@ class WindowsVersionValidator extends DoctorValidator { r'^(OS Version:\s*)([0-9]+\.[0-9]+\.[0-9]+)(.*)$', resultStdout); // Use the string split method to extract the major version - // and check against the [unsupportedVersions] list + // and check against the [kUnsupportedVersions] list final ValidationType windowsVersionStatus; final String statusInfo; if (matches.length == 1 && - !unsupportedVersions + !kUnsupportedVersions .contains(matches.elementAt(0).group(2)?.split('.').elementAt(0))) { windowsVersionStatus = ValidationType.installed; statusInfo = 'Installed version of Windows is version 10 or higher'; From 899a221d6eb21b304ca667ea34508bada6bcd5f9 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 7 Sep 2022 14:29:52 -0400 Subject: [PATCH 4/5] additional test added for a nonzero exit code from systeminfo --- .../windows_version_validator_test.dart | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/flutter_tools/test/general.shard/windows_version_validator_test.dart b/packages/flutter_tools/test/general.shard/windows_version_validator_test.dart index da849cab90f77..2762f16169221 100644 --- a/packages/flutter_tools/test/general.shard/windows_version_validator_test.dart +++ b/packages/flutter_tools/test/general.shard/windows_version_validator_test.dart @@ -120,6 +120,14 @@ const ValidationResult invalidWindowsValidationResult = ValidationResult( statusInfo: 'Unable to confirm if installed Windows version is 10 or greater', ); +/// Expected return from a nonzero exitcode when +/// running systeminfo +const ValidationResult invalidExitCodeValidationResult = ValidationResult( + ValidationType.missing, + [], + statusInfo: 'Exit status from running `systeminfo` was unsuccessful', +); + void main() { testWithoutContext('Successfully running windows version check on windows 10', () async { @@ -186,6 +194,27 @@ void main() { reason: 'The ValidationResult statusInfo messages should be the same'); }); + testWithoutContext('Running into an nonzero exit code from systeminfo command', () async { + final WindowsVersionValidator windowsVersionValidator = + WindowsVersionValidator( + processManager: FakeProcessManager.list( + [ + const FakeCommand( + command: ['systeminfo'], + exitCode: 1 + ), + ], + ), + ); + + final ValidationResult result = await windowsVersionValidator.validate(); + + expect(result.type, invalidExitCodeValidationResult.type, + reason: 'The ValidationResult type should be the same (missing)'); + expect(result.statusInfo, invalidExitCodeValidationResult.statusInfo, + reason: 'The ValidationResult statusInfo messages should be the same'); + }); + testWithoutContext('Unit testing on a regex pattern validator', () async { const String regexPattern = r'^(OS Version:\s*)([0-9]+\.[0-9]+\.[0-9]+)(.*)$'; From ababef68d421aad9bde9cb5c9b4bd3a59a631e2f Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 7 Sep 2022 15:36:09 -0400 Subject: [PATCH 5/5] removing trivial static method, added constant for semver pattern --- .../windows/windows_version_validator.dart | 29 +++++++------------ .../windows_version_validator_test.dart | 15 ++++------ 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/packages/flutter_tools/lib/src/windows/windows_version_validator.dart b/packages/flutter_tools/lib/src/windows/windows_version_validator.dart index e5d63423372d4..bc43193e3eace 100644 --- a/packages/flutter_tools/lib/src/windows/windows_version_validator.dart +++ b/packages/flutter_tools/lib/src/windows/windows_version_validator.dart @@ -14,6 +14,11 @@ const List kUnsupportedVersions = [ '8', ]; +/// Regex pattern for identifying line from systeminfo stdout with windows version +/// (ie. 10.5.4123) +const String kWindowsOSVersionSemVerPattern = + r'^(OS Version:\s*)([0-9]+\.[0-9]+\.[0-9]+)(.*)$'; + /// Validator for supported Windows host machine operating system version. class WindowsVersionValidator extends DoctorValidator { const WindowsVersionValidator({required ProcessManager processManager}) @@ -22,22 +27,10 @@ class WindowsVersionValidator extends DoctorValidator { final ProcessManager _processManager; - /// Provide a literal string as the Regex pattern - /// and a string to validate and get a boolean determining - /// if the string has at least one match - static Iterable validateString(String pattern, String str, - {bool multiLine = true}) { - final RegExp regex = RegExp( - pattern, - multiLine: multiLine, - ); - - return regex.allMatches(str); - } - @override Future validate() async { - final ProcessResult result = await _processManager.run(['systeminfo']); + final ProcessResult result = + await _processManager.run(['systeminfo']); if (result.exitCode != 0) { return const ValidationResult( @@ -49,11 +42,9 @@ class WindowsVersionValidator extends DoctorValidator { final String resultStdout = result.stdout as String; - // Regular expression pattern for identifying - // semantic versioned strings - // (ie. 10.5.4123) - final Iterable matches = validateString( - r'^(OS Version:\s*)([0-9]+\.[0-9]+\.[0-9]+)(.*)$', resultStdout); + final RegExp regex = + RegExp(kWindowsOSVersionSemVerPattern, multiLine: true); + final Iterable matches = regex.allMatches(resultStdout); // Use the string split method to extract the major version // and check against the [kUnsupportedVersions] list diff --git a/packages/flutter_tools/test/general.shard/windows_version_validator_test.dart b/packages/flutter_tools/test/general.shard/windows_version_validator_test.dart index 2762f16169221..0b5fa3b7805b2 100644 --- a/packages/flutter_tools/test/general.shard/windows_version_validator_test.dart +++ b/packages/flutter_tools/test/general.shard/windows_version_validator_test.dart @@ -194,15 +194,13 @@ void main() { reason: 'The ValidationResult statusInfo messages should be the same'); }); - testWithoutContext('Running into an nonzero exit code from systeminfo command', () async { + testWithoutContext( + 'Running into an nonzero exit code from systeminfo command', () async { final WindowsVersionValidator windowsVersionValidator = WindowsVersionValidator( processManager: FakeProcessManager.list( [ - const FakeCommand( - command: ['systeminfo'], - exitCode: 1 - ), + const FakeCommand(command: ['systeminfo'], exitCode: 1), ], ), ); @@ -216,8 +214,6 @@ void main() { }); testWithoutContext('Unit testing on a regex pattern validator', () async { - const String regexPattern = - r'^(OS Version:\s*)([0-9]+\.[0-9]+\.[0-9]+)(.*)$'; const String testStr = r''' OS Version: 10.0.19044 N/A Build 19044 OSz Version: 10.0.19044 N/A Build 19044 @@ -229,8 +225,9 @@ OS Version: 10.0.19044 N/A Build 19044 OS Version: .0.19044 N/A Build 19044 '''; - final Iterable matches = - WindowsVersionValidator.validateString(regexPattern, testStr); + final RegExp regex = + RegExp(kWindowsOSVersionSemVerPattern, multiLine: true); + final Iterable matches = regex.allMatches(testStr); expect(matches.length, 2, reason: 'There should be only two matches for the pattern provided');