From 4e3f0747291c0b2786b46922b15ef55ad82211c6 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 14 May 2025 08:53:10 -0700 Subject: [PATCH 1/3] Allow vacuuming a specific repository and branch. --- .../vacuum_github_commits.dart | 28 +++++++++++++---- .../vacuum_github_commits_test.dart | 30 +++++++++++++++---- dashboard/lib/service/appengine_cocoon.dart | 11 +++++-- dashboard/lib/service/cocoon.dart | 6 +++- dashboard/lib/service/dev_cocoon.dart | 6 +++- dashboard/lib/state/build.dart | 2 ++ .../test/service/appengine_cocoon_test.dart | 12 ++++++-- 7 files changed, 78 insertions(+), 17 deletions(-) diff --git a/app_dart/lib/src/request_handlers/vacuum_github_commits.dart b/app_dart/lib/src/request_handlers/vacuum_github_commits.dart index 721bc4af9..c6c911f81 100644 --- a/app_dart/lib/src/request_handlers/vacuum_github_commits.dart +++ b/app_dart/lib/src/request_handlers/vacuum_github_commits.dart @@ -26,15 +26,31 @@ final class VacuumGithubCommits extends ApiRequestHandler { }) : _scheduler = scheduler; final Scheduler _scheduler; - static const String branchParam = 'branch'; + + static const _paramRepo = 'repo'; + static const _paramBranch = 'branch'; @override Future get(Request request) async { - for (var slug in config.supportedRepos) { - final branch = - request.uri.queryParameters[branchParam] ?? - Config.defaultBranch(slug); - await _vacuumRepository(slug, branch: branch); + final Iterable repos; + if (request.uri.queryParameters[_paramRepo] case final specific?) { + repos = [gh.RepositorySlug.full(specific)]; + } else { + repos = config.supportedRepos; + } + + final String? branch; + if (request.uri.queryParameters[_paramBranch] case final specific?) { + branch = specific; + } else { + branch = null; + } + + for (final slug in repos) { + await _vacuumRepository( + slug, + branch: branch ?? Config.defaultBranch(slug), + ); } return Response.emptyOk; diff --git a/app_dart/test/request_handlers/vacuum_github_commits_test.dart b/app_dart/test/request_handlers/vacuum_github_commits_test.dart index 3acd82930..2a9383954 100644 --- a/app_dart/test/request_handlers/vacuum_github_commits_test.dart +++ b/app_dart/test/request_handlers/vacuum_github_commits_test.dart @@ -97,23 +97,43 @@ void main() { }); test('succeeds when GitHub returns no commits', () async { - fakeGithubCommitShas = []; - config.supportedBranchesValue = ['master']; + fakeGithubCommitShas = []; + config.supportedBranchesValue = ['master']; final body = await tester.get(handler); expect(firestore, existsInStorage(fs.Commit.metadata, isEmpty)); expect(await body.body.toList(), isEmpty); }); + test('succeeds on specific repo and branch set', () async { + fakeGithubCommitShas = ['123']; + config.supportedReposValue = {RepositorySlug('flutter', 'cocoon')}; + + tester.request.uri = tester.request.uri.replace( + queryParameters: {'repo': 'flutter/cocoon', 'branch': 'a-new-branch'}, + ); + + await tester.get(handler); + expect( + firestore, + existsInStorage(fs.Commit.metadata, [ + isCommit + .hasSha('123') + .hasRepositoryPath('flutter/cocoon') + .hasBranch('a-new-branch'), + ]), + ); + }); + test('does not fail on empty commit list', () async { - fakeGithubCommitShas = []; + fakeGithubCommitShas = []; expect(firestore, existsInStorage(fs.Commit.metadata, isEmpty)); await tester.get(handler); expect(firestore, existsInStorage(fs.Commit.metadata, isEmpty)); }); test('does not add recent commits', () async { - fakeGithubCommitShas = ['${DateTime.now().millisecondsSinceEpoch}']; + fakeGithubCommitShas = ['${DateTime.now().millisecondsSinceEpoch}']; expect(firestore, existsInStorage(fs.Commit.metadata, isEmpty)); await tester.get(handler); @@ -121,7 +141,7 @@ void main() { }); test('inserts all relevant fields of the commit', () async { - fakeGithubCommitShas = ['1']; + fakeGithubCommitShas = ['1']; expect(firestore, existsInStorage(fs.Commit.metadata, isEmpty)); await tester.get(handler); expect( diff --git a/dashboard/lib/service/appengine_cocoon.dart b/dashboard/lib/service/appengine_cocoon.dart index cfb2358ff..e16d54746 100644 --- a/dashboard/lib/service/appengine_cocoon.dart +++ b/dashboard/lib/service/appengine_cocoon.dart @@ -189,8 +189,15 @@ class AppEngineCocoonService implements CocoonService { } @override - Future> vacuumGitHubCommits(String idToken) async { - final refreshGitHubCommitsUrl = apiEndpoint('/api/vacuum-github-commits'); + Future> vacuumGitHubCommits( + String idToken, { + required String repo, + required String branch, + }) async { + final refreshGitHubCommitsUrl = apiEndpoint( + '/api/vacuum-github-commits', + queryParameters: {'repo': repo, 'branch': branch}, + ); final response = await _client.get( refreshGitHubCommitsUrl, headers: {'X-Flutter-IdToken': idToken}, diff --git a/dashboard/lib/service/cocoon.dart b/dashboard/lib/service/cocoon.dart index 6ae1786cf..78c151b44 100644 --- a/dashboard/lib/service/cocoon.dart +++ b/dashboard/lib/service/cocoon.dart @@ -70,7 +70,11 @@ abstract class CocoonService { }); /// Force update Cocoon to get the latest commits. - Future> vacuumGitHubCommits(String idToken); + Future> vacuumGitHubCommits( + String idToken, { + required String repo, + required String branch, + }); } /// Wrapper class for data this state serves. diff --git a/dashboard/lib/service/dev_cocoon.dart b/dashboard/lib/service/dev_cocoon.dart index caa694af6..43bfd3ad4 100644 --- a/dashboard/lib/service/dev_cocoon.dart +++ b/dashboard/lib/service/dev_cocoon.dart @@ -126,7 +126,11 @@ class DevelopmentCocoonService implements CocoonService { } @override - Future> vacuumGitHubCommits(String idToken) async { + Future> vacuumGitHubCommits( + String idToken, { + required String repo, + required String branch, + }) async { return const CocoonResponse.error( 'Unable to vacuum against fake data. Try building the app to use prod data.', statusCode: 501 /* Not implemented */, diff --git a/dashboard/lib/state/build.dart b/dashboard/lib/state/build.dart index fb95e9707..e28211d41 100644 --- a/dashboard/lib/state/build.dart +++ b/dashboard/lib/state/build.dart @@ -360,6 +360,8 @@ class BuildState extends ChangeNotifier { } final response = await cocoonService.vacuumGitHubCommits( await authService.idToken, + repo: currentRepo, + branch: currentBranch, ); if (response.error != null) { _errors.send('$errorMessageRefreshGitHubCommits: ${response.error}'); diff --git a/dashboard/test/service/appengine_cocoon_test.dart b/dashboard/test/service/appengine_cocoon_test.dart index 729903c20..9395e0032 100644 --- a/dashboard/test/service/appengine_cocoon_test.dart +++ b/dashboard/test/service/appengine_cocoon_test.dart @@ -254,7 +254,11 @@ void main() { test('should return true if request succeeds', () async { await expectLater( - service.vacuumGitHubCommits('fakeIdToken'), + service.vacuumGitHubCommits( + 'fakeIdToken', + repo: 'flutter', + branch: 'master', + ), completion( isA>().having((r) => r.data, 'data', isTrue), ), @@ -268,7 +272,11 @@ void main() { }), ); await expectLater( - service.vacuumGitHubCommits('fakeIdToken'), + service.vacuumGitHubCommits( + 'fakeIdToken', + repo: 'flutter', + branch: 'master', + ), completion( isA>().having((r) => r.error, 'data', isNotNull), ), From 1e74fece3890113cc103aac4d6bf8ee7606414a5 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 14 May 2025 09:25:36 -0700 Subject: [PATCH 2/3] Update mocks. --- dashboard/test/utils/mocks.mocks.dart | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/dashboard/test/utils/mocks.mocks.dart b/dashboard/test/utils/mocks.mocks.dart index 63a4762bf..49c8c01b0 100644 --- a/dashboard/test/utils/mocks.mocks.dart +++ b/dashboard/test/utils/mocks.mocks.dart @@ -417,13 +417,25 @@ class MockCocoonService extends _i1.Mock implements _i3.CocoonService { as _i8.Future<_i3.CocoonResponse>); @override - _i8.Future<_i3.CocoonResponse> vacuumGitHubCommits(String? idToken) => + _i8.Future<_i3.CocoonResponse> vacuumGitHubCommits( + String? idToken, { + required String? repo, + required String? branch, + }) => (super.noSuchMethod( - Invocation.method(#vacuumGitHubCommits, [idToken]), + Invocation.method( + #vacuumGitHubCommits, + [idToken], + {#repo: repo, #branch: branch}, + ), returnValue: _i8.Future<_i3.CocoonResponse>.value( _FakeCocoonResponse_2( this, - Invocation.method(#vacuumGitHubCommits, [idToken]), + Invocation.method( + #vacuumGitHubCommits, + [idToken], + {#repo: repo, #branch: branch}, + ), ), ), ) From c335be1293037de5ce4f161766edcec922f2cc34 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 14 May 2025 09:44:18 -0700 Subject: [PATCH 3/3] ++ --- dashboard/test/state/build_test.dart | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/dashboard/test/state/build_test.dart b/dashboard/test/state/build_test.dart index 00f5fb1f6..50491ed3a 100644 --- a/dashboard/test/state/build_test.dart +++ b/dashboard/test/state/build_test.dart @@ -544,7 +544,13 @@ void main() { final result = await buildState.refreshGitHubCommits(); expect(result, isFalse); - verifyNever(cocoonService.vacuumGitHubCommits(any)); + verifyNever( + cocoonService.vacuumGitHubCommits( + any, + branch: anyNamed('branch'), + repo: anyNamed('repo'), + ), + ); }); testWidgets( @@ -553,7 +559,13 @@ void main() { const idToken = 'id_token'; when(authService.isAuthenticated).thenReturn(true); when(authService.idToken).thenAnswer((_) async => idToken); - when(cocoonService.vacuumGitHubCommits(idToken)).thenAnswer( + when( + cocoonService.vacuumGitHubCommits( + idToken, + branch: anyNamed('branch'), + repo: anyNamed('repo'), + ), + ).thenAnswer( (_) async => const CocoonResponse.error('Bad user', statusCode: 401), ); @@ -574,7 +586,11 @@ void main() { when(authService.isAuthenticated).thenReturn(true); when(authService.idToken).thenAnswer((_) async => idToken); when( - cocoonService.vacuumGitHubCommits(idToken), + cocoonService.vacuumGitHubCommits( + idToken, + branch: anyNamed('branch'), + repo: anyNamed('repo'), + ), ).thenAnswer((_) async => const CocoonResponse.data(true)); final buildState = BuildState(