From 8755de632713f5ed564ff2e561d1eacb507cf801 Mon Sep 17 00:00:00 2001 From: Daniel Chevalier Date: Thu, 29 Jun 2023 14:19:29 -0400 Subject: [PATCH 1/3] manual minor+dev bump (#5968) ![](https://media.giphy.com/media/Pm08ZLlxa1mWttBOgt/giphy-downsized.gif) The automatic version bumping workflow is currently broken, so I'm pushing up the dev bump manually for now. --- packages/devtools_app/lib/devtools.dart | 2 +- packages/devtools_app/pubspec.yaml | 6 +++--- .../release_notes/NEXT_RELEASE_NOTES.md | 18 +++--------------- packages/devtools_shared/pubspec.yaml | 2 +- packages/devtools_test/pubspec.yaml | 6 +++--- 5 files changed, 11 insertions(+), 23 deletions(-) diff --git a/packages/devtools_app/lib/devtools.dart b/packages/devtools_app/lib/devtools.dart index 33752e66357..a23654ced93 100644 --- a/packages/devtools_app/lib/devtools.dart +++ b/packages/devtools_app/lib/devtools.dart @@ -9,4 +9,4 @@ // the constant declaration `const String version =`. // If you change the declaration you must also modify the regex in // tools/update_version.dart. -const String version = '2.25.0'; +const String version = '2.26.0-dev.0'; diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml index 1cfe43f77ac..3d9a361e80d 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml @@ -4,7 +4,7 @@ publish_to: none # Note: this version should only be updated by running tools/update_version.dart # that updates all versions of devtools packages (devtools_app, devtools_test). -version: 2.25.0 +version: 2.26.0-dev.0 repository: https://github.com/flutter/devtools/tree/master/packages/devtools_app @@ -24,7 +24,7 @@ dependencies: codicon: ^1.0.0 collection: ^1.15.0 dds_service_extensions: ^1.3.2 - devtools_shared: 2.25.0 + devtools_shared: 2.26.0-dev.0 file: ^6.0.0 file_selector: ^0.8.0 file_selector_linux: ^0.0.2 @@ -65,7 +65,7 @@ dependencies: dev_dependencies: args: ^2.4.2 build_runner: ^2.3.3 - devtools_test: 2.25.0 + devtools_test: 2.26.0-dev.0 fake_async: ^1.3.1 flutter_driver: sdk: flutter diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 4890cd43a9f..1a564f8f48c 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -1,49 +1,37 @@ This is draft for future release notes, that are going to land on [the Flutter website](https://docs.flutter.dev/development/tools/devtools/release-notes). -# DevTools 2.25.0 release notes +# DevTools 2.26.0 release notes Dart & Flutter DevTools - A Suite of Performance Tools for Dart and Flutter ## General updates - -- Improve DevTools tab bar navigation when the list of tabs is long - [#5875](https://github.com/flutter/devtools/pull/5875) -- Clear registered service methods between app connections - [#5960](https://github.com/flutter/devtools/pull/5960) +TODO: Remove this section if there are not any general updates. ## Inspector updates - TODO: Remove this section if there are not any general updates. ## Performance updates - TODO: Remove this section if there are not any general updates. ## CPU profiler updates - TODO: Remove this section if there are not any general updates. ## Memory updates - -- Add legend for class types - [#5937](https://github.com/flutter/devtools/pull/5937) -- Enable sampling for Memory > Profile - [#5947](https://github.com/flutter/devtools/pull/5947) +TODO: Remove this section if there are not any general updates. ## Debugger updates - TODO: Remove this section if there are not any general updates. ## Network profiler updates - TODO: Remove this section if there are not any general updates. ## Logging updates - TODO: Remove this section if there are not any general updates. ## App size tool updates - TODO: Remove this section if there are not any general updates. ## Full commit history - More details about changes and fixes are available from the [DevTools git log.](https://github.com/flutter/devtools/commits/master). diff --git a/packages/devtools_shared/pubspec.yaml b/packages/devtools_shared/pubspec.yaml index 70f109a5357..e8397b32319 100644 --- a/packages/devtools_shared/pubspec.yaml +++ b/packages/devtools_shared/pubspec.yaml @@ -1,7 +1,7 @@ name: devtools_shared description: Package of shared structures between devtools_app, dds, and other tools. -version: 2.25.0 +version: 2.26.0-dev.0 repository: https://github.com/flutter/devtools/tree/master/packages/devtools_shared diff --git a/packages/devtools_test/pubspec.yaml b/packages/devtools_test/pubspec.yaml index 2a2878b54f9..794d75703bd 100644 --- a/packages/devtools_test/pubspec.yaml +++ b/packages/devtools_test/pubspec.yaml @@ -7,7 +7,7 @@ publish_to: none # When publishing new versions of this package be sure to publish a new version # of package:devtools as well. package:devtools contains a compiled snapshot of # this package. -version: 2.25.0 +version: 2.26.0-dev.0 repository: https://github.com/flutter/devtools/tree/master/packages/devtools_test @@ -18,8 +18,8 @@ environment: dependencies: async: ^2.0.0 collection: ^1.15.0 - devtools_shared: 2.25.0 - devtools_app: 2.25.0 + devtools_shared: 2.26.0-dev.0 + devtools_app: 2.26.0-dev.0 flutter: sdk: flutter flutter_test: From 7733b732c336c2afd28b58886531fbf1936b2749 Mon Sep 17 00:00:00 2001 From: Harsh Kumar Date: Fri, 30 Jun 2023 01:09:11 +0530 Subject: [PATCH 2/3] feat(Network profiler): create response selector for http response (#5816) * feat(Network profiler): create response selector for http response * fix: spelling mistakes, parameters name and minor refactoring * refactor: convert HttpResponseTrailingDropDown to stateless widget * refactor: use RoundedDropDownButton and change drop down type * refactor: remove unused factory fromString constructor for NetworkResponseType * refactor: change NetworkResponseType to NetworkResponseViewType * refactor: remove unnecessary ignore comment * refactor: convert HttpResponseViewer to stateless widget * refactor: fix spelling * refactor: remove setCurrentLocalResponseTypeMethod * update release notes * test: Write tests for HttpTextResponseViewer and HttpTrailingDropDown * test: refactor test for HttpResponseViewer and HttpTrailingDropDown --- .../screens/network/network_controller.dart | 33 ++++ .../network/network_request_inspector.dart | 22 ++- .../network_request_inspector_views.dart | 138 +++++++++++++++- .../src/screens/network/network_screen.dart | 1 + .../release_notes/NEXT_RELEASE_NOTES.md | 2 +- .../test/network/network_profiler_test.dart | 2 + .../network_request_inspector_test.dart | 154 ++++++++++++++++++ 7 files changed, 339 insertions(+), 13 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/network/network_controller.dart b/packages/devtools_app/lib/src/screens/network/network_controller.dart index ba4ac56160f..9979d1525fd 100644 --- a/packages/devtools_app/lib/src/screens/network/network_controller.dart +++ b/packages/devtools_app/lib/src/screens/network/network_controller.dart @@ -19,6 +19,23 @@ import 'network_model.dart'; import 'network_screen.dart'; import 'network_service.dart'; +/// Different types of Network Response which can be used to visualise response +/// on Response tab +enum NetworkResponseViewType { + auto, + text, + json; + + @override + String toString() { + return switch (this) { + NetworkResponseViewType.json => 'Json', + NetworkResponseViewType.text => 'Text', + _ => 'Auto', + }; + } +} + class NetworkController extends DisposableController with SearchControllerMixin, @@ -50,6 +67,22 @@ class NetworkController extends DisposableController final _requests = ValueNotifier(NetworkRequests()); + /// Notifies that current response type has been changed + ValueListenable get currentResponseViewType => + _currentResponseViewType; + + final _currentResponseViewType = + ValueNotifier(NetworkResponseViewType.auto); + + /// Change current response type + set setResponseViewType(NetworkResponseViewType type) => + _currentResponseViewType.value = type; + + /// Reset drop down to initial state when current network request is changed + void resetDropDown() { + _currentResponseViewType.value = NetworkResponseViewType.auto; + } + final selectedRequest = ValueNotifier(null); late CurrentNetworkRequests _currentNetworkRequests; diff --git a/packages/devtools_app/lib/src/screens/network/network_request_inspector.dart b/packages/devtools_app/lib/src/screens/network/network_request_inspector.dart index 2e81cbe0f5a..2a1c54aadab 100644 --- a/packages/devtools_app/lib/src/screens/network/network_request_inspector.dart +++ b/packages/devtools_app/lib/src/screens/network/network_request_inspector.dart @@ -96,12 +96,26 @@ class NetworkRequestInspector extends StatelessWidget { ( tab: _buildTab( tabName: NetworkRequestInspector._responseTabTitle, - trailing: HttpViewTrailingCopyButton( - data, - (data) => data.responseBody, + trailing: Row( + children: [ + HttpResponseTrailingDropDown( + data, + currentResponseViewType: + controller.currentResponseViewType, + onChanged: (value) => + controller.setResponseViewType = value, + ), + HttpViewTrailingCopyButton( + data, + (data) => data.responseBody, + ), + ], ), ), - tabView: HttpResponseView(data), + tabView: HttpResponseView( + data, + currentResponseViewType: controller.currentResponseViewType, + ), ), if (data.hasCookies) ( diff --git a/packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart b/packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart index a57d8bfd180..5ac4c2d6f02 100644 --- a/packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart +++ b/packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:convert'; + +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:image/image.dart' as image; @@ -12,6 +15,7 @@ import '../../shared/primitives/utils.dart'; import '../../shared/table/table.dart'; import '../../shared/theme.dart'; import '../../shared/ui/colors.dart'; +import 'network_controller.dart'; import 'network_model.dart'; // Approximately double the indent of the expandable tile's title. @@ -201,10 +205,86 @@ class HttpViewTrailingCopyButton extends StatelessWidget { } } +/// A DropDownButton for selecting [NetworkResponseViewType]. +/// +/// If there is no content to visualise, the drop down will not show. Drop down +/// values will update as the request's data is updated. +class HttpResponseTrailingDropDown extends StatelessWidget { + const HttpResponseTrailingDropDown( + this.data, { + super.key, + required this.currentResponseViewType, + required this.onChanged, + }); + + final ValueListenable currentResponseViewType; + final DartIOHttpRequestData data; + final ValueChanged onChanged; + + bool isJsonDecodable() { + try { + json.decode(data.responseBody!); + return true; + } catch (_) { + return false; + } + } + + @override + Widget build(BuildContext context) { + return ValueListenableBuilder( + valueListenable: data.requestUpdatedNotifier, + builder: (_, __, ___) { + final bool visible = (data.contentType != null && + !data.contentType!.contains('image')) && + data.responseBody!.isNotEmpty; + + final List availableResponseTypes = [ + NetworkResponseViewType.auto, + if (isJsonDecodable()) NetworkResponseViewType.json, + NetworkResponseViewType.text, + ]; + + return Visibility( + visible: visible, + replacement: const SizedBox(), + child: ValueListenableBuilder( + valueListenable: currentResponseViewType, + builder: (_, currentType, __) { + return RoundedDropDownButton( + value: currentType, + items: availableResponseTypes + .map( + (e) => DropdownMenuItem( + value: e, + child: Text(e.toString()), + ), + ) + .toList(), + onChanged: (value) { + if (value == null) { + return; + } + onChanged(value); + }, + ); + }, + ), + ); + }, + ); + } +} + class HttpResponseView extends StatelessWidget { - const HttpResponseView(this.data, {super.key}); + const HttpResponseView( + this.data, { + super.key, + required this.currentResponseViewType, + }); final DartIOHttpRequestData data; + final ValueListenable currentResponseViewType; @override Widget build(BuildContext context) { @@ -226,14 +306,12 @@ class HttpResponseView extends StatelessWidget { } if (contentType != null && contentType.contains('image')) { child = ImageResponseView(data); - } else if (contentType != null && - contentType.contains('json') && - responseBody.isNotEmpty) { - child = JsonViewer(encodedJson: responseBody); } else { - child = Text( - responseBody, - style: theme.fixedFontStyle, + child = HttpTextResponseViewer( + contentType: contentType, + responseBody: responseBody, + currentResponseNotifier: currentResponseViewType, + textStyle: theme.fixedFontStyle, ); } return Padding( @@ -245,6 +323,50 @@ class HttpResponseView extends StatelessWidget { } } +class HttpTextResponseViewer extends StatelessWidget { + const HttpTextResponseViewer({ + super.key, + required this.contentType, + required this.responseBody, + required this.currentResponseNotifier, + required this.textStyle, + }); + + final String? contentType; + final String responseBody; + final ValueListenable currentResponseNotifier; + final TextStyle textStyle; + + @override + Widget build(BuildContext context) { + return ValueListenableBuilder( + valueListenable: currentResponseNotifier, + builder: (_, currentResponseType, __) { + NetworkResponseViewType currentLocalResponseType = currentResponseType; + + if (currentResponseType == NetworkResponseViewType.auto) { + if (contentType != null && + contentType!.contains('json') && + responseBody.isNotEmpty) { + currentLocalResponseType = NetworkResponseViewType.json; + } else { + currentLocalResponseType = NetworkResponseViewType.text; + } + } + + return switch (currentLocalResponseType) { + NetworkResponseViewType.json => JsonViewer(encodedJson: responseBody), + NetworkResponseViewType.text => Text( + responseBody, + style: textStyle, + ), + _ => const SizedBox() + }; + }, + ); + } +} + class ImageResponseView extends StatelessWidget { const ImageResponseView(this.data, {super.key}); diff --git a/packages/devtools_app/lib/src/screens/network/network_screen.dart b/packages/devtools_app/lib/src/screens/network/network_screen.dart index 8e8d0638e49..4cc6dc15353 100644 --- a/packages/devtools_app/lib/src/screens/network/network_screen.dart +++ b/packages/devtools_app/lib/src/screens/network/network_screen.dart @@ -353,6 +353,7 @@ class NetworkRequestsTable extends StatelessWidget { onItemSelected: (item) { if (item is DartIOHttpRequestData) { unawaited(item.getFullRequestData()); + networkController.resetDropDown(); } }, ), diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 1a564f8f48c..df1af05c6ca 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -24,7 +24,7 @@ TODO: Remove this section if there are not any general updates. TODO: Remove this section if there are not any general updates. ## Network profiler updates -TODO: Remove this section if there are not any general updates. +* Added a selector to customize the display type of text and json responses (thanks to @hhacker1999!) - [#5816](https://github.com/flutter/devtools/pull/5816) ## Logging updates TODO: Remove this section if there are not any general updates. diff --git a/packages/devtools_app/test/network/network_profiler_test.dart b/packages/devtools_app/test/network/network_profiler_test.dart index 29b5bc70d9d..e92c21761fa 100644 --- a/packages/devtools_app/test/network/network_profiler_test.dart +++ b/packages/devtools_app/test/network/network_profiler_test.dart @@ -173,6 +173,8 @@ void main() { .tap(find.byKey(NetworkRequestInspector.responseTabKey)); await tester.pumpAndSettle(); + expect(find.byType(HttpResponseTrailingDropDown), findsOneWidget); + expect(find.byType(HttpViewTrailingCopyButton), findsOneWidget); expect(find.byType(NetworkRequestOverviewView), findsNothing); expect(find.byType(HttpRequestHeadersView), findsNothing); expect(find.byType(HttpResponseView), findsOneWidget); diff --git a/packages/devtools_app/test/network/network_request_inspector_test.dart b/packages/devtools_app/test/network/network_request_inspector_test.dart index 8bf2e6376e4..e4db35b6d3d 100644 --- a/packages/devtools_app/test/network/network_request_inspector_test.dart +++ b/packages/devtools_app/test/network/network_request_inspector_test.dart @@ -6,7 +6,9 @@ import 'dart:convert'; import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app/src/screens/network/network_request_inspector.dart'; +import 'package:devtools_app/src/screens/network/network_request_inspector_views.dart'; import 'package:devtools_test/devtools_test.dart'; +import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:vm_service/vm_service.dart'; @@ -129,5 +131,157 @@ void main() { // pumpAndSettle so residual http timers can clear. await tester.pumpAndSettle(const Duration(seconds: 1)); }); + + group('HttpResponseTrailingDropDown', () { + testWidgets( + 'drop down value should update when response view type changes', + (tester) async { + NetworkResponseViewType? getCurrentDropDownValue() { + final RoundedDropDownButton dropDownWidget = + find + .byType(RoundedDropDownButton) + .evaluate() + .first + .widget as RoundedDropDownButton; + return dropDownWidget.value; + } + + final currentResponseViewType = ValueNotifier( + NetworkResponseViewType.auto, + ); + + // Matches Drop Down value with currentResponseViewType + void checkDropDownValue() { + final currentDropDownValue = getCurrentDropDownValue(); + expect(currentDropDownValue, equals(currentResponseViewType.value)); + } + + await tester.pumpWidget( + wrapWithControllers( + HttpResponseTrailingDropDown( + httpGet, + currentResponseViewType: currentResponseViewType, + onChanged: (value) { + currentResponseViewType.value = value; + }, + ), + debugger: createMockDebuggerControllerWithDefaults(), + ), + ); + + await tester.pumpAndSettle(); + checkDropDownValue(); + + currentResponseViewType.value = NetworkResponseViewType.text; + await tester.pumpAndSettle(); + checkDropDownValue(); + + currentResponseViewType.value = NetworkResponseViewType.auto; + await tester.pumpAndSettle(); + checkDropDownValue(); + + // pumpAndSettle so residual http timers can clear. + await tester.pumpAndSettle(const Duration(seconds: 1)); + }); + + testWidgets( + 'onChanged handler should trigger when changing drop down value', + (tester) async { + final currentResponseViewType = ValueNotifier( + NetworkResponseViewType.auto, + ); + String initial = 'Not changed'; + const String afterOnChanged = 'changed'; + + await tester.pumpWidget( + wrapWithControllers( + HttpResponseTrailingDropDown( + httpGet, + currentResponseViewType: currentResponseViewType, + onChanged: (value) { + initial = afterOnChanged; + }, + ), + debugger: createMockDebuggerControllerWithDefaults(), + ), + ); + + final dropDownFinder = find.byType( + RoundedDropDownButton, + ); + + await tester.tap(dropDownFinder); + await tester.pumpAndSettle(); + + // Select Json from drop down + await tester.tap( + find.text( + NetworkResponseViewType.json.toString(), + ), + ); + + await tester.pumpAndSettle(); + + expect( + initial, + afterOnChanged, + ); + + // pumpAndSettle so residual http timers can clear. + await tester.pumpAndSettle(const Duration(seconds: 1)); + }); + }); + + testWidgets( + 'should update response view display when drop down value changes', + (tester) async { + final currentResponseNotifier = + ValueNotifier(NetworkResponseViewType.auto); + const contentType = 'application/json'; + final responseBody = httpGet.requestBody ?? '{}'; + const textStyle = TextStyle(); + + await tester.pumpWidget( + wrapWithControllers( + Column( + children: [ + HttpTextResponseViewer( + contentType: contentType, + responseBody: responseBody, + currentResponseNotifier: currentResponseNotifier, + textStyle: textStyle, + ), + HttpResponseTrailingDropDown( + httpGet, + currentResponseViewType: currentResponseNotifier, + onChanged: (value) {}, + ), + ], + ), + debugger: createMockDebuggerControllerWithDefaults(), + ), + ); + + await tester.pumpAndSettle(); + + currentResponseNotifier.value = NetworkResponseViewType.json; + + await tester.pumpAndSettle(); + + // Check that Json viewer is visible + Finder jsonViewer = find.byType(JsonViewer); + expect(jsonViewer, findsOneWidget); + + currentResponseNotifier.value = NetworkResponseViewType.text; + + await tester.pumpAndSettle(); + + // Check that Json viewer is not visible + jsonViewer = find.byType(JsonViewer); + expect(jsonViewer, findsNothing); + + // pumpAndSettle so residual http timers can clear. + await tester.pumpAndSettle(const Duration(seconds: 1)); + }); }); } From 6658554cfc5b8c806bee5efdf0e1ecbfcd01328f Mon Sep 17 00:00:00 2001 From: Kenzie Davisson <43759233+kenzieschmoll@users.noreply.github.com> Date: Thu, 29 Jun 2023 15:05:19 -0700 Subject: [PATCH 3/3] Start chromedriver once for running all integration tests (#5976) --- .../integration_test/run_tests.dart | 55 +++++++++++------- .../test_infra/run/_chrome_driver.dart | 37 ++++++++++++ .../test_infra/run/_utils.dart | 13 +++++ .../test_infra/run/run_test.dart | 57 +++---------------- 4 files changed, 93 insertions(+), 69 deletions(-) create mode 100644 packages/devtools_app/integration_test/test_infra/run/_chrome_driver.dart create mode 100644 packages/devtools_app/integration_test/test_infra/run/_utils.dart diff --git a/packages/devtools_app/integration_test/run_tests.dart b/packages/devtools_app/integration_test/run_tests.dart index f0aab1dc394..5f563f0fcd1 100644 --- a/packages/devtools_app/integration_test/run_tests.dart +++ b/packages/devtools_app/integration_test/run_tests.dart @@ -4,6 +4,7 @@ import 'dart:io'; +import 'test_infra/run/_chrome_driver.dart'; import 'test_infra/run/_in_file_args.dart'; import 'test_infra/run/run_test.dart'; @@ -18,26 +19,42 @@ const _testSuffix = '_test.dart'; const _offlineIndicator = 'integration_test/test/offline'; void main(List args) async { - final testRunnerArgs = TestRunnerArgs(args, verifyValidTarget: false); - if (testRunnerArgs.testTarget != null) { - // TODO(kenz): add support for specifying a directory as the target instead - // of a single file. - await _runTest(testRunnerArgs); - } else { - // Run all tests since a target test was not provided. - final testDirectory = Directory(_testDirectory); - final testFiles = testDirectory - .listSync(recursive: true) - .where((testFile) => testFile.path.endsWith(_testSuffix)); - - for (final testFile in testFiles) { - final testTarget = testFile.path; - final newArgsWithTarget = TestRunnerArgs([ - ...args, - '--${TestRunnerArgs.testTargetArg}=$testTarget', - ]); - await _runTest(newArgsWithTarget); + Exception? exception; + final chromedriver = ChromeDriver(); + + try { + // Start chrome driver before running the flutter integration test. + await chromedriver.start(); + + final testRunnerArgs = TestRunnerArgs(args, verifyValidTarget: false); + if (testRunnerArgs.testTarget != null) { + // TODO(kenz): add support for specifying a directory as the target instead + // of a single file. + await _runTest(testRunnerArgs); + } else { + // Run all tests since a target test was not provided. + final testDirectory = Directory(_testDirectory); + final testFiles = testDirectory + .listSync(recursive: true) + .where((testFile) => testFile.path.endsWith(_testSuffix)); + + for (final testFile in testFiles) { + final testTarget = testFile.path; + final newArgsWithTarget = TestRunnerArgs([ + ...args, + '--${TestRunnerArgs.testTargetArg}=$testTarget', + ]); + await _runTest(newArgsWithTarget); + } } + } on Exception catch (e) { + exception = e; + } finally { + await chromedriver.stop(); + } + + if (exception != null) { + throw exception; } } diff --git a/packages/devtools_app/integration_test/test_infra/run/_chrome_driver.dart b/packages/devtools_app/integration_test/test_infra/run/_chrome_driver.dart new file mode 100644 index 00000000000..363cfcb915d --- /dev/null +++ b/packages/devtools_app/integration_test/test_infra/run/_chrome_driver.dart @@ -0,0 +1,37 @@ +// Copyright 2023 The Chromium 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 'dart:io'; + +import '_io_utils.dart'; +import '_utils.dart'; + +class ChromeDriver with IOMixin { + late final Process _process; + + // TODO(kenz): add error messaging if the chromedriver executable is not + // found. We can also consider using web installers directly in this script: + // https://github.com/flutter/flutter/wiki/Running-Flutter-Driver-tests-with-Web#web-installers-repo. + Future start() async { + try { + debugLog('starting the chromedriver process'); + _process = await Process.start( + 'chromedriver', + [ + '--port=4444', + ], + ); + listenToProcessOutput(_process, printTag: 'ChromeDriver'); + } catch (e) { + // ignore: avoid-throw-in-catch-block, by design + throw Exception('Error starting chromedriver: $e'); + } + } + + Future stop() async { + await cancelAllStreamSubscriptions(); + debugLog('killing the chromedriver process'); + _process.kill(); + } +} diff --git a/packages/devtools_app/integration_test/test_infra/run/_utils.dart b/packages/devtools_app/integration_test/test_infra/run/_utils.dart new file mode 100644 index 00000000000..012ed47e72f --- /dev/null +++ b/packages/devtools_app/integration_test/test_infra/run/_utils.dart @@ -0,0 +1,13 @@ +// Copyright 2023 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// ignore_for_file: avoid_print + +bool debugTestScript = false; + +void debugLog(String log) { + if (debugTestScript) { + print(log); + } +} diff --git a/packages/devtools_app/integration_test/test_infra/run/run_test.dart b/packages/devtools_app/integration_test/test_infra/run/run_test.dart index cbb5311ceda..cbf799cbc84 100644 --- a/packages/devtools_app/integration_test/test_infra/run/run_test.dart +++ b/packages/devtools_app/integration_test/test_infra/run/run_test.dart @@ -14,8 +14,7 @@ import 'package:collection/collection.dart'; import '_in_file_args.dart'; import '_io_utils.dart'; import '_test_app_driver.dart'; - -bool _debugTestScript = false; +import '_utils.dart'; /// Runs one test. /// @@ -48,16 +47,6 @@ Future runFlutterIntegrationTest( } } - // TODO(kenz): do we need to start chromedriver in headless mode? - // Start chrome driver before running the flutter integration test. - final chromedriver = ChromeDriver(); - try { - await chromedriver.start(); - } catch (e) { - // ignore: avoid-throw-in-catch-block, by design - throw Exception('Error starting chromedriver: $e'); - } - // Run the flutter integration test. final testRunner = TestRunner(); Exception? exception; @@ -75,16 +64,12 @@ Future runFlutterIntegrationTest( exception = e; } finally { if (testApp != null) { - _debugLog('killing the test app'); + debugLog('killing the test app'); await testApp.stop(); } - _debugLog('cancelling stream subscriptions'); + debugLog('cancelling stream subscriptions'); await testRunner.cancelAllStreamSubscriptions(); - await chromedriver.cancelAllStreamSubscriptions(); - - _debugLog('killing the chromedriver process'); - chromedriver.kill(); } if (exception != null) { @@ -92,28 +77,6 @@ Future runFlutterIntegrationTest( } } -class ChromeDriver with IOMixin { - late final Process _process; - - // TODO(kenz): add error messaging if the chromedriver executable is not - // found. We can also consider using web installers directly in this script: - // https://github.com/flutter/flutter/wiki/Running-Flutter-Driver-tests-with-Web#web-installers-repo. - Future start() async { - _debugLog('starting the chromedriver process'); - _process = await Process.start( - 'chromedriver', - [ - '--port=4444', - ], - ); - listenToProcessOutput(_process, printTag: 'ChromeDriver'); - } - - void kill() { - _process.kill(); - } -} - class TestRunner with IOMixin { static const _beginExceptionMarker = 'EXCEPTION CAUGHT'; static const _endExceptionMarker = '═════════════════════════'; @@ -130,7 +93,7 @@ class TestRunner with IOMixin { Map testAppArguments = const {}, }) async { Future runTest({required int attemptNumber}) async { - _debugLog('starting the flutter drive process'); + debugLog('starting the flutter drive process'); final process = await Process.start( 'flutter', [ @@ -210,9 +173,9 @@ class TestRunner with IOMixin { timeout, ]); - _debugLog('attempting to kill the flutter drive process'); + debugLog('attempting to kill the flutter drive process'); process.kill(); - _debugLog('flutter drive process has exited'); + debugLog('flutter drive process has exited'); // Ignore exception handling and retries if the tests passed. This is to // avoid bugs with the test runner where the test can fail after the test @@ -224,7 +187,7 @@ class TestRunner with IOMixin { 'Integration test timed out on try #$attemptNumber: $testTarget', ); } else { - _debugLog( + debugLog( 'Integration test timed out on try #$attemptNumber. Retrying ' '$testTarget now.', ); @@ -276,12 +239,6 @@ class _TestResult { } } -void _debugLog(String log) { - if (_debugTestScript) { - print(log); - } -} - class TestRunnerArgs { TestRunnerArgs(List args, {bool verifyValidTarget = true}) { final argParser = _buildArgParser();