From 95217eb72a5c21ec4fbaef64a2be404a6df21848 Mon Sep 17 00:00:00 2001 From: Lau Ching Jun Date: Tue, 3 Aug 2021 21:09:56 -0700 Subject: [PATCH] Add new hook for setupHotReload and after updating devFS is complete (#87532) * Add new hook for setupHotReload and after updating devFS is complete * Handle the case where updateDevFS failed --- packages/flutter_tools/lib/src/run_hot.dart | 29 +- .../test/general.shard/hot_test.dart | 484 ++++++++++++------ 2 files changed, 342 insertions(+), 171 deletions(-) diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index f2033f11b1a..f37198cc36d 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -52,6 +52,18 @@ class HotRunnerConfig { Future setupHotRestart() async { return true; } + + /// A hook for implementations to perform any necessary initialization prior + /// to a hot reload. Should return true if the hot restart should continue. + Future setupHotReload() async { + return true; + } + + /// A hook for implementations to perform any necessary cleanup after the + /// devfs sync is complete. At this point the flutter_tools no longer needs to + /// access the source files and assets. + void updateDevFSComplete() {} + /// A hook for implementations to perform any necessary operations right /// before the runner is about to be shut down. Future runPreShutdownOperations() async { @@ -546,7 +558,12 @@ class HotRunner extends ResidentRunner { String reason, }) async { final Stopwatch restartTimer = Stopwatch()..start(); - final UpdateFSReport updatedDevFS = await _updateDevFS(fullRestart: true); + UpdateFSReport updatedDevFS; + try { + updatedDevFS = await _updateDevFS(fullRestart: true); + } finally { + hotRunnerConfig.updateDevFSComplete(); + } if (!updatedDevFS.success) { for (final FlutterDevice device in flutterDevices) { if (device.generator != null) { @@ -857,8 +874,16 @@ class HotRunner extends ResidentRunner { } final Stopwatch reloadTimer = _stopwatchFactory.createStopwatch('reloadSources:reload')..start(); + if (!(await hotRunnerConfig.setupHotReload())) { + return OperationResult(1, 'setupHotReload failed'); + } final Stopwatch devFSTimer = Stopwatch()..start(); - final UpdateFSReport updatedDevFS = await _updateDevFS(); + UpdateFSReport updatedDevFS; + try { + updatedDevFS= await _updateDevFS(); + } finally { + hotRunnerConfig.updateDevFSComplete(); + } // Record time it took to synchronize to DevFS. bool shouldReportReloadTime = true; _addBenchmarkData('hotReloadDevFSSyncMilliseconds', devFSTimer.elapsed.inMilliseconds); diff --git a/packages/flutter_tools/test/general.shard/hot_test.dart b/packages/flutter_tools/test/general.shard/hot_test.dart index 2f728393feb..ff74534bbfa 100644 --- a/packages/flutter_tools/test/general.shard/hot_test.dart +++ b/packages/flutter_tools/test/general.shard/hot_test.dart @@ -157,37 +157,83 @@ void main() { testUsage = TestUsage(); }); - testUsingContext('setup function fails', () async { - fileSystem.file('.packages') - ..createSync(recursive: true) - ..writeAsStringSync('\n'); - final FakeDevice device = FakeDevice(); - final List devices = [ - FlutterDevice(device, generator: residentCompiler, buildInfo: BuildInfo.debug)..devFS = FakeDevFs(), - ]; - final OperationResult result = await HotRunner( - devices, - debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug), - target: 'main.dart', - devtoolsHandler: createNoOpHandler, - ).restart(fullRestart: true); - expect(result.isOk, false); - expect(result.message, 'setupHotRestart failed'); - }, overrides: { - HotRunnerConfig: () => TestHotRunnerConfig(successfulSetup: false), - Artifacts: () => Artifacts.test(), - FileSystem: () => fileSystem, - Platform: () => FakePlatform(operatingSystem: 'linux'), - ProcessManager: () => FakeProcessManager.any(), + group('fails to setup', () { + TestHotRunnerConfig failingTestingConfig; + setUp(() { + failingTestingConfig = TestHotRunnerConfig( + successfulHotRestartSetup: false, + successfulHotReloadSetup: false, + ); + }); + + testUsingContext('setupHotRestart function fails', () async { + fileSystem.file('.packages') + ..createSync(recursive: true) + ..writeAsStringSync('\n'); + final FakeDevice device = FakeDevice(); + final List devices = [ + FlutterDevice(device, generator: residentCompiler, buildInfo: BuildInfo.debug)..devFS = FakeDevFs(), + ]; + final OperationResult result = await HotRunner( + devices, + debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug), + target: 'main.dart', + devtoolsHandler: createNoOpHandler, + ).restart(fullRestart: true); + expect(result.isOk, false); + expect(result.message, 'setupHotRestart failed'); + expect(failingTestingConfig.updateDevFSCompleteCalled, false); + }, overrides: { + HotRunnerConfig: () => failingTestingConfig, + Artifacts: () => Artifacts.test(), + FileSystem: () => fileSystem, + Platform: () => FakePlatform(operatingSystem: 'linux'), + ProcessManager: () => FakeProcessManager.any(), + }); + + testUsingContext('setupHotReload function fails', () async { + fileSystem.file('.packages') + ..createSync(recursive: true) + ..writeAsStringSync('\n'); + final FakeDevice device = FakeDevice(); + final FakeFlutterDevice fakeFlutterDevice = FakeFlutterDevice(device); + final List devices = [ + fakeFlutterDevice, + ]; + final OperationResult result = await HotRunner( + devices, + debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug), + target: 'main.dart', + devtoolsHandler: createNoOpHandler, + reassembleHelper: ( + List flutterDevices, + Map> viewCache, + void Function(String message) onSlow, + String reloadMessage, + String fastReassembleClassName, + ) async => ReassembleResult( + {null: null}, + false, + true, + ), + ).restart(fullRestart: false); + expect(result.isOk, false); + expect(result.message, 'setupHotReload failed'); + expect(failingTestingConfig.updateDevFSCompleteCalled, false); + }, overrides: { + HotRunnerConfig: () => failingTestingConfig, + Artifacts: () => Artifacts.test(), + FileSystem: () => fileSystem, + Platform: () => FakePlatform(operatingSystem: 'linux'), + ProcessManager: () => FakeProcessManager.any(), + }); }); group('shutdown hook tests', () { TestHotRunnerConfig shutdownTestingConfig; setUp(() { - shutdownTestingConfig = TestHotRunnerConfig( - successfulSetup: true, - ); + shutdownTestingConfig = TestHotRunnerConfig(); }); testUsingContext('shutdown hook called after signal', () async { @@ -235,156 +281,242 @@ void main() { }); }); - testUsingContext('correctly tracks time spent for analytics for hot restart', () async { - final FakeDevice device = FakeDevice(); - final FakeFlutterDevice fakeFlutterDevice = FakeFlutterDevice(device); - final List devices = [ - fakeFlutterDevice, - ]; + group('successful hot restart', () { + TestHotRunnerConfig testingConfig; + setUp(() { + testingConfig = TestHotRunnerConfig( + successfulHotRestartSetup: true, + ); + }); + testUsingContext('correctly tracks time spent for analytics for hot restart', () async { + final FakeDevice device = FakeDevice(); + final FakeFlutterDevice fakeFlutterDevice = FakeFlutterDevice(device); + final List devices = [ + fakeFlutterDevice, + ]; - fakeFlutterDevice.updateDevFSReport = UpdateFSReport( - success: true, - invalidatedSourcesCount: 2, - syncedBytes: 4, - scannedSourcesCount: 8, - compileDuration: const Duration(seconds: 16), - transferDuration: const Duration(seconds: 32), - ); + fakeFlutterDevice.updateDevFSReportCallback = () async => UpdateFSReport( + success: true, + invalidatedSourcesCount: 2, + syncedBytes: 4, + scannedSourcesCount: 8, + compileDuration: const Duration(seconds: 16), + transferDuration: const Duration(seconds: 32), + ); - final FakeStopwatchFactory fakeStopwatchFactory = FakeStopwatchFactory( - stopwatches: { - 'fullRestartHelper': FakeStopwatch()..elapsed = const Duration(seconds: 64), - 'updateDevFS': FakeStopwatch()..elapsed = const Duration(seconds: 128), - }, - ); + final FakeStopwatchFactory fakeStopwatchFactory = FakeStopwatchFactory( + stopwatches: { + 'fullRestartHelper': FakeStopwatch()..elapsed = const Duration(seconds: 64), + 'updateDevFS': FakeStopwatch()..elapsed = const Duration(seconds: 128), + }, + ); - (fakeFlutterDevice.devFS as FakeDevFs).baseUri = Uri.parse('file:///base_uri'); + (fakeFlutterDevice.devFS as FakeDevFs).baseUri = Uri.parse('file:///base_uri'); - final OperationResult result = await HotRunner( - devices, - debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug), - target: 'main.dart', - devtoolsHandler: createNoOpHandler, - stopwatchFactory: fakeStopwatchFactory, - ).restart(fullRestart: true); + final OperationResult result = await HotRunner( + devices, + debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug), + target: 'main.dart', + devtoolsHandler: createNoOpHandler, + stopwatchFactory: fakeStopwatchFactory, + ).restart(fullRestart: true); - expect(result.isOk, true); - expect(testUsage.events, [ - const TestUsageEvent('hot', 'restart', parameters: CustomDimensions( - hotEventTargetPlatform: 'flutter-tester', - hotEventSdkName: 'Tester', - hotEventEmulator: false, - hotEventFullRestart: true, - hotEventOverallTimeInMs: 64000, - hotEventSyncedBytes: 4, - hotEventInvalidatedSourcesCount: 2, - hotEventTransferTimeInMs: 32000, - hotEventCompileTimeInMs: 16000, - hotEventFindInvalidatedTimeInMs: 128000, - hotEventScannedSourcesCount: 8, - )), - ]); - }, overrides: { - HotRunnerConfig: () => TestHotRunnerConfig(successfulSetup: true), - Artifacts: () => Artifacts.test(), - FileSystem: () => fileSystem, - Platform: () => FakePlatform(operatingSystem: 'linux'), - ProcessManager: () => FakeProcessManager.any(), - Usage: () => testUsage, + expect(result.isOk, true); + expect(testUsage.events, [ + const TestUsageEvent('hot', 'restart', parameters: CustomDimensions( + hotEventTargetPlatform: 'flutter-tester', + hotEventSdkName: 'Tester', + hotEventEmulator: false, + hotEventFullRestart: true, + hotEventOverallTimeInMs: 64000, + hotEventSyncedBytes: 4, + hotEventInvalidatedSourcesCount: 2, + hotEventTransferTimeInMs: 32000, + hotEventCompileTimeInMs: 16000, + hotEventFindInvalidatedTimeInMs: 128000, + hotEventScannedSourcesCount: 8, + )), + ]); + expect(testingConfig.updateDevFSCompleteCalled, true); + }, overrides: { + HotRunnerConfig: () => testingConfig, + Artifacts: () => Artifacts.test(), + FileSystem: () => fileSystem, + Platform: () => FakePlatform(operatingSystem: 'linux'), + ProcessManager: () => FakeProcessManager.any(), + Usage: () => testUsage, + }); }); - testUsingContext('correctly tracks time spent for analytics for hot reload', () async { - final FakeDevice device = FakeDevice(); - final FakeFlutterDevice fakeFlutterDevice = FakeFlutterDevice(device); - final List devices = [ - fakeFlutterDevice, - ]; + group('successful hot reload', () { + TestHotRunnerConfig testingConfig; + setUp(() { + testingConfig = TestHotRunnerConfig( + successfulHotReloadSetup: true, + ); + }); + testUsingContext('correctly tracks time spent for analytics for hot reload', () async { + final FakeDevice device = FakeDevice(); + final FakeFlutterDevice fakeFlutterDevice = FakeFlutterDevice(device); + final List devices = [ + fakeFlutterDevice, + ]; - fakeFlutterDevice.updateDevFSReport = UpdateFSReport( - success: true, - invalidatedSourcesCount: 6, - syncedBytes: 8, - scannedSourcesCount: 16, - compileDuration: const Duration(seconds: 16), - transferDuration: const Duration(seconds: 32), - ); + fakeFlutterDevice.updateDevFSReportCallback = () async => UpdateFSReport( + success: true, + invalidatedSourcesCount: 6, + syncedBytes: 8, + scannedSourcesCount: 16, + compileDuration: const Duration(seconds: 16), + transferDuration: const Duration(seconds: 32), + ); - final FakeStopwatchFactory fakeStopwatchFactory = FakeStopwatchFactory( - stopwatches: { - 'updateDevFS': FakeStopwatch()..elapsed = const Duration(seconds: 64), - 'reloadSources:reload': FakeStopwatch()..elapsed = const Duration(seconds: 128), - 'reloadSources:reassemble': FakeStopwatch()..elapsed = const Duration(seconds: 256), - 'reloadSources:vm': FakeStopwatch()..elapsed = const Duration(seconds: 512), - }, - ); + final FakeStopwatchFactory fakeStopwatchFactory = FakeStopwatchFactory( + stopwatches: { + 'updateDevFS': FakeStopwatch()..elapsed = const Duration(seconds: 64), + 'reloadSources:reload': FakeStopwatch()..elapsed = const Duration(seconds: 128), + 'reloadSources:reassemble': FakeStopwatch()..elapsed = const Duration(seconds: 256), + 'reloadSources:vm': FakeStopwatch()..elapsed = const Duration(seconds: 512), + }, + ); - (fakeFlutterDevice.devFS as FakeDevFs).baseUri = Uri.parse('file:///base_uri'); + (fakeFlutterDevice.devFS as FakeDevFs).baseUri = Uri.parse('file:///base_uri'); - final OperationResult result = await HotRunner( - devices, - debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug), - target: 'main.dart', - devtoolsHandler: createNoOpHandler, - stopwatchFactory: fakeStopwatchFactory, - reloadSourcesHelper: ( - HotRunner hotRunner, - List flutterDevices, - bool pause, - Map firstReloadDetails, - String targetPlatform, - String sdkName, - bool emulator, - String reason, - ) async { - firstReloadDetails['finalLibraryCount'] = 2; - firstReloadDetails['receivedLibraryCount'] = 3; - firstReloadDetails['receivedClassesCount'] = 4; - firstReloadDetails['receivedProceduresCount'] = 5; - return OperationResult.ok; - }, - reassembleHelper: ( - List flutterDevices, - Map> viewCache, - void Function(String message) onSlow, - String reloadMessage, - String fastReassembleClassName, - ) async => ReassembleResult( - {null: null}, - false, - true, - ), - ).restart(fullRestart: false); + final OperationResult result = await HotRunner( + devices, + debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug), + target: 'main.dart', + devtoolsHandler: createNoOpHandler, + stopwatchFactory: fakeStopwatchFactory, + reloadSourcesHelper: ( + HotRunner hotRunner, + List flutterDevices, + bool pause, + Map firstReloadDetails, + String targetPlatform, + String sdkName, + bool emulator, + String reason, + ) async { + firstReloadDetails['finalLibraryCount'] = 2; + firstReloadDetails['receivedLibraryCount'] = 3; + firstReloadDetails['receivedClassesCount'] = 4; + firstReloadDetails['receivedProceduresCount'] = 5; + return OperationResult.ok; + }, + reassembleHelper: ( + List flutterDevices, + Map> viewCache, + void Function(String message) onSlow, + String reloadMessage, + String fastReassembleClassName, + ) async => ReassembleResult( + {null: null}, + false, + true, + ), + ).restart(fullRestart: false); - expect(result.isOk, true); - expect(testUsage.events, [ - const TestUsageEvent('hot', 'reload', parameters: CustomDimensions( - hotEventFinalLibraryCount: 2, - hotEventSyncedLibraryCount: 3, - hotEventSyncedClassesCount: 4, - hotEventSyncedProceduresCount: 5, - hotEventSyncedBytes: 8, - hotEventInvalidatedSourcesCount: 6, - hotEventTransferTimeInMs: 32000, - hotEventOverallTimeInMs: 128000, - hotEventTargetPlatform: 'flutter-tester', - hotEventSdkName: 'Tester', - hotEventEmulator: false, - hotEventFullRestart: false, - fastReassemble: false, - hotEventCompileTimeInMs: 16000, - hotEventFindInvalidatedTimeInMs: 64000, - hotEventScannedSourcesCount: 16, - hotEventReassembleTimeInMs: 256000, - hotEventReloadVMTimeInMs: 512000, - )), - ]); - }, overrides: { - HotRunnerConfig: () => TestHotRunnerConfig(successfulSetup: true), - Artifacts: () => Artifacts.test(), - FileSystem: () => fileSystem, - Platform: () => FakePlatform(operatingSystem: 'linux'), - ProcessManager: () => FakeProcessManager.any(), - Usage: () => testUsage, + expect(result.isOk, true); + expect(testUsage.events, [ + const TestUsageEvent('hot', 'reload', parameters: CustomDimensions( + hotEventFinalLibraryCount: 2, + hotEventSyncedLibraryCount: 3, + hotEventSyncedClassesCount: 4, + hotEventSyncedProceduresCount: 5, + hotEventSyncedBytes: 8, + hotEventInvalidatedSourcesCount: 6, + hotEventTransferTimeInMs: 32000, + hotEventOverallTimeInMs: 128000, + hotEventTargetPlatform: 'flutter-tester', + hotEventSdkName: 'Tester', + hotEventEmulator: false, + hotEventFullRestart: false, + fastReassemble: false, + hotEventCompileTimeInMs: 16000, + hotEventFindInvalidatedTimeInMs: 64000, + hotEventScannedSourcesCount: 16, + hotEventReassembleTimeInMs: 256000, + hotEventReloadVMTimeInMs: 512000, + )), + ]); + expect(testingConfig.updateDevFSCompleteCalled, true); + }, overrides: { + HotRunnerConfig: () => testingConfig, + Artifacts: () => Artifacts.test(), + FileSystem: () => fileSystem, + Platform: () => FakePlatform(operatingSystem: 'linux'), + ProcessManager: () => FakeProcessManager.any(), + Usage: () => testUsage, + }); + }); + + group('hot restart that failed to sync dev fs', () { + TestHotRunnerConfig testingConfig; + setUp(() { + testingConfig = TestHotRunnerConfig( + successfulHotRestartSetup: true, + ); + }); + testUsingContext('still calls the devfs complete callback', () async { + final FakeDevice device = FakeDevice(); + final FakeFlutterDevice fakeFlutterDevice = FakeFlutterDevice(device); + final List devices = [ + fakeFlutterDevice, + ]; + fakeFlutterDevice.updateDevFSReportCallback = () async => throw 'updateDevFS failed'; + + final HotRunner runner = HotRunner( + devices, + debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug), + target: 'main.dart', + devtoolsHandler: createNoOpHandler, + ); + + await expectLater(runner.restart(fullRestart: true), throwsA('updateDevFS failed')); + expect(testingConfig.updateDevFSCompleteCalled, true); + }, overrides: { + HotRunnerConfig: () => testingConfig, + Artifacts: () => Artifacts.test(), + FileSystem: () => fileSystem, + Platform: () => FakePlatform(operatingSystem: 'linux'), + ProcessManager: () => FakeProcessManager.any(), + Usage: () => testUsage, + }); + }); + + group('hot reload that failed to sync dev fs', () { + TestHotRunnerConfig testingConfig; + setUp(() { + testingConfig = TestHotRunnerConfig( + successfulHotReloadSetup: true, + ); + }); + testUsingContext('still calls the devfs complete callback', () async { + final FakeDevice device = FakeDevice(); + final FakeFlutterDevice fakeFlutterDevice = FakeFlutterDevice(device); + final List devices = [ + fakeFlutterDevice, + ]; + fakeFlutterDevice.updateDevFSReportCallback = () async => throw 'updateDevFS failed'; + + final HotRunner runner = HotRunner( + devices, + debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug), + target: 'main.dart', + devtoolsHandler: createNoOpHandler, + ); + + await expectLater(runner.restart(fullRestart: false), throwsA('updateDevFS failed')); + expect(testingConfig.updateDevFSCompleteCalled, true); + }, overrides: { + HotRunnerConfig: () => testingConfig, + Artifacts: () => Artifacts.test(), + FileSystem: () => fileSystem, + Platform: () => FakePlatform(operatingSystem: 'linux'), + ProcessManager: () => FakeProcessManager.any(), + Usage: () => testUsage, + }); }); }); @@ -420,7 +552,7 @@ void main() { ); expect(exitCode, 2); }, overrides: { - HotRunnerConfig: () => TestHotRunnerConfig(successfulSetup: true), + HotRunnerConfig: () => TestHotRunnerConfig(), Artifacts: () => Artifacts.test(), FileSystem: () => fileSystem, Platform: () => FakePlatform(operatingSystem: 'linux'), @@ -519,7 +651,7 @@ class FakeFlutterDevice extends Fake implements FlutterDevice { FakeFlutterDevice(this.device); bool stoppedEchoingDeviceLog = false; - UpdateFSReport updateDevFSReport; + Future Function() updateDevFSReportCallback; @override final FakeDevice device; @@ -552,7 +684,7 @@ class FakeFlutterDevice extends Fake implements FlutterDevice { @required String dillOutputPath, @required List invalidatedFiles, @required PackageConfig packageConfig, - }) async => updateDevFSReport; + }) => updateDevFSReportCallback(); } class TestFlutterDevice extends FlutterDevice { @@ -585,13 +717,27 @@ class TestFlutterDevice extends FlutterDevice { } class TestHotRunnerConfig extends HotRunnerConfig { - TestHotRunnerConfig({@required this.successfulSetup}); - bool successfulSetup; + TestHotRunnerConfig({this.successfulHotRestartSetup, this.successfulHotReloadSetup}); + bool successfulHotRestartSetup; + bool successfulHotReloadSetup; bool shutdownHookCalled = false; + bool updateDevFSCompleteCalled = false; @override Future setupHotRestart() async { - return successfulSetup; + assert(successfulHotRestartSetup != null, 'setupHotRestart is not expected to be called in this test.'); + return successfulHotRestartSetup; + } + + @override + Future setupHotReload() async { + assert(successfulHotReloadSetup != null, 'setupHotReload is not expected to be called in this test.'); + return successfulHotReloadSetup; + } + + @override + void updateDevFSComplete() { + updateDevFSCompleteCalled = true; } @override