From 6504cf503019f500de8e34a9354659377443cd9a Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 30 Sep 2019 15:59:46 -0700 Subject: [PATCH 1/8] fetch tree build status in CocoonService --- app_flutter/lib/service/appengine_cocoon.dart | 41 ++++++++++++++++++ app_flutter/lib/service/cocoon.dart | 3 ++ app_flutter/lib/service/fake_cocoon.dart | 5 +++ app_flutter/lib/state/flutter_build.dart | 3 ++ .../test/service/appengine_cocoon_test.dart | 42 ++++++++++++++++++- 5 files changed, 93 insertions(+), 1 deletion(-) diff --git a/app_flutter/lib/service/appengine_cocoon.dart b/app_flutter/lib/service/appengine_cocoon.dart index 138d08bfcd..d14fb09e54 100644 --- a/app_flutter/lib/service/appengine_cocoon.dart +++ b/app_flutter/lib/service/appengine_cocoon.dart @@ -46,6 +46,47 @@ class AppEngineCocoonService implements CocoonService { return _commitStatusesFromJson(jsonResponse['Statuses']); } + @override + Future fetchTreeBuildStatus() async { + /// This endpoint returns JSON {AnticipatedBuildStatus: [BuildStatus]} + http.Response response = + await _client.get('$_baseApiUrl/public/build-status'); + + if (response.statusCode != HttpStatus.ok) { + throw HttpException( + '$_baseApiUrl/public/build-status returned ${response.statusCode}'); + } + + Map jsonResponse = jsonDecode(response.body); + + if (!_isBuildStatusResponseValid(jsonResponse)) { + throw HttpException( + '$_baseApiUrl/public/build-status had a malformed response'); + } + + return jsonResponse['AnticipatedBuildStatus'] == "Succeeded"; + } + + /// Check if [Map] follows the format for build-status. + /// + /// ```json + /// { + /// "AnticipatedBuildStatus": "Succeeded"|"Failed" + /// } + /// ``` + bool _isBuildStatusResponseValid(Map response) { + if (!response.containsKey('AnticipatedBuildStatus')) { + return false; + } + + String treeBuildStatus = response['AnticipatedBuildStatus']; + if (treeBuildStatus != 'Failed' && treeBuildStatus != 'Succeeded') { + return false; + } + + return true; + } + List _commitStatusesFromJson(List jsonCommitStatuses) { assert(jsonCommitStatuses != null); // TODO(chillers): Remove adapter code to just use proto fromJson method. https://github.com/flutter/cocoon/issues/441 diff --git a/app_flutter/lib/service/cocoon.dart b/app_flutter/lib/service/cocoon.dart index 3f9df8aecb..872e8872e1 100644 --- a/app_flutter/lib/service/cocoon.dart +++ b/app_flutter/lib/service/cocoon.dart @@ -31,4 +31,7 @@ abstract class CocoonService { /// /// TODO(chillers): Make configurable to get range of commits Future> fetchCommitStatuses(); + + /// Gets the current build status of flutter/flutter. + Future fetchTreeBuildStatus(); } diff --git a/app_flutter/lib/service/fake_cocoon.dart b/app_flutter/lib/service/fake_cocoon.dart index 4e2fb67243..6ce77a2273 100644 --- a/app_flutter/lib/service/fake_cocoon.dart +++ b/app_flutter/lib/service/fake_cocoon.dart @@ -18,6 +18,11 @@ class FakeCocoonService implements CocoonService { return Future.value(_createFakeCommitStatuses()); } + @override + Future fetchTreeBuildStatus() async { + return true; + } + List _createFakeCommitStatuses() { List stats = []; diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index c663a66947..3540387368 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -26,6 +26,9 @@ class FlutterBuildState extends ChangeNotifier { /// The current status of the commits loaded. List statuses = []; + /// Whether or not flutter/flutter currently passes tests. + bool isTreeBuilding; + /// Creates a new [FlutterBuildState]. /// /// If [CocoonService] is not specified, a new [CocoonService] instance is created. diff --git a/app_flutter/test/service/appengine_cocoon_test.dart b/app_flutter/test/service/appengine_cocoon_test.dart index 2811d53e58..78eae68164 100644 --- a/app_flutter/test/service/appengine_cocoon_test.dart +++ b/app_flutter/test/service/appengine_cocoon_test.dart @@ -63,8 +63,14 @@ const String jsonGetStatsResponse = """ } """; +const String jsonBuildStatusResponse = """ + { + "AnticipatedBuildStatus": "Succeeded" + } +"""; + void main() { - group('AppEngine CocoonService', () { + group('AppEngine CocoonService fetchCommitStatus ', () { AppEngineCocoonService service; setUp(() async { @@ -123,4 +129,38 @@ void main() { expect(service.fetchCommitStatuses(), throwsException); }); }); + + group('AppEngine CocoonService fetchTreeBuildStatus ', () { + AppEngineCocoonService service; + + setUp(() async { + service = AppEngineCocoonService(client: MockClient((request) async { + return Response(jsonBuildStatusResponse, 200); + })); + }); + + test('should return bool', () { + expect(service.fetchTreeBuildStatus(), TypeMatcher>()); + }); + + test('should return expected bool', () async { + bool treeBuildStatus = await service.fetchTreeBuildStatus(); + + expect(treeBuildStatus, true); + }); + + test('should throw exception if given non-200 response', () { + service = AppEngineCocoonService( + client: MockClient((request) async => Response('', 404))); + + expect(service.fetchTreeBuildStatus(), throwsException); + }); + + test('should throw exception if given bad response', () { + service = AppEngineCocoonService( + client: MockClient((request) async => Response('bad', 200))); + + expect(service.fetchTreeBuildStatus(), throwsException); + }); + }); } From 887de76657d26c1b741dca71c3ca2ec189ad1547 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 30 Sep 2019 16:02:49 -0700 Subject: [PATCH 2/8] added explainer for the json --- app_flutter/lib/service/appengine_cocoon.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/app_flutter/lib/service/appengine_cocoon.dart b/app_flutter/lib/service/appengine_cocoon.dart index d14fb09e54..4fec3d4705 100644 --- a/app_flutter/lib/service/appengine_cocoon.dart +++ b/app_flutter/lib/service/appengine_cocoon.dart @@ -69,6 +69,7 @@ class AppEngineCocoonService implements CocoonService { /// Check if [Map] follows the format for build-status. /// + /// Template of the response it should receive: /// ```json /// { /// "AnticipatedBuildStatus": "Succeeded"|"Failed" From 8ac0814bfdf5c755557b69e058842b91da617f91 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 30 Sep 2019 16:28:16 -0700 Subject: [PATCH 3/8] change the appbar color based on the state --- app_flutter/lib/main.dart | 3 +++ app_flutter/lib/state/flutter_build.dart | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app_flutter/lib/main.dart b/app_flutter/lib/main.dart index 56c04ecc35..f83982a877 100644 --- a/app_flutter/lib/main.dart +++ b/app_flutter/lib/main.dart @@ -43,6 +43,9 @@ class _BuildDashboardPageState extends State { return Scaffold( appBar: AppBar( title: Text('Flutter Build Dashboard v2'), + backgroundColor: buildState.isTreeBuilding + ? Theme.of(context).primaryColor + : Theme.of(context).errorColor, ), body: Column( children: [ diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index 3540387368..129f5cfce9 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -27,7 +27,7 @@ class FlutterBuildState extends ChangeNotifier { List statuses = []; /// Whether or not flutter/flutter currently passes tests. - bool isTreeBuilding; + bool isTreeBuilding = true; /// Creates a new [FlutterBuildState]. /// @@ -49,6 +49,8 @@ class FlutterBuildState extends ChangeNotifier { /// Request the latest [statuses] from [CocoonService]. void _fetchBuildStatusUpdate() async { statuses = await _cocoonService.fetchCommitStatuses(); + isTreeBuilding = await _cocoonService.fetchTreeBuildStatus(); + notifyListeners(); } From 8a2ec4944218d066dfa311dba9f744e635ad96b5 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 30 Sep 2019 18:09:06 -0700 Subject: [PATCH 4/8] theme as a final var --- app_flutter/lib/main.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app_flutter/lib/main.dart b/app_flutter/lib/main.dart index f83982a877..a6ac6dd8c5 100644 --- a/app_flutter/lib/main.dart +++ b/app_flutter/lib/main.dart @@ -40,12 +40,12 @@ class _BuildDashboardPageState extends State { @override Widget build(BuildContext context) { + final ThemeData theme = Theme.of(context); return Scaffold( appBar: AppBar( title: Text('Flutter Build Dashboard v2'), - backgroundColor: buildState.isTreeBuilding - ? Theme.of(context).primaryColor - : Theme.of(context).errorColor, + backgroundColor: + buildState.isTreeBuilding ? theme.primaryColor : theme.errorColor, ), body: Column( children: [ From b6d3d2e4837a398abc8e61b8f5b7bf0d0b841dce Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Mon, 30 Sep 2019 18:11:12 -0700 Subject: [PATCH 5/8] getter for tree status --- app_flutter/lib/state/flutter_build.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index 129f5cfce9..6a5de3446b 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -27,7 +27,8 @@ class FlutterBuildState extends ChangeNotifier { List statuses = []; /// Whether or not flutter/flutter currently passes tests. - bool isTreeBuilding = true; + bool get isTreeBuilding => _isTreeBuilding; + bool _isTreeBuilding = true; /// Creates a new [FlutterBuildState]. /// @@ -49,7 +50,7 @@ class FlutterBuildState extends ChangeNotifier { /// Request the latest [statuses] from [CocoonService]. void _fetchBuildStatusUpdate() async { statuses = await _cocoonService.fetchCommitStatuses(); - isTreeBuilding = await _cocoonService.fetchTreeBuildStatus(); + _isTreeBuilding = await _cocoonService.fetchTreeBuildStatus(); notifyListeners(); } From 62dad0941e598c67adf53243b426bc4da601ffaf Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Tue, 1 Oct 2019 07:22:48 -0700 Subject: [PATCH 6/8] Future.wait --- app_flutter/lib/state/flutter_build.dart | 13 +++++++++-- .../test/state/flutter_build_test.dart | 22 ++++++++++++++----- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/app_flutter/lib/state/flutter_build.dart b/app_flutter/lib/state/flutter_build.dart index 6a5de3446b..b302474ca8 100644 --- a/app_flutter/lib/state/flutter_build.dart +++ b/app_flutter/lib/state/flutter_build.dart @@ -43,14 +43,23 @@ class FlutterBuildState extends ChangeNotifier { return; } + /// [Timer.periodic] does not necessarily run at the start of the timer. + _fetchBuildStatusUpdate(); + refreshTimer = Timer.periodic(refreshRate, (t) => _fetchBuildStatusUpdate()); } /// Request the latest [statuses] from [CocoonService]. void _fetchBuildStatusUpdate() async { - statuses = await _cocoonService.fetchCommitStatuses(); - _isTreeBuilding = await _cocoonService.fetchTreeBuildStatus(); + await Future.wait([ + _cocoonService + .fetchCommitStatuses() + .then((commitStatuses) => statuses = commitStatuses), + _cocoonService + .fetchTreeBuildStatus() + .then((treeStatus) => _isTreeBuilding = treeStatus), + ]); notifyListeners(); } diff --git a/app_flutter/test/state/flutter_build_test.dart b/app_flutter/test/state/flutter_build_test.dart index 9599b43445..90b172255d 100644 --- a/app_flutter/test/state/flutter_build_test.dart +++ b/app_flutter/test/state/flutter_build_test.dart @@ -7,6 +7,8 @@ import 'dart:async'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; +import 'package:cocoon_service/protos.dart' show CommitStatus; + import 'package:app_flutter/service/fake_cocoon.dart'; import 'package:app_flutter/state/flutter_build.dart'; @@ -18,27 +20,31 @@ void main() { setUp(() { mockService = MockCocoonService(); buildState = FlutterBuildState(cocoonService: mockService); + + when(mockService.fetchCommitStatuses()) + .thenAnswer((_) => Future.value([])); + when(mockService.fetchTreeBuildStatus()) + .thenAnswer((_) => Future.value(true)); }); testWidgets('timer should periodically fetch updates', (WidgetTester tester) async { buildState.startFetchingBuildStateUpdates(); - verifyZeroInteractions(mockService); + + // startFetching immediately starts fetching results + verify(mockService.fetchCommitStatuses()).called(1); // Periodic timers don't necessarily run at the same time in each interval. // We double the refreshRate to gurantee a call would have been made. await tester.pump(buildState.refreshRate * 2); - verify(mockService.fetchCommitStatuses()).called(greaterThan(0)); - - await tester.pump(buildState.refreshRate * 2); - // Now we check to see if another call had been made verify(mockService.fetchCommitStatuses()).called(greaterThan(1)); // Tear down fails to cancel the timer before the test is over buildState.dispose(); }); - test('multiple start updates should not change the timer', () { + testWidgets('multiple start updates should not change the timer', + (WidgetTester tester) async { buildState.startFetchingBuildStateUpdates(); Timer refreshTimer = buildState.refreshTimer; @@ -47,6 +53,10 @@ void main() { expect(refreshTimer, equals(buildState.refreshTimer)); + // Since startFetching sends out requests on start, we need to wait + // for them to finish before disposing of the state. + await tester.pumpAndSettle(); + // Tear down fails to cancel the timer before the test is over buildState.dispose(); }); From 518a5b9d68f0b0c9a61bbf4866d25e1f0248556e Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Tue, 1 Oct 2019 07:23:36 -0700 Subject: [PATCH 7/8] removed unused dep --- app_flutter/test/state/flutter_build_test.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/app_flutter/test/state/flutter_build_test.dart b/app_flutter/test/state/flutter_build_test.dart index 90b172255d..f7c13e3926 100644 --- a/app_flutter/test/state/flutter_build_test.dart +++ b/app_flutter/test/state/flutter_build_test.dart @@ -7,8 +7,6 @@ import 'dart:async'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; -import 'package:cocoon_service/protos.dart' show CommitStatus; - import 'package:app_flutter/service/fake_cocoon.dart'; import 'package:app_flutter/state/flutter_build.dart'; From ab3cc801e8fd0108028d5304de9103d1fec07f87 Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Tue, 1 Oct 2019 09:53:54 -0700 Subject: [PATCH 8/8] added test for Failed --- .../test/service/appengine_cocoon_test.dart | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/app_flutter/test/service/appengine_cocoon_test.dart b/app_flutter/test/service/appengine_cocoon_test.dart index 78eae68164..2582c34528 100644 --- a/app_flutter/test/service/appengine_cocoon_test.dart +++ b/app_flutter/test/service/appengine_cocoon_test.dart @@ -63,12 +63,18 @@ const String jsonGetStatsResponse = """ } """; -const String jsonBuildStatusResponse = """ +const String jsonBuildStatusTrueResponse = """ { "AnticipatedBuildStatus": "Succeeded" } """; +const String jsonBuildStatusFalseResponse = """ + { + "AnticipatedBuildStatus": "Failed" + } +"""; + void main() { group('AppEngine CocoonService fetchCommitStatus ', () { AppEngineCocoonService service; @@ -135,7 +141,7 @@ void main() { setUp(() async { service = AppEngineCocoonService(client: MockClient((request) async { - return Response(jsonBuildStatusResponse, 200); + return Response(jsonBuildStatusTrueResponse, 200); })); }); @@ -143,12 +149,22 @@ void main() { expect(service.fetchTreeBuildStatus(), TypeMatcher>()); }); - test('should return expected bool', () async { + test('should return true when given Succeeded', () async { bool treeBuildStatus = await service.fetchTreeBuildStatus(); expect(treeBuildStatus, true); }); + test('should return false when given Failed', () async { + service = AppEngineCocoonService(client: MockClient((request) async { + return Response(jsonBuildStatusFalseResponse, 200); + })); + + bool treeBuildStatus = await service.fetchTreeBuildStatus(); + + expect(treeBuildStatus, false); + }); + test('should throw exception if given non-200 response', () { service = AppEngineCocoonService( client: MockClient((request) async => Response('', 404)));