diff --git a/app_dart/lib/server.dart b/app_dart/lib/server.dart index e8f95b014..a696eb6f1 100644 --- a/app_dart/lib/server.dart +++ b/app_dart/lib/server.dart @@ -111,6 +111,7 @@ Server createServer({ githubChecksService: githubChecksService, scheduler: scheduler, ciYamlFetcher: ciYamlFetcher, + firestore: firestore, ), '/api/v2/postsubmit-luci-subscription': PostsubmitLuciSubscription( cache: cache, @@ -218,6 +219,7 @@ Server createServer({ authenticationProvider: dashboardAuthProvider, firestore: firestore, config: config, + cache: cache, ), '/api/merge_queue_hooks': MergeQueueHooks( authenticationProvider: dashboardAuthProvider, diff --git a/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart b/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart index a6dcebfdf..4b85b5f86 100644 --- a/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart +++ b/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart @@ -7,22 +7,19 @@ import 'dart:convert'; import 'package:archive/archive.dart'; import 'package:buildbucket/buildbucket_pb.dart' as bbv2; import 'package:cocoon_server/logging.dart'; +import 'package:github/github.dart'; +import '../../cocoon_service.dart'; import '../model/bbv2_extension.dart'; import '../model/ci_yaml/ci_yaml.dart'; import '../model/ci_yaml/target.dart'; import '../model/commit_ref.dart'; import '../model/common/presubmit_completed_check.dart'; -import '../request_handling/authentication.dart'; import '../request_handling/exceptions.dart'; -import '../request_handling/request_handler.dart'; -import '../request_handling/response.dart'; import '../request_handling/subscription_handler.dart'; -import '../service/github_checks_service.dart'; -import '../service/luci_build_service.dart'; +import '../service/extensions/cache_service_test_suppression.dart'; import '../service/luci_build_service/build_tags.dart'; import '../service/luci_build_service/user_data.dart'; -import '../service/scheduler.dart'; import '../service/scheduler/ci_yaml_fetcher.dart'; /// An endpoint for listening to LUCI status updates for scheduled builds. @@ -46,17 +43,20 @@ final class PresubmitLuciSubscription extends SubscriptionHandler { required GithubChecksService githubChecksService, required CiYamlFetcher ciYamlFetcher, required Scheduler scheduler, + required FirestoreService firestore, AuthenticationProvider? authProvider, }) : _ciYamlFetcher = ciYamlFetcher, _githubChecksService = githubChecksService, _luciBuildService = luciBuildService, _scheduler = scheduler, + _firestore = firestore, super(subscriptionName: 'build-bucket-presubmit-sub'); final LuciBuildService _luciBuildService; final GithubChecksService _githubChecksService; final CiYamlFetcher _ciYamlFetcher; final Scheduler _scheduler; + final FirestoreService _firestore; @override Future post(Request request) async { @@ -134,12 +134,30 @@ final class PresubmitLuciSubscription extends SubscriptionHandler { } } if (!isUnifiedCheckRun) { + String? suppressedMessage; + CheckRunConclusion? override; + if (build.status.isTaskFailed() && !rescheduled) { + // If a test is suppressed; we avoid setting a failing status. + final isSuppressed = await cache.isTestSuppressed( + testName: builderName, + repository: userData.commit.slug, + firestore: _firestore, + ); + if (isSuppressed) { + override = CheckRunConclusion.neutral; + suppressedMessage = + '### ⚠️ Test failed but marked as suppressed on dashboard'; + } + } + await _githubChecksService.updateCheckStatus( checkRunId: userData.checkRunId!, build: build, luciBuildService: _luciBuildService, slug: userData.commit.slug, rescheduled: rescheduled, + conclusionOverride: override, + summaryPrepend: suppressedMessage, ); } if (!rescheduled) { diff --git a/app_dart/lib/src/request_handlers/update_suppressed_test.dart b/app_dart/lib/src/request_handlers/update_suppressed_test.dart index 1fe82cfab..41ab9dc75 100644 --- a/app_dart/lib/src/request_handlers/update_suppressed_test.dart +++ b/app_dart/lib/src/request_handlers/update_suppressed_test.dart @@ -7,12 +7,10 @@ import 'package:github/github.dart'; import 'package:googleapis/firestore/v1.dart'; import 'package:meta/meta.dart'; -import '../model/firestore/suppressed_test.dart'; +import '../../cocoon_service.dart'; import '../request_handling/api_request_handler.dart'; import '../request_handling/exceptions.dart'; -import '../request_handling/request_handler.dart'; // For Request -import '../request_handling/response.dart'; -import '../service/firestore.dart'; +import '../service/extensions/cache_service_test_suppression.dart'; /// Manually updates the test suppression status. /// @@ -30,12 +28,15 @@ final class UpdateSuppressedTest extends ApiRequestHandler { required FirestoreService firestore, required super.config, required super.authenticationProvider, + required CacheService cache, @visibleForTesting DateTime Function() now = DateTime.now, }) : _firestore = firestore, - _now = now; + _now = now, + _cache = cache; final FirestoreService _firestore; final DateTime Function() _now; + final CacheService _cache; static const _paramTestName = 'testName'; static const _paramRepository = 'repository'; @@ -239,5 +240,12 @@ final class UpdateSuppressedTest extends ApiRequestHandler { collectionId: SuppressedTest.kCollectionId, ); } + + // Update cache now that we've written it + await _cache.setTestSuppression( + testName: testName, + repository: repository, + isSuppressed: isSuppressed, + ); } } diff --git a/app_dart/lib/src/service/extensions/cache_service_test_suppression.dart b/app_dart/lib/src/service/extensions/cache_service_test_suppression.dart new file mode 100644 index 000000000..909810501 --- /dev/null +++ b/app_dart/lib/src/service/extensions/cache_service_test_suppression.dart @@ -0,0 +1,51 @@ +// Copyright 2026 The Flutter 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:github/github.dart'; + +import '../../model/firestore/suppressed_test.dart'; +import '../cache_service.dart'; +import '../firestore.dart'; + +/// Handy extension on the caching service for test suppression. +extension SuppressedTestCache on CacheService { + static const String _subcacheName = 'test_suppression'; + + Future isTestSuppressed({ + required String testName, + required RepositorySlug repository, + required FirestoreService firestore, + }) async { + final cacheValue = await getOrCreate( + _subcacheName, + '${repository.fullName}/$testName', + createFn: () async { + final latest = await SuppressedTest.getLatest( + firestore, + repository.fullName, + testName, + ); + if (latest == null) return false.toUint8List(); + return latest.isSuppressed.toUint8List(); + }, + // Only cache for a short while. + ttl: const Duration(minutes: 5), + ); + + return cacheValue?.toBool() ?? false; + } + + Future setTestSuppression({ + required String testName, + required RepositorySlug repository, + required bool isSuppressed, + }) async { + await set( + _subcacheName, + '${repository.fullName}/$testName', + isSuppressed.toUint8List(), + ttl: const Duration(minutes: 5), + ); + } +} diff --git a/app_dart/lib/src/service/github_checks_service.dart b/app_dart/lib/src/service/github_checks_service.dart index bc43392d9..4496164a0 100644 --- a/app_dart/lib/src/service/github_checks_service.dart +++ b/app_dart/lib/src/service/github_checks_service.dart @@ -47,6 +47,8 @@ class GithubChecksService { required github.RepositorySlug slug, required int checkRunId, bool rescheduled = false, + github.CheckRunConclusion? conclusionOverride, + String? summaryPrepend, }) async { var status = statusForResult(build.status); log.info('status for build ${build.id} is ${status.value}'); @@ -63,9 +65,11 @@ class GithubChecksService { 'name': build.builder.builder, }); - var conclusion = (terminalStatuses.contains(build.status)) - ? conclusionForResult(build.status) - : null; + var conclusion = + conclusionOverride ?? + ((terminalStatuses.contains(build.status)) + ? conclusionForResult(build.status) + : null); log.info( 'conclusion for build ${build.id} is ${(conclusion != null) ? conclusion.value : null}', ); @@ -94,10 +98,11 @@ class GithubChecksService { allFields: true, ), ); - output = github.CheckRunOutput( - title: checkRun.name!, - summary: getGithubSummary(buildbucketBuild.summaryMarkdown), - ); + var summary = getGithubSummary(buildbucketBuild.summaryMarkdown); + if (summaryPrepend != null && summaryPrepend.isNotEmpty) { + summary = '$summaryPrepend\n\n$summary'; + } + output = github.CheckRunOutput(title: checkRun.name!, summary: summary); log.debug( 'Updating check run with output: [${output.toJson().toString()}]', ); diff --git a/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart b/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart index 4a71a6a97..206a38bf1 100644 --- a/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart +++ b/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart @@ -34,9 +34,10 @@ void main() { late MockLuciBuildService mockLuciBuildService; late FakeCiYamlFetcher ciYamlFetcher; late MockScheduler mockScheduler; + late FakeFirestoreService firestore; setUp(() async { - final firestore = FakeFirestoreService(); + firestore = FakeFirestoreService(); config = FakeConfig( dynamicConfig: DynamicConfig.fromJson({ @@ -62,6 +63,7 @@ void main() { authProvider: FakeDashboardAuthentication(), scheduler: mockScheduler, ciYamlFetcher: ciYamlFetcher, + firestore: firestore, ); request = FakeHttpRequest(); @@ -274,6 +276,7 @@ void main() { authProvider: FakeDashboardAuthentication(), scheduler: mockScheduler, ciYamlFetcher: ciYamlFetcher, + firestore: firestore, ); await tester.post(luciHandler); @@ -506,6 +509,7 @@ void main() { authProvider: FakeDashboardAuthentication(), scheduler: mockScheduler, ciYamlFetcher: ciYamlFetcher, + firestore: firestore, ); await tester.post(luciHandler); @@ -577,4 +581,115 @@ void main() { .having((e) => e.headBranch, 'headBranch', 'master'), ); }); + + test('Requests when task failed and is suppressed', () async { + final userData = PresubmitUserData( + commit: CommitRef( + sha: 'abc', + branch: 'master', + slug: RepositorySlug('flutter', 'flutter'), + ), + checkRunId: 1, + checkSuiteId: 2, + ); + + // Setup Firestore + firestore.putDocument( + SuppressedTest( + name: 'Linux A', + repository: 'flutter/flutter', + issueLink: 'https://github.com/flutter/flutter/issues/123', + isSuppressed: true, + createTimestamp: DateTime.now(), + ) + ..name = firestore.resolveDocumentName( + SuppressedTest.kCollectionId, + 'suppressed_1', + ), + ); + + when( + mockGithubChecksService.updateCheckStatus( + build: anyNamed('build'), + checkRunId: anyNamed('checkRunId'), + luciBuildService: anyNamed('luciBuildService'), + slug: anyNamed('slug'), + conclusionOverride: github.CheckRunConclusion.neutral, + summaryPrepend: argThat( + contains('marked as suppressed'), + named: 'summaryPrepend', + ), + ), + ).thenAnswer((_) async => true); + + when( + mockScheduler.processCheckRunCompleted(any), + ).thenAnswer((_) async => true); + + tester.message = createPushMessage( + Int64(1), + status: bbv2.Status.FAILURE, + builder: 'Linux A', + userData: userData, + ); + + await tester.post(handler); + + verify( + mockGithubChecksService.updateCheckStatus( + build: anyNamed('build'), + checkRunId: anyNamed('checkRunId'), + luciBuildService: anyNamed('luciBuildService'), + slug: anyNamed('slug'), + conclusionOverride: github.CheckRunConclusion.neutral, + summaryPrepend: argThat( + contains('### ⚠️ Test failed but marked as suppressed on dashboard'), + named: 'summaryPrepend', + ), + ), + ).called(1); + }); + + test('Suppression check skipped when rescheduled', () async { + tester.message = createPushMessage( + Int64(1), + status: bbv2.Status.FAILURE, + builder: 'Linux presubmit_max_attempts=2', + userData: PresubmitUserData( + commit: CommitRef( + sha: 'abc', + branch: 'master', + slug: RepositorySlug('flutter', 'flutter'), + ), + checkRunId: 1, + checkSuiteId: 2, + ), + ); + + when( + mockGithubChecksService.updateCheckStatus( + build: anyNamed('build'), + checkRunId: anyNamed('checkRunId'), + luciBuildService: anyNamed('luciBuildService'), + slug: anyNamed('slug'), + rescheduled: true, + conclusionOverride: null, + summaryPrepend: null, + ), + ).thenAnswer((_) async => true); + + await tester.post(handler); + + verify( + mockGithubChecksService.updateCheckStatus( + build: anyNamed('build'), + checkRunId: anyNamed('checkRunId'), + luciBuildService: anyNamed('luciBuildService'), + slug: anyNamed('slug'), + rescheduled: true, + conclusionOverride: null, + summaryPrepend: null, + ), + ).called(1); + }); } diff --git a/app_dart/test/request_handlers/update_suppressed_test_test.dart b/app_dart/test/request_handlers/update_suppressed_test_test.dart index feae6971a..62d306fd2 100644 --- a/app_dart/test/request_handlers/update_suppressed_test_test.dart +++ b/app_dart/test/request_handlers/update_suppressed_test_test.dart @@ -9,6 +9,8 @@ import 'package:cocoon_server_test/test_logging.dart'; import 'package:cocoon_service/src/model/firestore/suppressed_test.dart'; import 'package:cocoon_service/src/request_handlers/update_suppressed_test.dart'; import 'package:cocoon_service/src/request_handling/exceptions.dart'; +import 'package:cocoon_service/src/service/cache_service.dart'; +import 'package:cocoon_service/src/service/extensions/cache_service_test_suppression.dart'; import 'package:cocoon_service/src/service/firestore.dart'; import 'package:cocoon_service/src/service/flags/dynamic_config.dart'; import 'package:github/github.dart'; @@ -24,6 +26,7 @@ void main() { late UpdateSuppressedTest handler; late FakeConfig config; late FakeGithubServiceWithIssue githubService; + late CacheService cache; final fakeNow = DateTime.now().toUtc(); @@ -31,6 +34,7 @@ void main() { firestore = FakeFirestoreService(); tester = ApiRequestHandlerTester(); githubService = FakeGithubServiceWithIssue(); + cache = CacheService(inMemory: true); // Enable feature flag by default for most tests final dynamicConfig = DynamicConfig.fromJson({ @@ -47,6 +51,7 @@ void main() { authenticationProvider: FakeDashboardAuthentication(), firestore: firestore, now: () => fakeNow, + cache: cache, ); }); @@ -304,6 +309,70 @@ void main() { ]), ); }); + + test('updates cache on SUPPRESS', () async { + githubService.issueResponse = Issue(state: 'open'); + + tester.request.body = jsonEncode({ + 'testName': 'my_test', + 'repository': 'flutter/flutter', + 'action': 'SUPPRESS', + 'issueLink': 'https://github.com/flutter/flutter/issues/123', + }); + + await tester.post(handler); + + // Verify cache was updated + final isSuppressed = await cache.isTestSuppressed( + testName: 'my_test', + repository: RepositorySlug('flutter', 'flutter'), + firestore: firestore, + ); + expect(isSuppressed, isTrue); + }); + + test('updates cache on UNSUPPRESS', () async { + githubService.issueResponse = Issue(state: 'open'); + + // Pre-populate Firestore so that the handler doesn't return early. + final existingDoc = + SuppressedTest( + name: 'my_test', + repository: 'flutter/flutter', + issueLink: 'https://github.com/flutter/flutter/issues/123', + isSuppressed: true, + createTimestamp: DateTime.now().toUtc(), + ) + ..name = firestore.resolveDocumentName( + SuppressedTest.kCollectionId, + 'existing_doc', + ); + firestore.putDocument(existingDoc); + + // Ensure cache is initially true + await cache.setTestSuppression( + testName: 'my_test', + repository: RepositorySlug('flutter', 'flutter'), + isSuppressed: true, + ); + + tester.request.body = jsonEncode({ + 'testName': 'my_test', + 'repository': 'flutter/flutter', + 'action': 'UNSUPPRESS', + 'issueLink': 'https://github.com/flutter/flutter/issues/123', + }); + + await tester.post(handler); + + // Verify cache was updated to false + final isSuppressed = await cache.isTestSuppressed( + testName: 'my_test', + repository: RepositorySlug('flutter', 'flutter'), + firestore: firestore, + ); + expect(isSuppressed, isFalse); + }); } class FakeGithubServiceWithIssue extends FakeGithubService { diff --git a/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart b/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart index 41bdedaed..bcef0a2e5 100644 --- a/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart +++ b/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart @@ -1871,6 +1871,8 @@ class MockGithubChecksService extends _i1.Mock required _i7.RepositorySlug? slug, required int? checkRunId, bool? rescheduled = false, + _i7.CheckRunConclusion? conclusionOverride, + String? summaryPrepend, }) => (super.noSuchMethod( Invocation.method(#updateCheckStatus, [], { @@ -1879,6 +1881,8 @@ class MockGithubChecksService extends _i1.Mock #slug: slug, #checkRunId: checkRunId, #rescheduled: rescheduled, + #conclusionOverride: conclusionOverride, + #summaryPrepend: summaryPrepend, }), returnValue: _i13.Future.value(false), )