Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 51 additions & 5 deletions app_dart/lib/src/model/firestore/ci_staging.dart
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ class CiStaging extends Document {
required String checkRun,
required String conclusion,
}) async {
final logCrumb = 'markConclusion(${slug.owner}_${slug.name}_${sha}_$stage, $checkRun, $conclusion)';
final changeCrumb = '${slug.owner}_${slug.name}_$sha';
final logCrumb = 'markConclusion(${changeCrumb}_$stage, $checkRun, $conclusion)';

// Marking needs to happen while in a transaction to ensure `remaining` is
// updated correctly. For that to happen correctly; we need to perform a
Expand All @@ -110,8 +111,10 @@ class CiStaging extends Document {

var remaining = -1;
var failed = -1;
var total = -1;
bool valid = false;
String? checkRunGuard;
String? recordedConclusion;

late final Document doc;

Expand Down Expand Up @@ -146,9 +149,15 @@ class CiStaging extends Document {
}
failed = maybeFailed;

final maybeTotal = int.tryParse(fields[kTotalField]?.integerValue ?? '');
if (maybeTotal == null) {
throw '$logCrumb: missing field "$kTotalField" for $transaction / ${doc.fields}';
}
total = maybeTotal;

// We will have check_runs scheduled after the engine was built successfully, so missing the checkRun field
// is an OK response to have. All fields should have been written at creation time.
final recordedConclusion = fields[checkRun]?.stringValue;
recordedConclusion = fields[checkRun]?.stringValue;
if (recordedConclusion == null) {
log.info('$logCrumb: $checkRun not present in doc for $transaction / $doc');
await docRes.rollback(RollbackRequest(transaction: transaction), kDatabase);
Expand All @@ -157,6 +166,8 @@ class CiStaging extends Document {
remaining: remaining,
checkRunGuard: null,
failed: failed,
summary: 'Check run "$checkRun" not present in $stage CI stage',
details: 'Change $changeCrumb',
);
}

Expand Down Expand Up @@ -207,7 +218,7 @@ class CiStaging extends Document {
fields[checkRun] = Value(stringValue: conclusion);
fields[kRemainingField] = Value(integerValue: '$remaining');
fields[kFailedField] = Value(integerValue: '$failed');
} on DetailedApiRequestError catch (e) {
} on DetailedApiRequestError catch (e, stack) {
if (e.status == 404) {
// An attempt to read a document not in firestore should not be retried.
log.info('$logCrumb: staging document not found for $transaction');
Expand All @@ -217,6 +228,15 @@ class CiStaging extends Document {
remaining: -1,
checkRunGuard: null,
failed: failed,
summary: 'Internal server error',
details: '''
Staging document not found for CI stage "$stage" for $changeCrumb. Got 404 from
Firestore.

Error:
${e.toString()}
$stack
''',
);
}
// All other errors should bubble up and be retried.
Expand All @@ -239,6 +259,15 @@ class CiStaging extends Document {
remaining: remaining,
checkRunGuard: checkRunGuard,
failed: failed,
summary: valid ? 'All tests passed' : 'Not a valid state transition for $checkRun',
details: valid
? '''
For CI stage $stage:
Total check runs scheduled: $total
Pending: $remaining
Failed: $failed
'''
: 'Attempted to transition the state of check run $checkRun from "$recordedConclusion" to "$conclusion".',
);
}

Expand Down Expand Up @@ -343,12 +372,16 @@ class StagingConclusion {
final int remaining;
final String? checkRunGuard;
final int failed;
final String summary;
final String details;

const StagingConclusion({
required this.result,
required this.remaining,
required this.checkRunGuard,
required this.failed,
required this.summary,
required this.details,
});

bool get isOk => result == StagingConclusionResult.ok;
Expand All @@ -366,8 +399,21 @@ class StagingConclusion {
other.result == result &&
other.remaining == remaining &&
other.checkRunGuard == checkRunGuard &&
other.failed == failed);
other.failed == failed &&
other.summary == summary &&
other.details == details);

@override
int get hashCode => Object.hashAll([
result,
remaining,
checkRunGuard,
failed,
summary,
details,
]);

@override
int get hashCode => Object.hashAll([result, remaining, checkRunGuard, failed]);
String toString() =>
'StagingConclusion("$result", "$remaining", "$checkRunGuard", "$failed", "$summary", "$details")';
}
29 changes: 27 additions & 2 deletions app_dart/lib/src/service/config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,37 @@ const String kDefaultBranchName = 'master';
class Config {
Config(this._db, this._cache);

/// Labels autosubmit looks for on pull requests
/// When present on a pull request, instructs Cocoon to submit it
/// automatically as soon as all the required checks pass.
///
/// Keep this in sync with the similar `Config` class in `auto_submit`.
static const String kAutosubmitLabel = 'autosubmit';

/// When present on a pull request, allows it to land without passing all the
/// checks, and jumps the merge queue.
///
/// Keep this in sync with the similar `Config` class in `auto_submit`.
static const String kEmergencyLabel = 'emergency';

/// Validates that CI tasks were successfully created from the .ci.yaml file.
///
/// If this check fails, it means Cocoon failed to fully populate the list of
/// CI checks and the PR/commit should be treated as failing.
static const String kCiYamlCheckName = 'ci.yaml validation';

/// A required check that stays in pending state until a sufficient subset of
/// checks pass.
///
/// This check is "required", meaning that it must pass before Github will
/// allow a PR to land in the merge queue, or a merge group to land on the
/// target branch (main or master).
///
/// IMPORTANT: the name of this task - "Merge Queue Guard" - must strictly
/// match the name of the required check configured in the repo settings.
/// Changing the name here or in the settings alone will break the PR
/// workflow.
static const String kMergeQueueLockName = 'Merge Queue Guard';

final DatastoreDB _db;

final CacheService _cache;
Expand Down Expand Up @@ -150,7 +176,6 @@ class Config {

// GitHub App properties.
Future<String> get githubPrivateKey => _getSingleValue('githubapp_private_pem');
Future<String> get overrideTreeStatusLabel => _getSingleValue('override_tree_status_label');
Future<String> get githubPublicKey => _getSingleValue('githubapp_public_pem');
Future<String> get githubAppId => _getSingleValue('githubapp_id');
Future<Map<String, dynamic>> get githubAppInstallations async {
Expand Down
Loading