From 7fd4c6b0d5e72fb0c973a9d48e83c0f60a3daff0 Mon Sep 17 00:00:00 2001 From: Dmitry Grand Date: Wed, 22 Jul 2026 13:58:06 -0700 Subject: [PATCH 1/3] enabled test suppression for unified check run --- .../common/presubmit_completed_check.dart | 5 +- .../presubmit_subscription.dart | 29 +-- .../presubmit_completed_check_test.dart | 27 +++ .../presubmit_luci_subscription_test.dart | 199 ++++++++++++++++++ 4 files changed, 245 insertions(+), 15 deletions(-) diff --git a/app_dart/lib/src/model/common/presubmit_completed_check.dart b/app_dart/lib/src/model/common/presubmit_completed_check.dart index 6de08aab8..4a8d69abf 100644 --- a/app_dart/lib/src/model/common/presubmit_completed_check.dart +++ b/app_dart/lib/src/model/common/presubmit_completed_check.dart @@ -69,6 +69,7 @@ class PresubmitCompletedJob { Build build, PresubmitUserData userData, { TaskStatus? status, + String? summaryPrepend, }) { return PresubmitCompletedJob( name: build.builder.builder, @@ -85,7 +86,9 @@ class PresubmitCompletedJob { attempt: _getAttempt(build), startTime: build.startTime.toDateTime().millisecondsSinceEpoch, endTime: build.endTime.toDateTime().millisecondsSinceEpoch, - summary: build.summaryMarkdown, + summary: summaryPrepend != null && summaryPrepend.isNotEmpty + ? '$summaryPrepend\n\n${build.summaryMarkdown}' + : build.summaryMarkdown, buildNumber: build.number, buildId: build.id, ); diff --git a/app_dart/lib/src/request_handlers/presubmit_subscription.dart b/app_dart/lib/src/request_handlers/presubmit_subscription.dart index 6be2b623f..968dbceff 100644 --- a/app_dart/lib/src/request_handlers/presubmit_subscription.dart +++ b/app_dart/lib/src/request_handlers/presubmit_subscription.dart @@ -173,21 +173,21 @@ base class PresubmitSubscription extends SubscriptionHandler { } } CheckRunConclusion? override; - if (!isUnifiedCheckRun) { - String? suppressedMessage; - 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'; - } + String? suppressedMessage; + 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'; } + } + if (!isUnifiedCheckRun) { if (userData.checkRunId == null) { log.error('checkRunId is null for non-unified check run'); return; @@ -209,6 +209,7 @@ base class PresubmitSubscription extends SubscriptionHandler { status: override == CheckRunConclusion.neutral ? TaskStatus.neutral : null, + summaryPrepend: suppressedMessage, ); await _scheduler.processCheckRunCompleted(check); } diff --git a/app_dart/test/model/common/presubmit_completed_check_test.dart b/app_dart/test/model/common/presubmit_completed_check_test.dart index d2fdc3737..9849e22f4 100644 --- a/app_dart/test/model/common/presubmit_completed_check_test.dart +++ b/app_dart/test/model/common/presubmit_completed_check_test.dart @@ -91,5 +91,32 @@ void main() { expect(check.buildNumber, 1234); expect(check.buildId, Int64.MAX_VALUE); }); + + test('fromBuild handles custom status and summaryPrepend', () { + final build = Build( + id: Int64.MAX_VALUE, + builder: BuilderID(builder: 'test_builder'), + status: Status.FAILURE, + summaryMarkdown: 'Build failed.', + ); + + final userData = PresubmitUserData( + commit: CommitRef(slug: slug, sha: sha, branch: 'master'), + stage: CiStage.fusionEngineBuild, + pullRequestNumber: 1, + guardCheckRunId: 123, + checkSuiteId: 456, + ); + + final check = PresubmitCompletedJob.fromBuild( + build, + userData, + status: TaskStatus.neutral, + summaryPrepend: 'Suppressed message', + ); + + expect(check.status, TaskStatus.neutral); + expect(check.summary, 'Suppressed message\n\nBuild failed.'); + }); }); } 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 8e3467719..0b10ce785 100644 --- a/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart +++ b/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart @@ -36,6 +36,7 @@ void main() { late MockScheduler mockScheduler; late FakeFirestoreService firestore; late FakePubSub pubSub; + late FakeBuildBucketClient buildBucketClient; setUp(() async { firestore = FakeFirestoreService(); @@ -50,12 +51,14 @@ void main() { ciYaml: examplePresubmitRescheduleFusionConfig, ); + buildBucketClient = FakeBuildBucketClient(); handler = PresubmitLuciSubscription( cache: CacheService.inMemory(), config: config, luciBuildService: FakeLuciBuildService( config: config, firestore: firestore, + buildBucketClient: buildBucketClient, ), githubChecksService: mockGithubChecksService, authProvider: FakeDashboardAuthentication(), @@ -647,8 +650,163 @@ void main() { ), ), ).called(1); + + final captured = verify( + mockScheduler.processCheckRunCompleted(captureAny), + ).captured; + expect(captured, hasLength(1)); + expect( + captured[0], + isA() + .having((e) => e.status, 'status', TaskStatus.neutral) + .having( + (e) => e.summary, + 'summary', + contains( + '### ⚠️ Test failed but marked as suppressed on dashboard', + ), + ), + ); + }); + + test('Requests when unified check run failed and is suppressed', () async { + final userData = PresubmitUserData( + commit: CommitRef( + sha: 'abc', + branch: 'master', + slug: RepositorySlug('flutter', 'flutter'), + ), + guardCheckRunId: 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', + ), + ); + + buildBucketClient.getBuildResponse = Future.value( + bbv2.Build() + ..id = Int64(1) + ..builder = (bbv2.BuilderID()..builder = 'Linux A') + ..status = bbv2.Status.FAILURE + ..summaryMarkdown = 'test summary', + ); + + 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); + + verifyNever( + mockGithubChecksService.updateCheckStatus( + build: anyNamed('build'), + checkRunId: anyNamed('checkRunId'), + luciBuildService: anyNamed('luciBuildService'), + slug: anyNamed('slug'), + conclusionOverride: anyNamed('conclusionOverride'), + summaryPrepend: anyNamed('summaryPrepend'), + ), + ); + + final captured = verify( + mockScheduler.processCheckRunCompleted(captureAny), + ).captured; + expect(captured, hasLength(1)); + expect( + captured[0], + isA() + .having((e) => e.status, 'status', TaskStatus.neutral) + .having( + (e) => e.summary, + 'summary', + contains( + '### ⚠️ Test failed but marked as suppressed on dashboard', + ), + ), + ); }); + test( + 'Requests when unified check run failed and is NOT suppressed', + () async { + final userData = PresubmitUserData( + commit: CommitRef( + sha: 'abc', + branch: 'master', + slug: RepositorySlug('flutter', 'flutter'), + ), + guardCheckRunId: 1, + checkSuiteId: 2, + ); + + buildBucketClient.getBuildResponse = Future.value( + bbv2.Build() + ..id = Int64(1) + ..builder = (bbv2.BuilderID()..builder = 'Linux A') + ..status = bbv2.Status.FAILURE + ..summaryMarkdown = 'test summary', + ); + + 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); + + verifyNever( + mockGithubChecksService.updateCheckStatus( + build: anyNamed('build'), + checkRunId: anyNamed('checkRunId'), + luciBuildService: anyNamed('luciBuildService'), + slug: anyNamed('slug'), + conclusionOverride: anyNamed('conclusionOverride'), + summaryPrepend: anyNamed('summaryPrepend'), + ), + ); + + final captured = verify( + mockScheduler.processCheckRunCompleted(captureAny), + ).captured; + expect(captured, hasLength(1)); + expect( + captured[0], + isA() + .having((e) => e.status, 'status', TaskStatus.failed) + .having( + (e) => e.summary, + 'summary', + isNot(contains('marked as suppressed')), + ), + ); + }, + ); + test('Suppression check skipped when rescheduled', () async { tester.message = createPushMessage( Int64(1), @@ -692,6 +850,47 @@ void main() { ).called(1); }); + test('Unified Suppression check skipped when rescheduled', () async { + buildBucketClient.getBuildResponse = Future.value( + bbv2.Build() + ..id = Int64(1) + ..builder = (bbv2.BuilderID() + ..builder = 'Linux presubmit_max_attempts=2') + ..status = bbv2.Status.FAILURE + ..summaryMarkdown = 'test summary', + ); + + 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'), + ), + guardCheckRunId: 1, + checkSuiteId: 2, + ), + ); + + await tester.post(handler); + + verifyNever( + mockGithubChecksService.updateCheckStatus( + build: anyNamed('build'), + checkRunId: anyNamed('checkRunId'), + luciBuildService: anyNamed('luciBuildService'), + slug: anyNamed('slug'), + conclusionOverride: anyNamed('conclusionOverride'), + summaryPrepend: anyNamed('summaryPrepend'), + ), + ); + + verifyNever(mockScheduler.processCheckRunCompleted(any)); + }); + test( 'publishes build message to ordered-presubmit topic with orderingKey when ordering_key flag exists', () async { From 13b291f8470c2a1ec91a27bf3afadbfcf33bf3be Mon Sep 17 00:00:00 2001 From: Dmitry Grand Date: Wed, 22 Jul 2026 14:03:43 -0700 Subject: [PATCH 2/3] ai review --- .../lib/src/model/common/presubmit_completed_check.dart | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app_dart/lib/src/model/common/presubmit_completed_check.dart b/app_dart/lib/src/model/common/presubmit_completed_check.dart index 4a8d69abf..11e61f1c2 100644 --- a/app_dart/lib/src/model/common/presubmit_completed_check.dart +++ b/app_dart/lib/src/model/common/presubmit_completed_check.dart @@ -86,9 +86,10 @@ class PresubmitCompletedJob { attempt: _getAttempt(build), startTime: build.startTime.toDateTime().millisecondsSinceEpoch, endTime: build.endTime.toDateTime().millisecondsSinceEpoch, - summary: summaryPrepend != null && summaryPrepend.isNotEmpty - ? '$summaryPrepend\n\n${build.summaryMarkdown}' - : build.summaryMarkdown, + summary: [ + if (summaryPrepend != null && summaryPrepend.isNotEmpty) summaryPrepend, + if (build.summaryMarkdown.isNotEmpty) build.summaryMarkdown, + ].join('\n\n'), buildNumber: build.number, buildId: build.id, ); From 0670d3b7f23a6b8c0db6e3f1215e7759d476de2d Mon Sep 17 00:00:00 2001 From: Dmitry Grand Date: Wed, 22 Jul 2026 15:59:31 -0700 Subject: [PATCH 3/3] added dividing line --- app_dart/lib/src/model/common/presubmit_completed_check.dart | 2 +- app_dart/test/model/common/presubmit_completed_check_test.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app_dart/lib/src/model/common/presubmit_completed_check.dart b/app_dart/lib/src/model/common/presubmit_completed_check.dart index 11e61f1c2..d7a3139bc 100644 --- a/app_dart/lib/src/model/common/presubmit_completed_check.dart +++ b/app_dart/lib/src/model/common/presubmit_completed_check.dart @@ -89,7 +89,7 @@ class PresubmitCompletedJob { summary: [ if (summaryPrepend != null && summaryPrepend.isNotEmpty) summaryPrepend, if (build.summaryMarkdown.isNotEmpty) build.summaryMarkdown, - ].join('\n\n'), + ].join('\n---\n'), buildNumber: build.number, buildId: build.id, ); diff --git a/app_dart/test/model/common/presubmit_completed_check_test.dart b/app_dart/test/model/common/presubmit_completed_check_test.dart index 9849e22f4..a2b9de29d 100644 --- a/app_dart/test/model/common/presubmit_completed_check_test.dart +++ b/app_dart/test/model/common/presubmit_completed_check_test.dart @@ -116,7 +116,7 @@ void main() { ); expect(check.status, TaskStatus.neutral); - expect(check.summary, 'Suppressed message\n\nBuild failed.'); + expect(check.summary, 'Suppressed message\n---\nBuild failed.'); }); }); }