From 89f5f94c1b9e83ff200f764befd34d155544f36d Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Tue, 1 Oct 2019 16:34:12 -0700 Subject: [PATCH 01/17] CocoonResponse --- app_flutter/lib/state/flutter_build.dart | 29 +++++++++++++++++------- app_flutter/lib/status_grid.dart | 6 ++--- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index 8ad36c065d..86fc2ee257 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -30,11 +30,14 @@ class FlutterBuildState extends ChangeNotifier { Timer refreshTimer; /// The current status of the commits loaded. - List statuses = []; + final CocoonResponse> _statuses = + CocoonResponse>()..data = []; + CocoonResponse> get statuses => _statuses; /// Whether or not flutter/flutter currently passes tests. - bool get isTreeBuilding => _isTreeBuilding; - bool _isTreeBuilding = true; + final CocoonResponse _isTreeBuilding = CocoonResponse() + ..data = false; + CocoonResponse get isTreeBuilding => _isTreeBuilding; /// Start a fixed interval loop that fetches build state updates based on [refreshRate]. Future startFetchingBuildStateUpdates() async { @@ -52,12 +55,17 @@ class FlutterBuildState extends ChangeNotifier { /// Request the latest [statuses] from [CocoonService]. Future _fetchBuildStatusUpdate() async { - await Future.wait(>[ - _cocoonService.fetchCommitStatuses().then>( - (List commitStatuses) => statuses = commitStatuses), + await Future.wait(>[ _cocoonService - .fetchTreeBuildStatus() - .then((bool treeStatus) => _isTreeBuilding = treeStatus), + .fetchCommitStatuses() + .then((List commitStatuses) { + _statuses.data = commitStatuses; + _statuses.error = null; + }), + _cocoonService.fetchTreeBuildStatus().then((bool treeStatus) { + _isTreeBuilding.data = treeStatus; + _isTreeBuilding.error = null; + }), ]); notifyListeners(); @@ -69,3 +77,8 @@ class FlutterBuildState extends ChangeNotifier { super.dispose(); } } + +class CocoonResponse { + T data; + String error; +} diff --git a/app_flutter/lib/status_grid.dart b/app_flutter/lib/status_grid.dart index eefc7a2b55..993067f2c0 100644 --- a/app_flutter/lib/status_grid.dart +++ b/app_flutter/lib/status_grid.dart @@ -23,10 +23,10 @@ class StatusGridContainer extends StatelessWidget { Widget build(BuildContext context) { return Consumer( builder: (_, FlutterBuildState buildState, Widget child) { - final List statuses = buildState.statuses; + CocoonResponse> statuses = buildState.statuses; // Assume if there is no data that it is loading. - if (statuses.isEmpty) { + if (statuses.data.isEmpty) { return const Expanded( child: Center( child: CircularProgressIndicator(), @@ -35,7 +35,7 @@ class StatusGridContainer extends StatelessWidget { } return StatusGrid( - statuses: statuses, + statuses: statuses.data, ); }, ); From fd8712cb8aead7a1d84dbdc212bd6050c59ecd44 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Wed, 2 Oct 2019 09:44:36 -0700 Subject: [PATCH 02/17] show snackbar when there is an error --- app_flutter/lib/service/fake_cocoon.dart | 1 + app_flutter/lib/state/flutter_build.dart | 3 +++ app_flutter/lib/status_grid.dart | 13 ++++++++++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app_flutter/lib/service/fake_cocoon.dart b/app_flutter/lib/service/fake_cocoon.dart index 7f2e9d68cd..2e4275d6cb 100644 --- a/app_flutter/lib/service/fake_cocoon.dart +++ b/app_flutter/lib/service/fake_cocoon.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:io'; import 'dart:math'; import 'package:fixnum/fixnum.dart'; diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index 86fc2ee257..ad2cfde0a1 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -61,6 +61,9 @@ class FlutterBuildState extends ChangeNotifier { .then((List commitStatuses) { _statuses.data = commitStatuses; _statuses.error = null; + }, onError: (Exception error) { + _statuses.error = error.toString(); + print('error occurred'); }), _cocoonService.fetchTreeBuildStatus().then((bool treeStatus) { _isTreeBuilding.data = treeStatus; diff --git a/app_flutter/lib/status_grid.dart b/app_flutter/lib/status_grid.dart index 993067f2c0..ac034dcddf 100644 --- a/app_flutter/lib/status_grid.dart +++ b/app_flutter/lib/status_grid.dart @@ -21,9 +21,20 @@ class StatusGridContainer extends StatelessWidget { @override Widget build(BuildContext context) { + final ThemeData theme = Theme.of(context); return Consumer( builder: (_, FlutterBuildState buildState, Widget child) { - CocoonResponse> statuses = buildState.statuses; + final CocoonResponse> statuses = buildState.statuses; + if (statuses.error != null) { + print('there was an error!'); + final SnackBar snackbar = SnackBar( + content: const Text('Failed to get the latest commit statuses'), + backgroundColor: theme.errorColor, + duration: const Duration(seconds: 10), + behavior: SnackBarBehavior.floating, + ); + Scaffold.of(context).showSnackBar(snackbar); + } // Assume if there is no data that it is loading. if (statuses.data.isEmpty) { From efa41de6911fd2bf520e4dd5bbf6a9f87036c6b5 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Fri, 4 Oct 2019 11:00:00 -0700 Subject: [PATCH 03/17] moved error handling to parent BuildDashboard --- app_flutter/lib/build_dashboard.dart | 38 ++++++++++++++++-------- app_flutter/lib/service/fake_cocoon.dart | 1 - app_flutter/lib/state/flutter_build.dart | 16 ++++++---- app_flutter/lib/status_grid.dart | 10 ------- 4 files changed, 37 insertions(+), 28 deletions(-) diff --git a/app_flutter/lib/build_dashboard.dart b/app_flutter/lib/build_dashboard.dart index e5a46ae261..2778c47b30 100644 --- a/app_flutter/lib/build_dashboard.dart +++ b/app_flutter/lib/build_dashboard.dart @@ -44,20 +44,34 @@ class _BuildDashboardPageState extends State { class BuildDashboard extends StatelessWidget { @override Widget build(BuildContext context) { + final GlobalKey _scaffoldKey = GlobalKey(); final ThemeData theme = Theme.of(context); return Consumer( - builder: (_, FlutterBuildState buildState, Widget child) => Scaffold( - appBar: AppBar( - title: const Text('Flutter Build Dashboard v2'), - backgroundColor: - buildState.isTreeBuilding ? theme.primaryColor : theme.errorColor, - ), - body: Column( - children: const [ - StatusGridContainer(), - ], - ), - ), + builder: (_, FlutterBuildState buildState, Widget child) { + if (buildState.hasError) { + const SnackBar snackbar = SnackBar( + content: Text('Cocoon Backend is having issues'), + duration: Duration(seconds: 10), + behavior: SnackBarBehavior.floating, + ); + _scaffoldKey.currentState.showSnackBar(snackbar); + } + + return Scaffold( + key: _scaffoldKey, + appBar: AppBar( + title: const Text('Flutter Build Dashboard v2'), + backgroundColor: buildState.isTreeBuilding.data + ? theme.primaryColor + : theme.errorColor, + ), + body: Column( + children: const [ + StatusGridContainer(), + ], + ), + ); + }, ); } } diff --git a/app_flutter/lib/service/fake_cocoon.dart b/app_flutter/lib/service/fake_cocoon.dart index 2e4275d6cb..7f2e9d68cd 100644 --- a/app_flutter/lib/service/fake_cocoon.dart +++ b/app_flutter/lib/service/fake_cocoon.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:io'; import 'dart:math'; import 'package:fixnum/fixnum.dart'; diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index ad2cfde0a1..13cb688d37 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -39,6 +39,9 @@ class FlutterBuildState extends ChangeNotifier { ..data = false; CocoonResponse get isTreeBuilding => _isTreeBuilding; + /// Whether an error occured getting the latest data for the fields of this state. + bool get hasError => _isTreeBuilding.error ?? _statuses.error ?? false; + /// Start a fixed interval loop that fetches build state updates based on [refreshRate]. Future startFetchingBuildStateUpdates() async { if (refreshTimer != null) { @@ -56,18 +59,18 @@ class FlutterBuildState extends ChangeNotifier { /// Request the latest [statuses] from [CocoonService]. Future _fetchBuildStatusUpdate() async { await Future.wait(>[ - _cocoonService - .fetchCommitStatuses() - .then((List commitStatuses) { + _cocoonService.fetchCommitStatuses().then( + (List commitStatuses) { _statuses.data = commitStatuses; _statuses.error = null; - }, onError: (Exception error) { + }, onError: (dynamic error) { _statuses.error = error.toString(); - print('error occurred'); }), _cocoonService.fetchTreeBuildStatus().then((bool treeStatus) { _isTreeBuilding.data = treeStatus; _isTreeBuilding.error = null; + }, onError: (dynamic error) { + _isTreeBuilding.error = error.toString(); }), ]); @@ -81,6 +84,9 @@ class FlutterBuildState extends ChangeNotifier { } } +/// Wrapper class for data in this state. +/// +/// Holds [data] and information for errors. class CocoonResponse { T data; String error; diff --git a/app_flutter/lib/status_grid.dart b/app_flutter/lib/status_grid.dart index ac034dcddf..97c7658394 100644 --- a/app_flutter/lib/status_grid.dart +++ b/app_flutter/lib/status_grid.dart @@ -25,16 +25,6 @@ class StatusGridContainer extends StatelessWidget { return Consumer( builder: (_, FlutterBuildState buildState, Widget child) { final CocoonResponse> statuses = buildState.statuses; - if (statuses.error != null) { - print('there was an error!'); - final SnackBar snackbar = SnackBar( - content: const Text('Failed to get the latest commit statuses'), - backgroundColor: theme.errorColor, - duration: const Duration(seconds: 10), - behavior: SnackBarBehavior.floating, - ); - Scaffold.of(context).showSnackBar(snackbar); - } // Assume if there is no data that it is loading. if (statuses.data.isEmpty) { From 30796695349e2204b5d37e3fa0f8213758531e74 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Fri, 4 Oct 2019 11:15:19 -0700 Subject: [PATCH 04/17] better documentation --- app_flutter/lib/build_dashboard.dart | 5 +++++ app_flutter/lib/state/flutter_build.dart | 9 ++++++--- app_flutter/lib/status_grid.dart | 1 - 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app_flutter/lib/build_dashboard.dart b/app_flutter/lib/build_dashboard.dart index 2778c47b30..e0c2a14bf0 100644 --- a/app_flutter/lib/build_dashboard.dart +++ b/app_flutter/lib/build_dashboard.dart @@ -49,6 +49,11 @@ class BuildDashboard extends StatelessWidget { return Consumer( builder: (_, FlutterBuildState buildState, Widget child) { if (buildState.hasError) { + print('FlutterBuildState has an error'); + print('isTreeBuilding: ${buildState.isTreeBuilding.error}'); + print('statuses: ${buildState.statuses.error}'); + + // TODO(chillers): Make this display better on the TV by using a bigger widget. const SnackBar snackbar = SnackBar( content: Text('Cocoon Backend is having issues'), duration: Duration(seconds: 10), diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index 13cb688d37..a785fb78ba 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -56,7 +56,7 @@ class FlutterBuildState extends ChangeNotifier { Timer.periodic(refreshRate, (_) => _fetchBuildStatusUpdate()); } - /// Request the latest [statuses] from [CocoonService]. + /// Request the latest [statuses] and [isTreeBuilding] from [CocoonService]. Future _fetchBuildStatusUpdate() async { await Future.wait(>[ _cocoonService.fetchCommitStatuses().then( @@ -84,10 +84,13 @@ class FlutterBuildState extends ChangeNotifier { } } -/// Wrapper class for data in this state. +/// Wrapper class for data this state serves. /// -/// Holds [data] and information for errors. +/// Holds [data] and possible error information. class CocoonResponse { + /// The data that gets used from [CocoonService]. T data; + + /// Error information that can be used for debugging. String error; } diff --git a/app_flutter/lib/status_grid.dart b/app_flutter/lib/status_grid.dart index 97c7658394..190be21362 100644 --- a/app_flutter/lib/status_grid.dart +++ b/app_flutter/lib/status_grid.dart @@ -21,7 +21,6 @@ class StatusGridContainer extends StatelessWidget { @override Widget build(BuildContext context) { - final ThemeData theme = Theme.of(context); return Consumer( builder: (_, FlutterBuildState buildState, Widget child) { final CocoonResponse> statuses = buildState.statuses; From 3ac867de94bcb1b378a787193a6a469a2f46a973 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Fri, 4 Oct 2019 11:17:06 -0700 Subject: [PATCH 05/17] simplified status grid statuses --- app_flutter/lib/status_grid.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app_flutter/lib/status_grid.dart b/app_flutter/lib/status_grid.dart index 190be21362..0cc222cef5 100644 --- a/app_flutter/lib/status_grid.dart +++ b/app_flutter/lib/status_grid.dart @@ -23,10 +23,10 @@ class StatusGridContainer extends StatelessWidget { Widget build(BuildContext context) { return Consumer( builder: (_, FlutterBuildState buildState, Widget child) { - final CocoonResponse> statuses = buildState.statuses; + final List statuses = buildState.statuses.data; // Assume if there is no data that it is loading. - if (statuses.data.isEmpty) { + if (statuses.isEmpty) { return const Expanded( child: Center( child: CircularProgressIndicator(), @@ -35,7 +35,7 @@ class StatusGridContainer extends StatelessWidget { } return StatusGrid( - statuses: statuses.data, + statuses: statuses, ); }, ); From e2c72468965af5c7b72de7d814e9c489cd80f996 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Fri, 4 Oct 2019 13:50:52 -0700 Subject: [PATCH 06/17] added test for setting error in CocoonResponse --- .../test/state/flutter_build_test.dart | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/app_flutter/test/state/flutter_build_test.dart b/app_flutter/test/state/flutter_build_test.dart index 3f83f129b9..f5ce6bdafc 100644 --- a/app_flutter/test/state/flutter_build_test.dart +++ b/app_flutter/test/state/flutter_build_test.dart @@ -60,6 +60,35 @@ void main() { // Tear down fails to cancel the timer before the test is over buildState.dispose(); }); + + testWidgets('error when fetching statuses should be set in CocoonResponse', + (WidgetTester tester) async { + when(mockService.fetchCommitStatuses()) + .thenAnswer((_) => Future>.error(42)); + + buildState.startFetchingBuildStateUpdates(); + + await tester.pumpAndSettle(); + + expect(buildState.statuses.error, isNotNull); + + buildState.dispose(); + }); + + testWidgets( + 'error when fetching tree build status should be set in CocoonResponse', + (WidgetTester tester) async { + when(mockService.fetchTreeBuildStatus()) + .thenAnswer((_) => Future.error(42)); + + buildState.startFetchingBuildStateUpdates(); + + await tester.pumpAndSettle(); + + expect(buildState.isTreeBuilding.error, isNotNull); + + buildState.dispose(); + }); }); } From 8375553640f77801485b9ab53e84c67aeda1c0e8 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Fri, 4 Oct 2019 13:55:45 -0700 Subject: [PATCH 07/17] tested hasError --- app_flutter/lib/state/flutter_build.dart | 2 +- app_flutter/test/state/flutter_build_test.dart | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index a785fb78ba..9ce579a65d 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -40,7 +40,7 @@ class FlutterBuildState extends ChangeNotifier { CocoonResponse get isTreeBuilding => _isTreeBuilding; /// Whether an error occured getting the latest data for the fields of this state. - bool get hasError => _isTreeBuilding.error ?? _statuses.error ?? false; + bool get hasError => _isTreeBuilding != null || _statuses.error != null; /// Start a fixed interval loop that fetches build state updates based on [refreshRate]. Future startFetchingBuildStateUpdates() async { diff --git a/app_flutter/test/state/flutter_build_test.dart b/app_flutter/test/state/flutter_build_test.dart index f5ce6bdafc..6b72bd9a82 100644 --- a/app_flutter/test/state/flutter_build_test.dart +++ b/app_flutter/test/state/flutter_build_test.dart @@ -70,6 +70,7 @@ void main() { await tester.pumpAndSettle(); + expect(buildState.hasError, true); expect(buildState.statuses.error, isNotNull); buildState.dispose(); @@ -85,6 +86,7 @@ void main() { await tester.pumpAndSettle(); + expect(buildState.hasError, true); expect(buildState.isTreeBuilding.error, isNotNull); buildState.dispose(); From f942ace942accbd290db270ac9ac6ab5f8b75ef4 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Fri, 4 Oct 2019 14:51:21 -0700 Subject: [PATCH 08/17] working on seeing if BuildDashboard handles error --- app_flutter/lib/state/flutter_build.dart | 2 +- app_flutter/test/build_dashboard_test.dart | 48 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 app_flutter/test/build_dashboard_test.dart diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index 9ce579a65d..ae31a57705 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -40,7 +40,7 @@ class FlutterBuildState extends ChangeNotifier { CocoonResponse get isTreeBuilding => _isTreeBuilding; /// Whether an error occured getting the latest data for the fields of this state. - bool get hasError => _isTreeBuilding != null || _statuses.error != null; + bool get hasError => _isTreeBuilding.error != null || _statuses.error != null; /// Start a fixed interval loop that fetches build state updates based on [refreshRate]. Future startFetchingBuildStateUpdates() async { diff --git a/app_flutter/test/build_dashboard_test.dart b/app_flutter/test/build_dashboard_test.dart new file mode 100644 index 0000000000..3610b95a84 --- /dev/null +++ b/app_flutter/test/build_dashboard_test.dart @@ -0,0 +1,48 @@ +// Copyright (c) 2019 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 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:provider/provider.dart'; + +import 'package:cocoon_service/protos.dart' show CommitStatus; + +import 'package:app_flutter/build_dashboard.dart'; +import 'package:app_flutter/state/flutter_build.dart'; + +void main() { + group('BuildDashboard', () { + testWidgets('should show error when FlutterBuildState has error', + (WidgetTester tester) async { + final MockFlutterBuildState buildState = MockFlutterBuildState(); + + when(buildState.hasError).thenReturn(true); + when(buildState.statuses).thenReturn( + CocoonResponse>()..error = 'status error'); + when(buildState.isTreeBuilding) + .thenReturn(CocoonResponse()..data = false); + + buildState.startFetchingBuildStateUpdates(); + + await tester.pumpWidget( + MaterialApp( + home: ChangeNotifierProvider( + builder: (_) => buildState, + child: BuildDashboard(), + ), + ), + ); + + await tester.pump(const Duration(seconds: 1)); + + expect(find.text('Cocoon Backend is having issues'), findsOneWidget); + + buildState.dispose(); + }); + }); +} + +/// [FlutterBuildState] for giving errors. +class MockFlutterBuildState extends Mock implements FlutterBuildState {} From 3f8390a823971e546b85e9b502742e18707272d7 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Fri, 4 Oct 2019 15:42:19 -0700 Subject: [PATCH 09/17] it aint much but its honest work --- app_flutter/test/build_dashboard_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/app_flutter/test/build_dashboard_test.dart b/app_flutter/test/build_dashboard_test.dart index 3610b95a84..3f0cd8f154 100644 --- a/app_flutter/test/build_dashboard_test.dart +++ b/app_flutter/test/build_dashboard_test.dart @@ -36,6 +36,7 @@ void main() { ); await tester.pump(const Duration(seconds: 1)); + await tester.pump(); expect(find.text('Cocoon Backend is having issues'), findsOneWidget); From 442c6206c9bd37e4a16852744579bc2776797d4d Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 7 Oct 2019 10:58:04 -0700 Subject: [PATCH 10/17] made error message a variable --- app_flutter/lib/build_dashboard.dart | 5 ++++- app_flutter/test/build_dashboard_test.dart | 22 ++++++++++++---------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/app_flutter/lib/build_dashboard.dart b/app_flutter/lib/build_dashboard.dart index e0c2a14bf0..1e9ec11743 100644 --- a/app_flutter/lib/build_dashboard.dart +++ b/app_flutter/lib/build_dashboard.dart @@ -42,6 +42,9 @@ class _BuildDashboardPageState extends State { /// The tree's current build status is reflected in the color of [AppBar]. /// The results from tasks run on individual commits is shown in [StatusGrid]. class BuildDashboard extends StatelessWidget { + @visibleForTesting + static const String errorCocoonBackend = 'Cocoon Backend is having issues'; + @override Widget build(BuildContext context) { final GlobalKey _scaffoldKey = GlobalKey(); @@ -55,7 +58,7 @@ class BuildDashboard extends StatelessWidget { // TODO(chillers): Make this display better on the TV by using a bigger widget. const SnackBar snackbar = SnackBar( - content: Text('Cocoon Backend is having issues'), + content: Text(errorCocoonBackend), duration: Duration(seconds: 10), behavior: SnackBarBehavior.floating, ); diff --git a/app_flutter/test/build_dashboard_test.dart b/app_flutter/test/build_dashboard_test.dart index 3f0cd8f154..5cd9898bb9 100644 --- a/app_flutter/test/build_dashboard_test.dart +++ b/app_flutter/test/build_dashboard_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:app_flutter/service/fake_cocoon.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; @@ -16,13 +17,15 @@ void main() { group('BuildDashboard', () { testWidgets('should show error when FlutterBuildState has error', (WidgetTester tester) async { - final MockFlutterBuildState buildState = MockFlutterBuildState(); + final MockCocoonService errorService = MockCocoonService(); - when(buildState.hasError).thenReturn(true); - when(buildState.statuses).thenReturn( - CocoonResponse>()..error = 'status error'); - when(buildState.isTreeBuilding) - .thenReturn(CocoonResponse()..data = false); + when(errorService.fetchCommitStatuses()) + .thenAnswer((_) => Future>.error(0)); + when(errorService.fetchTreeBuildStatus()) + .thenAnswer((_) => Future.value(false)); + + final FlutterBuildState buildState = + FlutterBuildState(cocoonService: errorService); buildState.startFetchingBuildStateUpdates(); @@ -35,15 +38,14 @@ void main() { ), ); - await tester.pump(const Duration(seconds: 1)); await tester.pump(); - expect(find.text('Cocoon Backend is having issues'), findsOneWidget); + expect(find.text(BuildDashboard.errorCocoonBackend), findsOneWidget); buildState.dispose(); }); }); } -/// [FlutterBuildState] for giving errors. -class MockFlutterBuildState extends Mock implements FlutterBuildState {} +/// [FakeCocoonService] for giving errors. +class MockCocoonService extends Mock implements FakeCocoonService {} From ccecf0a11c5d5cab463d3d568d36edafb3ba8bb4 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 7 Oct 2019 11:27:29 -0700 Subject: [PATCH 11/17] moved flutterbuildstate error snackbar down a level --- app_flutter/lib/build_dashboard.dart | 19 -------- app_flutter/lib/status_grid.dart | 18 ++++++++ app_flutter/test/build_dashboard_test.dart | 51 ---------------------- app_flutter/test/status_grid_test.dart | 29 ++++++++++++ 4 files changed, 47 insertions(+), 70 deletions(-) delete mode 100644 app_flutter/test/build_dashboard_test.dart diff --git a/app_flutter/lib/build_dashboard.dart b/app_flutter/lib/build_dashboard.dart index 1e9ec11743..3eb8978098 100644 --- a/app_flutter/lib/build_dashboard.dart +++ b/app_flutter/lib/build_dashboard.dart @@ -42,31 +42,12 @@ class _BuildDashboardPageState extends State { /// The tree's current build status is reflected in the color of [AppBar]. /// The results from tasks run on individual commits is shown in [StatusGrid]. class BuildDashboard extends StatelessWidget { - @visibleForTesting - static const String errorCocoonBackend = 'Cocoon Backend is having issues'; - @override Widget build(BuildContext context) { - final GlobalKey _scaffoldKey = GlobalKey(); final ThemeData theme = Theme.of(context); return Consumer( builder: (_, FlutterBuildState buildState, Widget child) { - if (buildState.hasError) { - print('FlutterBuildState has an error'); - print('isTreeBuilding: ${buildState.isTreeBuilding.error}'); - print('statuses: ${buildState.statuses.error}'); - - // TODO(chillers): Make this display better on the TV by using a bigger widget. - const SnackBar snackbar = SnackBar( - content: Text(errorCocoonBackend), - duration: Duration(seconds: 10), - behavior: SnackBarBehavior.floating, - ); - _scaffoldKey.currentState.showSnackBar(snackbar); - } - return Scaffold( - key: _scaffoldKey, appBar: AppBar( title: const Text('Flutter Build Dashboard v2'), backgroundColor: buildState.isTreeBuilding.data diff --git a/app_flutter/lib/status_grid.dart b/app_flutter/lib/status_grid.dart index 0cc222cef5..a8e2f9e4ce 100644 --- a/app_flutter/lib/status_grid.dart +++ b/app_flutter/lib/status_grid.dart @@ -16,15 +16,33 @@ import 'task_icon.dart'; /// Container that manages the layout and data handling for [StatusGrid]. /// /// If there's no data for [StatusGrid], it shows [CircularProgressIndicator]. +/// If [FlutterBuildState] had an error, it shows [Snackbar] stating there are issues. class StatusGridContainer extends StatelessWidget { const StatusGridContainer({Key key}) : super(key: key); + @visibleForTesting + static const String errorCocoonBackend = 'Cocoon Backend is having issues'; + @override Widget build(BuildContext context) { return Consumer( builder: (_, FlutterBuildState buildState, Widget child) { final List statuses = buildState.statuses.data; + if (buildState.hasError) { + print('FlutterBuildState has an error'); + print('isTreeBuilding: ${buildState.isTreeBuilding.error}'); + print('statuses: ${buildState.statuses.error}'); + + // TODO(chillers): Make this display better on the TV by using a bigger widget. + const SnackBar snackbar = SnackBar( + content: Text(errorCocoonBackend), + duration: Duration(seconds: 10), + behavior: SnackBarBehavior.floating, + ); + Scaffold.of(context).showSnackBar(snackbar); + } + // Assume if there is no data that it is loading. if (statuses.isEmpty) { return const Expanded( diff --git a/app_flutter/test/build_dashboard_test.dart b/app_flutter/test/build_dashboard_test.dart deleted file mode 100644 index 5cd9898bb9..0000000000 --- a/app_flutter/test/build_dashboard_test.dart +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) 2019 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 'package:app_flutter/service/fake_cocoon.dart'; -import 'package:flutter/material.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:mockito/mockito.dart'; -import 'package:provider/provider.dart'; - -import 'package:cocoon_service/protos.dart' show CommitStatus; - -import 'package:app_flutter/build_dashboard.dart'; -import 'package:app_flutter/state/flutter_build.dart'; - -void main() { - group('BuildDashboard', () { - testWidgets('should show error when FlutterBuildState has error', - (WidgetTester tester) async { - final MockCocoonService errorService = MockCocoonService(); - - when(errorService.fetchCommitStatuses()) - .thenAnswer((_) => Future>.error(0)); - when(errorService.fetchTreeBuildStatus()) - .thenAnswer((_) => Future.value(false)); - - final FlutterBuildState buildState = - FlutterBuildState(cocoonService: errorService); - - buildState.startFetchingBuildStateUpdates(); - - await tester.pumpWidget( - MaterialApp( - home: ChangeNotifierProvider( - builder: (_) => buildState, - child: BuildDashboard(), - ), - ), - ); - - await tester.pump(); - - expect(find.text(BuildDashboard.errorCocoonBackend), findsOneWidget); - - buildState.dispose(); - }); - }); -} - -/// [FakeCocoonService] for giving errors. -class MockCocoonService extends Mock implements FakeCocoonService {} diff --git a/app_flutter/test/status_grid_test.dart b/app_flutter/test/status_grid_test.dart index 213052dc62..37b3232fc2 100644 --- a/app_flutter/test/status_grid_test.dart +++ b/app_flutter/test/status_grid_test.dart @@ -4,6 +4,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; import 'package:provider/provider.dart'; import 'package:cocoon_service/protos.dart' show CommitStatus, Task; @@ -104,5 +105,33 @@ void main() { expect(lastTaskWidget.task, lastTask); }); + + testWidgets('shows error message when FlutterBuildState has error', + (WidgetTester tester) async { + final MockFlutterBuildState buildState = MockFlutterBuildState(); + + when(buildState.hasError).thenReturn(true); + when(buildState.statuses) + .thenReturn(CocoonResponse>()..error = 'error!'); + when(buildState.isTreeBuilding) + .thenReturn(CocoonResponse()..data = false); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: ChangeNotifierProvider( + builder: (_) => buildState, + child: const StatusGridContainer(), + ), + ), + ), + ); + + await tester.pump(); + + expect(find.text(StatusGridContainer.errorCocoonBackend), findsOneWidget); + }); }); } + +class MockFlutterBuildState extends Mock implements FlutterBuildState {} From 63abd2837b71916d1406d997efe456729997d59d Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 7 Oct 2019 11:48:05 -0700 Subject: [PATCH 12/17] print appengine error responses --- app_flutter/lib/service/appengine_cocoon.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app_flutter/lib/service/appengine_cocoon.dart b/app_flutter/lib/service/appengine_cocoon.dart index f5d69c67c6..a3d4f4223d 100644 --- a/app_flutter/lib/service/appengine_cocoon.dart +++ b/app_flutter/lib/service/appengine_cocoon.dart @@ -37,6 +37,7 @@ class AppEngineCocoonService implements CocoonService { await _client.get('$_baseApiUrl/public/get-status'); if (response.statusCode != HttpStatus.ok) { + print(response); throw HttpException( '$_baseApiUrl/public/get-status returned ${response.statusCode}'); } @@ -53,6 +54,7 @@ class AppEngineCocoonService implements CocoonService { await _client.get('$_baseApiUrl/public/build-status'); if (response.statusCode != HttpStatus.ok) { + print(response); throw HttpException( '$_baseApiUrl/public/build-status returned ${response.statusCode}'); } From ee1ce537637c6ad453291506ebae829245c90166 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 7 Oct 2019 12:09:22 -0700 Subject: [PATCH 13/17] todo ui --- app_flutter/lib/status_grid.dart | 8 +------- app_flutter/test/status_grid_test.dart | 28 -------------------------- 2 files changed, 1 insertion(+), 35 deletions(-) diff --git a/app_flutter/lib/status_grid.dart b/app_flutter/lib/status_grid.dart index a8e2f9e4ce..03cbc0eead 100644 --- a/app_flutter/lib/status_grid.dart +++ b/app_flutter/lib/status_grid.dart @@ -34,13 +34,7 @@ class StatusGridContainer extends StatelessWidget { print('isTreeBuilding: ${buildState.isTreeBuilding.error}'); print('statuses: ${buildState.statuses.error}'); - // TODO(chillers): Make this display better on the TV by using a bigger widget. - const SnackBar snackbar = SnackBar( - content: Text(errorCocoonBackend), - duration: Duration(seconds: 10), - behavior: SnackBarBehavior.floating, - ); - Scaffold.of(context).showSnackBar(snackbar); + // TODO(chillers): Display the error } // Assume if there is no data that it is loading. diff --git a/app_flutter/test/status_grid_test.dart b/app_flutter/test/status_grid_test.dart index 37b3232fc2..16b0f6f175 100644 --- a/app_flutter/test/status_grid_test.dart +++ b/app_flutter/test/status_grid_test.dart @@ -105,33 +105,5 @@ void main() { expect(lastTaskWidget.task, lastTask); }); - - testWidgets('shows error message when FlutterBuildState has error', - (WidgetTester tester) async { - final MockFlutterBuildState buildState = MockFlutterBuildState(); - - when(buildState.hasError).thenReturn(true); - when(buildState.statuses) - .thenReturn(CocoonResponse>()..error = 'error!'); - when(buildState.isTreeBuilding) - .thenReturn(CocoonResponse()..data = false); - - await tester.pumpWidget( - MaterialApp( - home: Scaffold( - body: ChangeNotifierProvider( - builder: (_) => buildState, - child: const StatusGridContainer(), - ), - ), - ), - ); - - await tester.pump(); - - expect(find.text(StatusGridContainer.errorCocoonBackend), findsOneWidget); - }); }); } - -class MockFlutterBuildState extends Mock implements FlutterBuildState {} From 576a0407b60d926d73b7bb81bfb17443098b23d5 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 7 Oct 2019 12:10:56 -0700 Subject: [PATCH 14/17] revert --- app_flutter/lib/build_dashboard.dart | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/app_flutter/lib/build_dashboard.dart b/app_flutter/lib/build_dashboard.dart index 3eb8978098..0b038a325e 100644 --- a/app_flutter/lib/build_dashboard.dart +++ b/app_flutter/lib/build_dashboard.dart @@ -46,21 +46,19 @@ class BuildDashboard extends StatelessWidget { Widget build(BuildContext context) { final ThemeData theme = Theme.of(context); return Consumer( - builder: (_, FlutterBuildState buildState, Widget child) { - return Scaffold( - appBar: AppBar( - title: const Text('Flutter Build Dashboard v2'), - backgroundColor: buildState.isTreeBuilding.data - ? theme.primaryColor - : theme.errorColor, - ), - body: Column( - children: const [ - StatusGridContainer(), - ], - ), - ); - }, + builder: (_, FlutterBuildState buildState, Widget child) => Scaffold( + appBar: AppBar( + title: const Text('Flutter Build Dashboard v2'), + backgroundColor: buildState.isTreeBuilding.data + ? theme.primaryColor + : theme.errorColor, + ), + body: Column( + children: const [ + StatusGridContainer(), + ], + ), + ), ); } } From 9c883cedeae78b0e05df2e183d4c5687b9ad2ab9 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 7 Oct 2019 12:12:16 -0700 Subject: [PATCH 15/17] removed documentaiton on ui that was not implemented --- app_flutter/lib/status_grid.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/app_flutter/lib/status_grid.dart b/app_flutter/lib/status_grid.dart index 03cbc0eead..ab9c789d91 100644 --- a/app_flutter/lib/status_grid.dart +++ b/app_flutter/lib/status_grid.dart @@ -16,7 +16,6 @@ import 'task_icon.dart'; /// Container that manages the layout and data handling for [StatusGrid]. /// /// If there's no data for [StatusGrid], it shows [CircularProgressIndicator]. -/// If [FlutterBuildState] had an error, it shows [Snackbar] stating there are issues. class StatusGridContainer extends StatelessWidget { const StatusGridContainer({Key key}) : super(key: key); From 96cd4d16c02d3f9d06a0f1aaa2bdfe2f681f5716 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 7 Oct 2019 12:13:09 -0700 Subject: [PATCH 16/17] removed old dep --- app_flutter/test/status_grid_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/app_flutter/test/status_grid_test.dart b/app_flutter/test/status_grid_test.dart index 16b0f6f175..213052dc62 100644 --- a/app_flutter/test/status_grid_test.dart +++ b/app_flutter/test/status_grid_test.dart @@ -4,7 +4,6 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:mockito/mockito.dart'; import 'package:provider/provider.dart'; import 'package:cocoon_service/protos.dart' show CommitStatus, Task; From 4ad8da9115b8e72602fbd657d903f85bc91cd950 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Tue, 8 Oct 2019 06:59:57 -0700 Subject: [PATCH 17/17] moved cocoonresponse into cocoonservice --- app_flutter/lib/service/appengine_cocoon.dart | 43 +++++++++----- app_flutter/lib/service/cocoon.dart | 15 ++++- app_flutter/lib/service/fake_cocoon.dart | 9 +-- app_flutter/lib/state/flutter_build.dart | 33 +++-------- .../test/service/appengine_cocoon_test.dart | 58 ++++++++++++------- .../test/state/flutter_build_test.dart | 42 +++----------- app_flutter/test/status_grid_test.dart | 5 +- 7 files changed, 100 insertions(+), 105 deletions(-) diff --git a/app_flutter/lib/service/appengine_cocoon.dart b/app_flutter/lib/service/appengine_cocoon.dart index a3d4f4223d..7583918aeb 100644 --- a/app_flutter/lib/service/appengine_cocoon.dart +++ b/app_flutter/lib/service/appengine_cocoon.dart @@ -31,42 +31,55 @@ class AppEngineCocoonService implements CocoonService { final http.Client _client; @override - Future> fetchCommitStatuses() async { + Future>> fetchCommitStatuses() async { /// This endpoint returns JSON [List, List] final http.Response response = await _client.get('$_baseApiUrl/public/get-status'); if (response.statusCode != HttpStatus.ok) { - print(response); - throw HttpException( - '$_baseApiUrl/public/get-status returned ${response.statusCode}'); + print(response.body); + return CocoonResponse>() + ..error = + '$_baseApiUrl/public/get-status returned ${response.statusCode}'; } - final Map jsonResponse = jsonDecode(response.body); - - return _commitStatusesFromJson(jsonResponse['Statuses']); + try { + final Map jsonResponse = jsonDecode(response.body); + return CocoonResponse>() + ..data = _commitStatusesFromJson(jsonResponse['Statuses']); + } catch (error) { + return CocoonResponse>()..error = error.toString(); + } } @override - Future fetchTreeBuildStatus() async { + Future> fetchTreeBuildStatus() async { /// This endpoint returns JSON {AnticipatedBuildStatus: [BuildStatus]} final http.Response response = await _client.get('$_baseApiUrl/public/build-status'); if (response.statusCode != HttpStatus.ok) { - print(response); - throw HttpException( - '$_baseApiUrl/public/build-status returned ${response.statusCode}'); + print(response.body); + return CocoonResponse() + ..error = + '$_baseApiUrl/public/build-status returned ${response.statusCode}'; } - final Map jsonResponse = jsonDecode(response.body); + Map jsonResponse; + try { + jsonResponse = jsonDecode(response.body); + } catch (error) { + return CocoonResponse() + ..error = '$_baseApiUrl/public/build-status had a malformed response'; + } if (!_isBuildStatusResponseValid(jsonResponse)) { - throw const HttpException( - '$_baseApiUrl/public/build-status had a malformed response'); + return CocoonResponse() + ..error = '$_baseApiUrl/public/build-status had a malformed response'; } - return jsonResponse['AnticipatedBuildStatus'] == 'Succeeded'; + return CocoonResponse() + ..data = jsonResponse['AnticipatedBuildStatus'] == 'Succeeded'; } /// Check if [Map] follows the format for build-status. diff --git a/app_flutter/lib/service/cocoon.dart b/app_flutter/lib/service/cocoon.dart index c11e8debc8..7ca9523c23 100644 --- a/app_flutter/lib/service/cocoon.dart +++ b/app_flutter/lib/service/cocoon.dart @@ -30,8 +30,19 @@ abstract class CocoonService { /// Gets build information from the last 200 commits. /// // TODO(chillers): Make configurable to get range of commits, https://github.com/flutter/cocoon/issues/458 - Future> fetchCommitStatuses(); + Future>> fetchCommitStatuses(); /// Gets the current build status of flutter/flutter. - Future fetchTreeBuildStatus(); + Future> fetchTreeBuildStatus(); +} + +/// Wrapper class for data this state serves. +/// +/// Holds [data] and possible error information. +class CocoonResponse { + /// The data that gets used from [CocoonService]. + T data; + + /// Error information that can be used for debugging. + String error; } diff --git a/app_flutter/lib/service/fake_cocoon.dart b/app_flutter/lib/service/fake_cocoon.dart index 7f2e9d68cd..474bd84dea 100644 --- a/app_flutter/lib/service/fake_cocoon.dart +++ b/app_flutter/lib/service/fake_cocoon.dart @@ -20,13 +20,14 @@ class FakeCocoonService implements CocoonService { final Random random; @override - Future> fetchCommitStatuses() async { - return _createFakeCommitStatuses(); + Future>> fetchCommitStatuses() async { + return CocoonResponse>() + ..data = _createFakeCommitStatuses(); } @override - Future fetchTreeBuildStatus() async { - return random.nextBool(); + Future> fetchTreeBuildStatus() async { + return CocoonResponse()..data = random.nextBool(); } List _createFakeCommitStatuses() { diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index ae31a57705..7d29843a23 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -30,13 +30,12 @@ class FlutterBuildState extends ChangeNotifier { Timer refreshTimer; /// The current status of the commits loaded. - final CocoonResponse> _statuses = + CocoonResponse> _statuses = CocoonResponse>()..data = []; CocoonResponse> get statuses => _statuses; /// Whether or not flutter/flutter currently passes tests. - final CocoonResponse _isTreeBuilding = CocoonResponse() - ..data = false; + CocoonResponse _isTreeBuilding = CocoonResponse()..data = false; CocoonResponse get isTreeBuilding => _isTreeBuilding; /// Whether an error occured getting the latest data for the fields of this state. @@ -60,18 +59,11 @@ class FlutterBuildState extends ChangeNotifier { Future _fetchBuildStatusUpdate() async { await Future.wait(>[ _cocoonService.fetchCommitStatuses().then( - (List commitStatuses) { - _statuses.data = commitStatuses; - _statuses.error = null; - }, onError: (dynamic error) { - _statuses.error = error.toString(); - }), - _cocoonService.fetchTreeBuildStatus().then((bool treeStatus) { - _isTreeBuilding.data = treeStatus; - _isTreeBuilding.error = null; - }, onError: (dynamic error) { - _isTreeBuilding.error = error.toString(); - }), + (CocoonResponse> response) => + _statuses = response), + _cocoonService + .fetchTreeBuildStatus() + .then((CocoonResponse response) => _isTreeBuilding = response), ]); notifyListeners(); @@ -83,14 +75,3 @@ class FlutterBuildState extends ChangeNotifier { super.dispose(); } } - -/// Wrapper class for data this state serves. -/// -/// Holds [data] and possible error information. -class CocoonResponse { - /// The data that gets used from [CocoonService]. - T data; - - /// Error information that can be used for debugging. - String error; -} diff --git a/app_flutter/test/service/appengine_cocoon_test.dart b/app_flutter/test/service/appengine_cocoon_test.dart index de6e409d53..79c028a5ef 100644 --- a/app_flutter/test/service/appengine_cocoon_test.dart +++ b/app_flutter/test/service/appengine_cocoon_test.dart @@ -7,10 +7,12 @@ import 'package:http/http.dart' show Request, Response; import 'package:http/testing.dart'; import 'package:test/test.dart'; -import 'package:app_flutter/service/appengine_cocoon.dart'; import 'package:cocoon_service/protos.dart' show Commit, CommitStatus, Stage, Task; +import 'package:app_flutter/service/appengine_cocoon.dart'; +import 'package:app_flutter/service/cocoon.dart'; + // This is based off data the Cocoon backend sends out from v1. // It doesn't map directly to protos since the backend does // not use protos yet. @@ -86,13 +88,14 @@ void main() { })); }); - test('should return List', () { + test('should return CocoonResponse>', () { expect(service.fetchCommitStatuses(), - const TypeMatcher>>()); + const TypeMatcher>>>()); }); test('should return expected List', () async { - final List statuses = await service.fetchCommitStatuses(); + final CocoonResponse> statuses = + await service.fetchCommitStatuses(); final CommitStatus expectedStatus = CommitStatus() ..commit = (Commit() @@ -118,22 +121,26 @@ void main() { ..stageName = 'devicelab' ..status = 'Succeeded')); - expect(statuses.length, 1); - expect(statuses.first, expectedStatus); + expect(statuses.data.length, 1); + expect(statuses.data.first, expectedStatus); }); - test('should throw exception if given non-200 response', () { + test('should have error if given non-200 response', () async { service = AppEngineCocoonService( client: MockClient((Request request) async => Response('', 404))); - expect(service.fetchCommitStatuses(), throwsException); + final CocoonResponse> response = + await service.fetchCommitStatuses(); + expect(response.error, isNotNull); }); - test('should throw exception if given bad response', () { + test('should have error if given bad response', () async { service = AppEngineCocoonService( client: MockClient((Request request) async => Response('bad', 200))); - expect(service.fetchCommitStatuses(), throwsException); + final CocoonResponse> response = + await service.fetchCommitStatuses(); + expect(response.error, isNotNull); }); }); @@ -147,39 +154,46 @@ void main() { })); }); - test('should return bool', () { - expect(service.fetchTreeBuildStatus(), const TypeMatcher>()); + test('should return CocoonResponse', () { + expect(service.fetchTreeBuildStatus(), + const TypeMatcher>>()); }); - test('should return true when given Succeeded', () async { - final bool treeBuildStatus = await service.fetchTreeBuildStatus(); + test('data should be true when given Succeeded', () async { + final CocoonResponse treeBuildStatus = + await service.fetchTreeBuildStatus(); - expect(treeBuildStatus, true); + expect(treeBuildStatus.data, true); }); - test('should return false when given Failed', () async { + test('data should be false when given Failed', () async { service = AppEngineCocoonService(client: MockClient((Request request) async { return Response(jsonBuildStatusFalseResponse, 200); })); - final bool treeBuildStatus = await service.fetchTreeBuildStatus(); + final CocoonResponse treeBuildStatus = + await service.fetchTreeBuildStatus(); - expect(treeBuildStatus, false); + expect(treeBuildStatus.data, false); }); - test('should throw exception if given non-200 response', () { + test('should have error if given non-200 response', () async { service = AppEngineCocoonService( client: MockClient((Request request) async => Response('', 404))); - expect(service.fetchTreeBuildStatus(), throwsException); + final CocoonResponse response = + await service.fetchTreeBuildStatus(); + expect(response.error, isNotNull); }); - test('should throw exception if given bad response', () { + test('should have error if given bad response', () async { service = AppEngineCocoonService( client: MockClient((Request request) async => Response('bad', 200))); - expect(service.fetchTreeBuildStatus(), throwsException); + final CocoonResponse response = + await service.fetchTreeBuildStatus(); + expect(response.error, isNotNull); }); }); } diff --git a/app_flutter/test/state/flutter_build_test.dart b/app_flutter/test/state/flutter_build_test.dart index 6b72bd9a82..dfa2b4fe10 100644 --- a/app_flutter/test/state/flutter_build_test.dart +++ b/app_flutter/test/state/flutter_build_test.dart @@ -9,6 +9,7 @@ import 'package:mockito/mockito.dart'; import 'package:cocoon_service/protos.dart' show CommitStatus; +import 'package:app_flutter/service/cocoon.dart'; import 'package:app_flutter/service/fake_cocoon.dart'; import 'package:app_flutter/state/flutter_build.dart'; @@ -21,10 +22,12 @@ void main() { mockService = MockCocoonService(); buildState = FlutterBuildState(cocoonService: mockService); - when(mockService.fetchCommitStatuses()).thenAnswer( - (_) => Future>.value([])); - when(mockService.fetchTreeBuildStatus()) - .thenAnswer((_) => Future.value(true)); + when(mockService.fetchCommitStatuses()).thenAnswer((_) => + Future>>.value( + CocoonResponse>()..data = [])); + when(mockService.fetchTreeBuildStatus()).thenAnswer((_) => + Future>.value( + CocoonResponse()..data = true)); }); testWidgets('timer should periodically fetch updates', @@ -60,37 +63,6 @@ void main() { // Tear down fails to cancel the timer before the test is over buildState.dispose(); }); - - testWidgets('error when fetching statuses should be set in CocoonResponse', - (WidgetTester tester) async { - when(mockService.fetchCommitStatuses()) - .thenAnswer((_) => Future>.error(42)); - - buildState.startFetchingBuildStateUpdates(); - - await tester.pumpAndSettle(); - - expect(buildState.hasError, true); - expect(buildState.statuses.error, isNotNull); - - buildState.dispose(); - }); - - testWidgets( - 'error when fetching tree build status should be set in CocoonResponse', - (WidgetTester tester) async { - when(mockService.fetchTreeBuildStatus()) - .thenAnswer((_) => Future.error(42)); - - buildState.startFetchingBuildStateUpdates(); - - await tester.pumpAndSettle(); - - expect(buildState.hasError, true); - expect(buildState.isTreeBuilding.error, isNotNull); - - buildState.dispose(); - }); }); } diff --git a/app_flutter/test/status_grid_test.dart b/app_flutter/test/status_grid_test.dart index 213052dc62..c65bd26aa3 100644 --- a/app_flutter/test/status_grid_test.dart +++ b/app_flutter/test/status_grid_test.dart @@ -8,6 +8,7 @@ import 'package:provider/provider.dart'; import 'package:cocoon_service/protos.dart' show CommitStatus, Task; +import 'package:app_flutter/service/cocoon.dart'; import 'package:app_flutter/service/fake_cocoon.dart'; import 'package:app_flutter/state/flutter_build.dart'; import 'package:app_flutter/commit_box.dart'; @@ -20,7 +21,9 @@ void main() { setUpAll(() async { final FakeCocoonService service = FakeCocoonService(); - statuses = await service.fetchCommitStatuses(); + final CocoonResponse> response = + await service.fetchCommitStatuses(); + statuses = response.data; }); testWidgets('shows loading indicator when statuses is empty',