From 0eb28917c3134ee84f86f72019701a9c45b40c6c Mon Sep 17 00:00:00 2001 From: John McDole Date: Thu, 19 Mar 2026 10:11:17 -0700 Subject: [PATCH 1/6] presubmit neutral status should be treated as success --- .../src/request_handlers/presubmit_luci_subscription.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 7177da42ad..b13bd932e7 100644 --- a/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart +++ b/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart @@ -6,7 +6,6 @@ import 'dart:convert'; import 'package:archive/archive.dart'; import 'package:buildbucket/buildbucket_pb.dart' as bbv2; -import 'package:cocoon_common/task_status.dart'; import 'package:cocoon_server/logging.dart'; import 'package:github/github.dart'; @@ -168,9 +167,7 @@ final class PresubmitLuciSubscription extends SubscriptionHandler { final check = PresubmitCompletedCheck.fromBuild( build, userData, - status: override == CheckRunConclusion.neutral - ? TaskStatus.neutral - : null, + status: override == .neutral ? .neutral : null, ); await _scheduler.processCheckRunCompleted(check); } From a28a2c1c07c5f0eda1d40d0a04ac0da0aacc7839 Mon Sep 17 00:00:00 2001 From: John McDole Date: Thu, 19 Mar 2026 10:12:12 -0700 Subject: [PATCH 2/6] cleanup: dart 3.10 + status updates - update to dart 3.10 - use dot shorthands in some places - refactor "status" maps to work. --- .../src/model/common/checks_extension.dart | 62 ++++++++++--------- .../lib/src/model/firestore/ci_staging.dart | 19 +++--- .../service/firestore/unified_check_run.dart | 12 ++-- app_dart/lib/src/service/scheduler.dart | 2 +- app_dart/pubspec.yaml | 2 +- .../model/common/checks_extension_test.dart | 53 ++++++++++++++++ .../test/model/firestore/ci_staging_test.dart | 43 +++++++++++++ auto_submit/pubspec.yaml | 2 +- dashboard/lib/service/data_seeder.dart | 20 +++--- dashboard/lib/views/presubmit_view.dart | 16 ++--- dashboard/lib/widgets/task_overlay.dart | 18 +++--- .../test/utils/generate_task_for_tests.dart | 16 ++--- packages/cocoon_common/lib/task_status.dart | 36 +++++------ packages/cocoon_common/pubspec.yaml | 2 +- .../cocoon_common/test/task_status_test.dart | 31 +++++----- packages/cocoon_common_test/pubspec.yaml | 2 +- packages/cocoon_server/pubspec.yaml | 2 +- packages/cocoon_server_test/pubspec.yaml | 2 +- pubspec.yaml | 2 +- 19 files changed, 218 insertions(+), 124 deletions(-) create mode 100644 app_dart/test/model/common/checks_extension_test.dart diff --git a/app_dart/lib/src/model/common/checks_extension.dart b/app_dart/lib/src/model/common/checks_extension.dart index d6e2d7622b..f8b9fa568f 100644 --- a/app_dart/lib/src/model/common/checks_extension.dart +++ b/app_dart/lib/src/model/common/checks_extension.dart @@ -11,20 +11,22 @@ extension ChecksExtension on TaskStatus { /// Converts a [TaskStatus] to a [TaskConclusion]. TaskConclusion toTaskConclusion() { return switch (this) { - TaskStatus.succeeded => TaskConclusion.success, - TaskStatus.failed || TaskStatus.infraFailure => TaskConclusion.failure, - TaskStatus.inProgress => TaskConclusion.scheduled, - _ => TaskConclusion.unknown, + .succeeded => .success, + .neutral => .neutral, + .failed || .infraFailure => .failure, + .inProgress => .scheduled, + _ => .unknown, }; } /// Converts a [TaskStatus] to a GitHub check conclusion string. String toConclusion() { return switch (this) { - TaskStatus.succeeded => 'success', - TaskStatus.failed || TaskStatus.infraFailure => 'failure', - TaskStatus.cancelled => 'cancelled', - TaskStatus.skipped => 'skipped', + .succeeded => 'success', + .neutral => 'neutral', + .failed || .infraFailure => 'failure', + .cancelled => 'cancelled', + .skipped => 'skipped', _ => '', }; } @@ -32,39 +34,39 @@ extension ChecksExtension on TaskStatus { /// Converts a [TaskStatus] to a GitHub check conclusion string. CheckRunConclusion toCheckRunConclusion() { return switch (this) { - TaskStatus.succeeded => CheckRunConclusion.success, - TaskStatus.failed || - TaskStatus.infraFailure => CheckRunConclusion.failure, - TaskStatus.cancelled => CheckRunConclusion.cancelled, - TaskStatus.skipped => CheckRunConclusion.skipped, - _ => CheckRunConclusion.empty, + .succeeded => .success, + .failed || .infraFailure => .failure, + .cancelled => .cancelled, + .skipped => .skipped, + .neutral => .neutral, + _ => .empty, }; } /// Converts a GitHub check conclusion string to a [TaskStatus]. static TaskStatus fromConclusion(String? conclusion) { return switch (conclusion) { - 'success' => TaskStatus.succeeded, - 'failure' => TaskStatus.failed, - 'neutral' => TaskStatus.succeeded, - 'cancelled' => TaskStatus.cancelled, - 'timed_out' => TaskStatus.failed, - 'action_required' => TaskStatus.failed, - 'skipped' => TaskStatus.skipped, - _ => TaskStatus.failed, + 'success' => .succeeded, + 'failure' => .failed, + 'neutral' => .neutral, + 'cancelled' => .cancelled, + 'timed_out' => .failed, + 'action_required' => .failed, + 'skipped' => .skipped, + _ => .failed, }; } static TaskStatus fromCheckRunConclusion(CheckRunConclusion? conclusion) { return switch (conclusion) { - CheckRunConclusion.success => TaskStatus.succeeded, - CheckRunConclusion.failure => TaskStatus.failed, - CheckRunConclusion.neutral => TaskStatus.succeeded, - CheckRunConclusion.cancelled => TaskStatus.cancelled, - CheckRunConclusion.timedOut => TaskStatus.failed, - CheckRunConclusion.actionRequired => TaskStatus.failed, - CheckRunConclusion.skipped => TaskStatus.skipped, - _ => TaskStatus.failed, + .success => .succeeded, + .failure => .failed, + .neutral => .neutral, + .cancelled => .cancelled, + .timedOut => .failed, + .actionRequired => .failed, + .skipped => .skipped, + _ => .failed, }; } } diff --git a/app_dart/lib/src/model/firestore/ci_staging.dart b/app_dart/lib/src/model/firestore/ci_staging.dart index bae6cb2ebc..f97246c74d 100644 --- a/app_dart/lib/src/model/firestore/ci_staging.dart +++ b/app_dart/lib/src/model/firestore/ci_staging.dart @@ -289,8 +289,7 @@ final class CiStaging extends AppDocument { // recordedConclusion == failure && conclusion == success: down (-1) // recordedConclusion != failure && conclusion == failure: up (+1) // So if the test existed and either remaining or failed_count is changed; the response is valid. - if (recordedConclusion == TaskConclusion.scheduled && - conclusion != TaskConclusion.scheduled) { + if (recordedConclusion == .scheduled && conclusion != .scheduled) { // Guard against going negative and log enough info so we can debug. if (remaining == 0) { throw '$logCrumb: field "$kRemainingField" is already zero for $transaction / ${doc.fields}'; @@ -301,8 +300,7 @@ final class CiStaging extends AppDocument { // Only rollback the "failed" counter if this is a successful test run, // i.e. the test failed, the user requested a rerun, and now it passes. - if (recordedConclusion == TaskConclusion.failure && - conclusion == TaskConclusion.success) { + if (recordedConclusion == .failure && conclusion.isSuccess) { log.info( '$logCrumb: conclusion flipped to positive - assuming test was re-run', ); @@ -314,9 +312,8 @@ final class CiStaging extends AppDocument { } // Only increment the "failed" counter if the new conclusion flips from positive or neutral to failure. - if ((recordedConclusion == TaskConclusion.scheduled || - recordedConclusion == TaskConclusion.success) && - conclusion == TaskConclusion.failure) { + if ((recordedConclusion == .scheduled || recordedConclusion.isSuccess) && + conclusion == .failure) { log.info('$logCrumb: test failed'); valid = true; failed = failed + 1; @@ -460,6 +457,9 @@ enum TaskConclusion { /// A task was completed as a success. success, + /// A task was completed, but status is ignored. + neutral, + /// A task was completed as a failure. failure; @@ -477,5 +477,8 @@ enum TaskConclusion { bool get isComplete => this != scheduled; /// Whether the task is a success or not. - bool get isSuccess => this == success; + bool get isSuccess => this == success || this == neutral; + + /// Whether the task is a failure or not. + bool get isFailure => this == .failure; } diff --git a/app_dart/lib/src/service/firestore/unified_check_run.dart b/app_dart/lib/src/service/firestore/unified_check_run.dart index 99aae7c675..bfcba9dcfc 100644 --- a/app_dart/lib/src/service/firestore/unified_check_run.dart +++ b/app_dart/lib/src/service/firestore/unified_check_run.dart @@ -213,9 +213,9 @@ final class UnifiedCheckRun { // request re-run we have to only increment remaining builds. // If build is still in progress re-run is not possible but if some how they // manage to request re-run we should not touch any counters. - if (currentStatus.isBuildCompleted) { + if (currentStatus.isComplete) { guard.remainingBuilds += 1; - if (currentStatus.isBuildFailed && guard.failedBuilds > 0) { + if (currentStatus.isFailure && guard.failedBuilds > 0) { guard.failedBuilds -= 1; } } @@ -584,20 +584,20 @@ final class UnifiedCheckRun { } else if (state.status == TaskStatus.inProgress) { presubmitCheck.startTime = state.startTime!; // If the build is not completed, update the status. - if (!status.isBuildCompleted) { + if (!status.isComplete) { status = state.status; } valid = true; } else { // If build already compleated remaining and failed should not updated. - if (!status.isBuildCompleted) { + if (!status.isComplete) { // "remaining" should go down if build is succeeded or failed. // "failed_count" can go up or down depending on: // attemptNumber > 1 && buildSuccessed: down (-1) // attemptNumber = 1 && buildFailed: up (+1) // So if the test existed and either remaining or failed_count is changed; // the response is valid. - if (state.status.isBuildCompleted) { + if (state.status.isComplete) { // Guard against going negative and log enough info so we can debug. if (remaining == 0) { throw '$logCrumb: field "${PresubmitGuard.fieldRemainingBuilds}" is already zero for $transaction / ${presubmitGuardDocument.fields}'; @@ -606,7 +606,7 @@ final class UnifiedCheckRun { valid = true; } - if (state.status.isBuildFailed) { + if (state.status.isFailure) { log.info('$logCrumb: test failed'); failed += 1; valid = true; diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index 9d809310f3..891ca7c600 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -1025,7 +1025,7 @@ detailsUrl: $detailsUrl } else { // for github flow check runs are processed only if the build succeeded or // some kind of failure occurred. - if (!check.status.isBuildCompleted) { + if (!check.status.isComplete) { return true; } // Check runs are fired at every stage. However, at this point it is unknown diff --git a/app_dart/pubspec.yaml b/app_dart/pubspec.yaml index 749f35d761..2caf9b00c5 100644 --- a/app_dart/pubspec.yaml +++ b/app_dart/pubspec.yaml @@ -10,7 +10,7 @@ publish_to: none resolution: workspace environment: - sdk: ^3.9.0 + sdk: ^3.10.0 dependencies: appengine: 0.13.11 diff --git a/app_dart/test/model/common/checks_extension_test.dart b/app_dart/test/model/common/checks_extension_test.dart new file mode 100644 index 0000000000..ac75eaf171 --- /dev/null +++ b/app_dart/test/model/common/checks_extension_test.dart @@ -0,0 +1,53 @@ +// 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:cocoon_common/task_status.dart'; +import 'package:cocoon_service/src/model/common/checks_extension.dart'; +import 'package:cocoon_service/src/model/firestore/ci_staging.dart'; +import 'package:github/github.dart'; +import 'package:test/test.dart'; + +void main() { + group('ChecksExtension', () { + test('toTaskConclusion mapping for neutral', () { + expect(TaskStatus.neutral.toTaskConclusion(), TaskConclusion.neutral); + }); + + test('toConclusion mapping for neutral', () { + expect(TaskStatus.neutral.toConclusion(), 'neutral'); + }); + + test('toCheckRunConclusion mapping for neutral', () { + expect( + TaskStatus.neutral.toCheckRunConclusion(), + CheckRunConclusion.neutral, + ); + }); + + test('fromConclusion mapping for neutral', () { + expect(ChecksExtension.fromConclusion('neutral'), TaskStatus.neutral); + }); + + test('fromCheckRunConclusion mapping for neutral', () { + expect( + ChecksExtension.fromCheckRunConclusion(CheckRunConclusion.neutral), + TaskStatus.neutral, + ); + }); + }); + + group('TaskConclusion', () { + test('isSuccess returns true for neutral', () { + expect(TaskConclusion.neutral.isSuccess, isTrue); + }); + + test('isFailure returns false for neutral', () { + expect(TaskConclusion.neutral.isFailure, isFalse); + }); + + test('isComplete returns true for neutral', () { + expect(TaskConclusion.neutral.isComplete, isTrue); + }); + }); +} diff --git a/app_dart/test/model/firestore/ci_staging_test.dart b/app_dart/test/model/firestore/ci_staging_test.dart index 60f926f9ee..fec100b633 100644 --- a/app_dart/test/model/firestore/ci_staging_test.dart +++ b/app_dart/test/model/firestore/ci_staging_test.dart @@ -324,6 +324,49 @@ For CI stage engine: ); }); + test('handles a test flip-flop (neutral) after re-running', () async { + firestoreService.putDocument( + Document( + name: expectedName, + fields: { + CiStaging.kRemainingField: 1.toValue(), + CiStaging.kFailedField: 1.toValue(), + CiStaging.kTotalField: 1.toValue(), + CiStaging.kCheckRunGuardField: '{}'.toValue(), + 'MacOS build_test': TaskConclusion.failure.name.toValue(), + }, + ), + ); + + final future = CiStaging.markConclusion( + firestoreService: firestoreService, + slug: slug, + sha: '1234', + stage: CiStage.fusionEngineBuild, + checkRun: 'MacOS build_test', + conclusion: TaskConclusion.neutral, + ); + + final result = await future; + // Remaining == 1 because our test was already concluded. + expect( + result, + const PresubmitGuardConclusion( + remaining: 1, + result: PresubmitGuardConclusionResult.ok, + failed: 0, + checkRunGuard: '{}', + summary: 'All tests passed', + details: ''' +For CI stage engine: + Total check runs scheduled: 1 + Pending: 1 + Failed: 0 +''', + ), + ); + }); + test('ignored repeat failures', () async { firestoreService.putDocument( Document( diff --git a/auto_submit/pubspec.yaml b/auto_submit/pubspec.yaml index ced03d44fc..af6f6dd2d2 100644 --- a/auto_submit/pubspec.yaml +++ b/auto_submit/pubspec.yaml @@ -10,7 +10,7 @@ homepage: https://github.com/flutter/cocoon/blob/main/autosubmit resolution: workspace environment: - sdk: ^3.9.0 + sdk: ^3.10.0 dependencies: appengine: 0.13.11 diff --git a/dashboard/lib/service/data_seeder.dart b/dashboard/lib/service/data_seeder.dart index 67b78f8f6a..23289c59f5 100644 --- a/dashboard/lib/service/data_seeder.dart +++ b/dashboard/lib/service/data_seeder.dart @@ -411,7 +411,7 @@ class DataSeeder { .where((status) => status.isFailure) .length; final remainingBuilds = builds.values - .where((status) => !status.isBuildCompleted) + .where((status) => !status.isComplete) .length; return PresubmitGuard( @@ -448,20 +448,20 @@ class DataSeeder { creationTime: creationTime, buildNumber: 1337 + attemptNumber, summary: switch (status) { - TaskStatus.succeeded => + .succeeded => '[INFO] Starting task $buildName...\n[SUCCESS] All tests passed (452/452)', - TaskStatus.failed => + .failed => '[INFO] Starting task $buildName...\n[ERROR] Test failed: Dummy Tests', - TaskStatus.infraFailure => + .infraFailure => '[INFO] Starting task $buildName...\n[ERROR] Infrastructure failure: Dummy Tests', - TaskStatus.cancelled => + .cancelled => '[INFO] Starting task $buildName...\n[ERROR] Test cancelled: Dummy Tests', - TaskStatus.inProgress => '[INFO] Starting task $buildName...', - TaskStatus.waitingForBackfill => null, - TaskStatus.skipped => + .inProgress => '[INFO] Starting task $buildName...', + .waitingForBackfill => null, + .skipped => '[INFO] Starting task $buildName...\n[INFO] Test skipped: Dummy Tests', - TaskStatus.neutral => - '[INFO] Starting task $buildName...\n[INFO] Test status neutral: Dummy Tests', + .neutral => + '[INFO] Starting task $buildName...\n[INFO] Test neutral: Dummy Tests', }, startTime: creationTime + 30000, endTime: creationTime + 60000, diff --git a/dashboard/lib/views/presubmit_view.dart b/dashboard/lib/views/presubmit_view.dart index 84113f45b2..6ffce20489 100644 --- a/dashboard/lib/views/presubmit_view.dart +++ b/dashboard/lib/views/presubmit_view.dart @@ -717,45 +717,45 @@ class _CheckItem extends StatelessWidget { Widget _getStatusIcon(TaskStatus status) { switch (status) { - case TaskStatus.succeeded: + case .succeeded: return Icon( Icons.check_circle_outline, color: TaskBox.statusColor[status], size: 18, ); - case TaskStatus.failed: + case .failed: return Icon( Icons.error_outline, color: TaskBox.statusColor[status], size: 18, ); - case TaskStatus.infraFailure: + case .infraFailure: return Icon( Icons.error_outline, color: TaskBox.statusColor[status], size: 18, ); - case TaskStatus.waitingForBackfill: + case .waitingForBackfill: return Icon( Icons.not_started_outlined, color: TaskBox.statusColor[status], size: 18, ); - case TaskStatus.skipped: + case .skipped: return Icon( Icons.do_not_disturb_on_outlined, color: TaskBox.statusColor[status], size: 18, ); - case TaskStatus.neutral: + case .neutral: return Icon(Icons.flaky, color: TaskBox.statusColor[status], size: 18); - case TaskStatus.cancelled: + case .cancelled: return Icon( Icons.block_outlined, color: TaskBox.statusColor[status], size: 18, ); - case TaskStatus.inProgress: + case .inProgress: return SizedBox( width: 14, height: 14, diff --git a/dashboard/lib/widgets/task_overlay.dart b/dashboard/lib/widgets/task_overlay.dart index ba1665dbd4..cd65217d8d 100644 --- a/dashboard/lib/widgets/task_overlay.dart +++ b/dashboard/lib/widgets/task_overlay.dart @@ -252,22 +252,22 @@ class TaskOverlayContents extends StatelessWidget { ); switch (task.status) { - case TaskStatus.inProgress when wasQueued: + case .inProgress when wasQueued: final ranFor = now.difference(startTime); buffer.write('Running for ${ranFor.inMinutes} minutes'); - case TaskStatus.skipped: + case .skipped: buffer.write('Skipped'); - case TaskStatus.neutral: + case .neutral: buffer.write('Neutral'); - case TaskStatus.cancelled: + case .cancelled: buffer.write('Cancelled'); - case TaskStatus.succeeded: - case TaskStatus.failed: - case TaskStatus.infraFailure: + case .succeeded: + case .failed: + case .infraFailure: final ranFor = endTime.difference(startTime); buffer.write('Ran for ${ranFor.inMinutes} minutes'); - case TaskStatus.waitingForBackfill: - case TaskStatus.inProgress: + case .waitingForBackfill: + case .inProgress: } return buffer.toString(); diff --git a/dashboard/test/utils/generate_task_for_tests.dart b/dashboard/test/utils/generate_task_for_tests.dart index ae12cc1cce..29795331ef 100644 --- a/dashboard/test/utils/generate_task_for_tests.dart +++ b/dashboard/test/utils/generate_task_for_tests.dart @@ -64,18 +64,18 @@ Task generateTaskForTest({ final bool started; final bool completed; switch (status) { - case TaskStatus.cancelled: - case TaskStatus.failed: - case TaskStatus.infraFailure: - case TaskStatus.succeeded: - case TaskStatus.neutral: + case .cancelled: + case .failed: + case .infraFailure: + case .succeeded: + case .neutral: started = true; completed = true; - case TaskStatus.inProgress: + case .inProgress: started = true; completed = false; - case TaskStatus.waitingForBackfill: - case TaskStatus.skipped: + case .waitingForBackfill: + case .skipped: started = false; completed = false; } diff --git a/packages/cocoon_common/lib/task_status.dart b/packages/cocoon_common/lib/task_status.dart index a40451b383..4644559517 100644 --- a/packages/cocoon_common/lib/task_status.dart +++ b/packages/cocoon_common/lib/task_status.dart @@ -31,7 +31,12 @@ enum TaskStatus { neutral('Neutral'), /// The task was skipped instead of being executed. - skipped('Skipped'); + skipped('Skipped'), + + /// The task was completed, but we record the results as non-blocking. + /// + /// Example: suppressed tests + neutral('Neutral'); const TaskStatus(this.value); @@ -55,15 +60,19 @@ enum TaskStatus { final String value; /// Whether the status represents a completed task reaching a terminal state. - bool get isComplete => _complete.contains(this); - static const _complete = {..._failed, succeeded, skipped}; + bool get isComplete => isSuccess || isFailure; /// Whether the status represents a failure state. - bool get isFailure => _failed.contains(this); - static const _failed = {cancelled, infraFailure, failed}; + bool get isFailure => switch (this) { + cancelled || infraFailure || failed => true, + _ => false, + }; /// Whether the status represents a success state. - bool get isSuccess => this == succeeded; + bool get isSuccess => switch (this) { + succeeded || skipped || neutral => true, + _ => false, + }; /// Whether the status represents a skipped state. bool get isSkipped => this == skipped; @@ -73,20 +82,7 @@ enum TaskStatus { /// Returns true if the build is waiting for backfill or in progress. bool get isBuildInProgress => - this == TaskStatus.waitingForBackfill || this == TaskStatus.inProgress; - - /// Returns true if the build succeeded or was skipped. - bool get isBuildSuccessed => - this == TaskStatus.succeeded || this == TaskStatus.skipped; - - /// Returns true if the build failed, had an infra failure, or was cancelled. - bool get isBuildFailed => - this == TaskStatus.failed || - this == TaskStatus.infraFailure || - this == TaskStatus.cancelled; - - /// Returns true if the build succeeded or some kind of failure occurred. - bool get isBuildCompleted => isBuildSuccessed || isBuildFailed; + this == .waitingForBackfill || this == .inProgress; /// Returns the JSON representation of `this`. Object? toJson() => value; diff --git a/packages/cocoon_common/pubspec.yaml b/packages/cocoon_common/pubspec.yaml index 421132905f..597a576d4f 100644 --- a/packages/cocoon_common/pubspec.yaml +++ b/packages/cocoon_common/pubspec.yaml @@ -3,7 +3,7 @@ description: Shared functionality and interfaces across flutter/cocoon publish_to: none environment: - sdk: ^3.9.0 + sdk: ^3.10.0 resolution: workspace diff --git a/packages/cocoon_common/test/task_status_test.dart b/packages/cocoon_common/test/task_status_test.dart index bf1abec12e..297050360c 100644 --- a/packages/cocoon_common/test/task_status_test.dart +++ b/packages/cocoon_common/test/task_status_test.dart @@ -38,6 +38,7 @@ void main() { TaskStatus.failed: 'Failed', TaskStatus.succeeded: 'Succeeded', TaskStatus.skipped: 'Skipped', + TaskStatus.neutral: 'Neutral', }.entries) { test('$value == $expectation', () { expect(value.value, expectation); @@ -48,15 +49,21 @@ void main() { group('isSuccess', () { test('true for succeeded', () { expect(TaskStatus.succeeded.isSuccess, isTrue); + expect(TaskStatus.neutral.isSuccess, isTrue); + expect(TaskStatus.skipped.isSuccess, isTrue); }); test('false for everything else', () { - expect( - TaskStatus.values - .where((v) => v != TaskStatus.succeeded) - .map((t) => t.isSuccess), - everyElement(isFalse), - ); + for (final value in TaskStatus.values) { + switch (value) { + case TaskStatus.succeeded: + case TaskStatus.skipped: + case TaskStatus.neutral: + expect(value.isSuccess, isTrue); + default: + expect(value.isSuccess, isFalse); + } + } }); }); @@ -87,7 +94,7 @@ void main() { group('false otherwise', () { for (final status in TaskStatus.values.where( - (v) => !v.isFailure && !v.isSuccess && !v.isSkipped, + (v) => !(v.isSuccess || v.isFailure), )) { test('$status', () { expect(status.isComplete, isFalse); @@ -120,16 +127,6 @@ void main() { }); }); - test('isSuccess', () { - expect(TaskStatus.succeeded.isSuccess, isTrue); - expect( - TaskStatus.values.where((v) => v != TaskStatus.succeeded), - everyElement( - isA().having((t) => t.isSuccess, 'isSuccess', isFalse), - ), - ); - }); - test('isSkipped', () { expect(TaskStatus.skipped.isSkipped, isTrue); expect( diff --git a/packages/cocoon_common_test/pubspec.yaml b/packages/cocoon_common_test/pubspec.yaml index 1760ed7f8c..4c3f76549d 100644 --- a/packages/cocoon_common_test/pubspec.yaml +++ b/packages/cocoon_common_test/pubspec.yaml @@ -3,7 +3,7 @@ description: Testing specific functionality for package:cocoon_common publish_to: none environment: - sdk: ^3.9.0 + sdk: ^3.10.0 resolution: workspace diff --git a/packages/cocoon_server/pubspec.yaml b/packages/cocoon_server/pubspec.yaml index 888c28305e..cf1a185722 100644 --- a/packages/cocoon_server/pubspec.yaml +++ b/packages/cocoon_server/pubspec.yaml @@ -5,7 +5,7 @@ publish_to: none resolution: workspace environment: - sdk: ^3.9.0 + sdk: ^3.10.0 # Version constraints in this package are intentionally relaxed, because the # exact versions are locked by app_dart and auto_submit apps. diff --git a/packages/cocoon_server_test/pubspec.yaml b/packages/cocoon_server_test/pubspec.yaml index b67fd2dbee..9b7b045e15 100644 --- a/packages/cocoon_server_test/pubspec.yaml +++ b/packages/cocoon_server_test/pubspec.yaml @@ -5,7 +5,7 @@ publish_to: none resolution: workspace environment: - sdk: ^3.9.0 + sdk: ^3.10.0 dependencies: cocoon_common: diff --git a/pubspec.yaml b/pubspec.yaml index bb83e7b2e0..73e1c03738 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -6,7 +6,7 @@ name: _cocoon_workspace # Required for workspace support. environment: - sdk: ^3.9.0 + sdk: ^3.10.0 workspace: - app_dart From 0e85200e84033eac2aacf9b5ad185ae2b9c7746f Mon Sep 17 00:00:00 2001 From: John McDole Date: Thu, 19 Mar 2026 10:54:06 -0700 Subject: [PATCH 3/6] Bad merge --- packages/cocoon_common/lib/task_status.dart | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/cocoon_common/lib/task_status.dart b/packages/cocoon_common/lib/task_status.dart index 4644559517..e2246ccdc0 100644 --- a/packages/cocoon_common/lib/task_status.dart +++ b/packages/cocoon_common/lib/task_status.dart @@ -31,12 +31,7 @@ enum TaskStatus { neutral('Neutral'), /// The task was skipped instead of being executed. - skipped('Skipped'), - - /// The task was completed, but we record the results as non-blocking. - /// - /// Example: suppressed tests - neutral('Neutral'); + skipped('Skipped'); const TaskStatus(this.value); From d93cd73122cc7610e916818006b0500549f43db0 Mon Sep 17 00:00:00 2001 From: John McDole Date: Thu, 19 Mar 2026 11:11:24 -0700 Subject: [PATCH 4/6] use test logger lint --- app_dart/test/model/common/checks_extension_test.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app_dart/test/model/common/checks_extension_test.dart b/app_dart/test/model/common/checks_extension_test.dart index ac75eaf171..c5065a1981 100644 --- a/app_dart/test/model/common/checks_extension_test.dart +++ b/app_dart/test/model/common/checks_extension_test.dart @@ -3,12 +3,15 @@ // found in the LICENSE file. import 'package:cocoon_common/task_status.dart'; +import 'package:cocoon_server_test/test_logging.dart'; import 'package:cocoon_service/src/model/common/checks_extension.dart'; import 'package:cocoon_service/src/model/firestore/ci_staging.dart'; import 'package:github/github.dart'; import 'package:test/test.dart'; void main() { + useTestLoggerPerTest(); + group('ChecksExtension', () { test('toTaskConclusion mapping for neutral', () { expect(TaskStatus.neutral.toTaskConclusion(), TaskConclusion.neutral); From 0b8840df35027d3feec4448e3df7bb76ca102256 Mon Sep 17 00:00:00 2001 From: John McDole Date: Thu, 19 Mar 2026 11:15:02 -0700 Subject: [PATCH 5/6] tweaks --- app_dart/lib/src/model/firestore/ci_staging.dart | 2 +- packages/cocoon_common/lib/task_status.dart | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app_dart/lib/src/model/firestore/ci_staging.dart b/app_dart/lib/src/model/firestore/ci_staging.dart index f97246c74d..00d64353dc 100644 --- a/app_dart/lib/src/model/firestore/ci_staging.dart +++ b/app_dart/lib/src/model/firestore/ci_staging.dart @@ -480,5 +480,5 @@ enum TaskConclusion { bool get isSuccess => this == success || this == neutral; /// Whether the task is a failure or not. - bool get isFailure => this == .failure; + bool get isFailure => this == failure; } diff --git a/packages/cocoon_common/lib/task_status.dart b/packages/cocoon_common/lib/task_status.dart index e2246ccdc0..6aef6159c7 100644 --- a/packages/cocoon_common/lib/task_status.dart +++ b/packages/cocoon_common/lib/task_status.dart @@ -76,8 +76,10 @@ enum TaskStatus { bool get isRunning => this == inProgress; /// Returns true if the build is waiting for backfill or in progress. - bool get isBuildInProgress => - this == .waitingForBackfill || this == .inProgress; + bool get isBuildInProgress => switch (this) { + waitingForBackfill || inProgress => true, + _ => false, + }; /// Returns the JSON representation of `this`. Object? toJson() => value; From 84ae37bd4bc8947d3d5f697515a7cfb47403418a Mon Sep 17 00:00:00 2001 From: John McDole Date: Thu, 19 Mar 2026 11:31:36 -0700 Subject: [PATCH 6/6] codefu is a horrible failure --- app_dart/lib/src/model/firestore/ci_staging.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app_dart/lib/src/model/firestore/ci_staging.dart b/app_dart/lib/src/model/firestore/ci_staging.dart index 00d64353dc..d0f57caf41 100644 --- a/app_dart/lib/src/model/firestore/ci_staging.dart +++ b/app_dart/lib/src/model/firestore/ci_staging.dart @@ -300,7 +300,7 @@ final class CiStaging extends AppDocument { // Only rollback the "failed" counter if this is a successful test run, // i.e. the test failed, the user requested a rerun, and now it passes. - if (recordedConclusion == .failure && conclusion.isSuccess) { + if (recordedConclusion.isFailure && conclusion.isSuccess) { log.info( '$logCrumb: conclusion flipped to positive - assuming test was re-run', ); @@ -313,7 +313,7 @@ final class CiStaging extends AppDocument { // Only increment the "failed" counter if the new conclusion flips from positive or neutral to failure. if ((recordedConclusion == .scheduled || recordedConclusion.isSuccess) && - conclusion == .failure) { + conclusion.isFailure) { log.info('$logCrumb: test failed'); valid = true; failed = failed + 1;