From f796e047fd968204d53097c8b4b7ea075d5795c2 Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Thu, 16 Jul 2020 11:31:03 -0700 Subject: [PATCH] Verbose process exceptions (#61552) --- packages/flutter_tools/lib/src/base/os.dart | 1 + .../flutter_tools/lib/src/base/process.dart | 9 +++++- .../test/general.shard/base/os_test.dart | 26 +++++++++++++++++ .../test/general.shard/base/process_test.dart | 29 ++++++++++++++++--- packages/flutter_tools/test/src/common.dart | 18 +++++++----- 5 files changed, 70 insertions(+), 13 deletions(-) diff --git a/packages/flutter_tools/lib/src/base/os.dart b/packages/flutter_tools/lib/src/base/os.dart index 2f7e242ecbc..0c7929cc916 100644 --- a/packages/flutter_tools/lib/src/base/os.dart +++ b/packages/flutter_tools/lib/src/base/os.dart @@ -226,6 +226,7 @@ class _PosixUtils extends OperatingSystemUtils { _processUtils.runSync( ['unzip', '-o', '-q', file.path, '-d', targetDirectory.path], throwOnError: true, + verboseExceptions: true, ); } diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index a4d33ff1697..a2965d2474b 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/packages/flutter_tools/lib/src/base/process.dart @@ -231,6 +231,7 @@ abstract class ProcessUtils { RunResult runSync( List cmd, { bool throwOnError = false, + bool verboseExceptions = false, RunResultChecker allowedFailures, bool hideStdout = false, String workingDirectory, @@ -408,6 +409,7 @@ class _DefaultProcessUtils implements ProcessUtils { RunResult runSync( List cmd, { bool throwOnError = false, + bool verboseExceptions = false, RunResultChecker allowedFailures, bool hideStdout = false, String workingDirectory, @@ -449,7 +451,12 @@ class _DefaultProcessUtils implements ProcessUtils { } if (failedExitCode && throwOnError) { - runResult.throwException('The command failed'); + String message = 'The command failed'; + if (verboseExceptions) { + message = 'The command failed\nStdout:\n${runResult.stdout}\n' + 'Stderr:\n${runResult.stderr}'; + } + runResult.throwException(message); } return runResult; diff --git a/packages/flutter_tools/test/general.shard/base/os_test.dart b/packages/flutter_tools/test/general.shard/base/os_test.dart index 6013ea0db61..a2e7ac16ab2 100644 --- a/packages/flutter_tools/test/general.shard/base/os_test.dart +++ b/packages/flutter_tools/test/general.shard/base/os_test.dart @@ -89,6 +89,31 @@ void main() { }); }); + testWithoutContext('If unzip fails, include stderr in exception text', () { + const String exceptionMessage = 'Something really bad happened.'; + when(mockProcessManager.runSync( + ['unzip', '-o', '-q', null, '-d', null], + )).thenReturn(ProcessResult(0, 1, '', exceptionMessage)); + final MockFileSystem fileSystem = MockFileSystem(); + final MockFile mockFile = MockFile(); + final MockDirectory mockDirectory = MockDirectory(); + when(fileSystem.file(any)).thenReturn(mockFile); + when(mockFile.readAsBytesSync()).thenThrow( + const FileSystemException(exceptionMessage), + ); + final OperatingSystemUtils osUtils = OperatingSystemUtils( + fileSystem: fileSystem, + logger: BufferLogger.test(), + platform: FakePlatform(operatingSystem: 'linux'), + processManager: mockProcessManager, + ); + + expect( + () => osUtils.unzip(mockFile, mockDirectory), + throwsProcessException(message: exceptionMessage), + ); + }); + group('gzip on Windows:', () { testWithoutContext('verifyGzip returns false on a FileSystemException', () { final MockFileSystem fileSystem = MockFileSystem(); @@ -148,5 +173,6 @@ void main() { } class MockProcessManager extends Mock implements ProcessManager {} +class MockDirectory extends Mock implements Directory {} class MockFileSystem extends Mock implements FileSystem {} class MockFile extends Mock implements File {} diff --git a/packages/flutter_tools/test/general.shard/base/process_test.dart b/packages/flutter_tools/test/general.shard/base/process_test.dart index a66708d9d5d..12f9104cc30 100644 --- a/packages/flutter_tools/test/general.shard/base/process_test.dart +++ b/packages/flutter_tools/test/general.shard/base/process_test.dart @@ -286,12 +286,33 @@ void main() { expect(processUtils.runSync(['boohoo']).exitCode, 1); }); - testWithoutContext(' throws on failure with throwOnError', () async { + testWithoutContext('throws on failure with throwOnError', () async { + const String stderr = 'Something went wrong.'; when(mockProcessManager.runSync(['kaboom'])).thenReturn( - ProcessResult(0, 1, '', '') + ProcessResult(0, 1, '', stderr), + ); + try { + processUtils.runSync(['kaboom'], throwOnError: true); + fail('ProcessException expected.'); + } on ProcessException catch (e) { + expect(e, isA()); + expect(e.message.contains(stderr), false); + } + }); + + testWithoutContext('throws with stderr in exception on failure with verboseExceptions', () async { + const String stderr = 'Something went wrong.'; + when(mockProcessManager.runSync(['verybad'])).thenReturn( + ProcessResult(0, 1, '', stderr), + ); + expect( + () => processUtils.runSync( + ['verybad'], + throwOnError: true, + verboseExceptions: true, + ), + throwsProcessException(message: stderr), ); - expect(() => processUtils.runSync(['kaboom'], throwOnError: true), - throwsA(isA())); }); testWithoutContext(' does not throw on allowed Failures', () async { diff --git a/packages/flutter_tools/test/src/common.dart b/packages/flutter_tools/test/src/common.dart index ed4dbece992..734b9a0ee62 100644 --- a/packages/flutter_tools/test/src/common.dart +++ b/packages/flutter_tools/test/src/common.dart @@ -13,7 +13,7 @@ import 'package:vm_service/vm_service.dart' as vm_service; import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/base/file_system.dart'; -import 'package:flutter_tools/src/base/process.dart'; +import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/commands/create.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; import 'package:flutter_tools/src/runner/flutter_command_runner.dart'; @@ -106,15 +106,17 @@ Matcher throwsToolExit({ int exitCode, Pattern message }) { /// Matcher for [ToolExit]s. final test_package.TypeMatcher isToolExit = isA(); -/// Matcher for functions that throw [ProcessExit]. -Matcher throwsProcessExit([ dynamic exitCode ]) { - return exitCode == null - ? throwsA(isProcessExit) - : throwsA(allOf(isProcessExit, (ProcessExit e) => e.exitCode == exitCode)); +/// Matcher for functions that throw [ProcessException]. +Matcher throwsProcessException({ Pattern message }) { + Matcher matcher = isProcessException; + if (message != null) { + matcher = allOf(matcher, (ProcessException e) => e.message?.contains(message)); + } + return throwsA(matcher); } -/// Matcher for [ProcessExit]s. -final test_package.TypeMatcher isProcessExit = isA(); +/// Matcher for [ProcessException]s. +final test_package.TypeMatcher isProcessException = isA(); /// Creates a flutter project in the [temp] directory using the /// [arguments] list if specified, or `--no-pub` if not.