Skip to content

Commit e57b369

Browse files
authored
Fix backfiller when fetching ToT when running on a non-default branch. (#4711)
Closes flutter/flutter#169086. This was actually a pretty bad bug, it's lucky we caught this (thx experimental branch!)
1 parent f89f8e0 commit e57b369

3 files changed

Lines changed: 46 additions & 21 deletions

File tree

app_dart/lib/src/request_handlers/scheduler/batch_backfiller.dart

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import 'package:meta/meta.dart';
1212

1313
import '../../../cocoon_service.dart';
1414
import '../../model/ci_yaml/ci_yaml.dart';
15-
import '../../model/commit_ref.dart';
1615
import '../../model/firestore/task.dart' as fs;
1716
import '../../request_handling/exceptions.dart';
1817
import '../../service/firestore/commit_and_tasks.dart';
@@ -63,6 +62,7 @@ final class BatchBackfiller extends RequestHandler {
6362
//
6463
// See https://github.com/flutter/flutter/issues/168738.
6564
Future<void> _backfillExperimentalBranch(RepositorySlug slug) async {
65+
log.debug('Running experimental branch backfiller for "$slug"');
6666
final fsGrid = await _firestore.queryRecentCommitsAndTasks(
6767
slug,
6868
commitLimit: config.flags.backfillerCommitLimit,
@@ -116,21 +116,20 @@ final class BatchBackfiller extends RequestHandler {
116116
);
117117

118118
// Download the ToT .ci.yaml targets.
119-
final ciYaml = await _ciYamlFetcher.getCiYamlByCommit(
120-
CommitRef(
121-
slug: slug,
122-
sha: fsGrid.first.commit.sha,
123-
branch: Config.defaultBranch(slug),
124-
),
125-
postsubmit: true,
126-
);
119+
final totCiYaml = await _ciYamlFetcher.getTipOfTreeCiYaml(slug: slug);
127120

128121
final totTargets = [
129-
...ciYaml.postsubmitTargets(),
130-
if (ciYaml.isFusion)
131-
...ciYaml.postsubmitTargets(type: CiType.fusionEngine),
122+
...totCiYaml.postsubmitTargets(),
123+
if (totCiYaml.isFusion)
124+
...totCiYaml.postsubmitTargets(type: CiType.fusionEngine),
132125
];
133-
log.debug('Fetched ${totTargets.length} tip-of-tree targets');
126+
if (totTargets.isEmpty) {
127+
log.warn(
128+
'Did not fetch any tip-of-tree targets. Backfill will do nothing!',
129+
);
130+
} else {
131+
log.debug('Fetched ${totTargets.length} tip-of-tree targets');
132+
}
134133

135134
grid = BackfillGrid.from([
136135
for (final CommitAndTasks(:commit, :tasks) in fsGrid)

app_dart/lib/src/service/scheduler/ci_yaml_fetcher.dart

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,7 @@ interface class CiYamlFetcher {
6969
}
7070

7171
final isFusion = commit.slug == Config.flutterSlug;
72-
final totCommit = await _fetchTipOfTreeCommit(slug: commit.slug);
73-
final totYaml = await _getCiYaml(
74-
commit: totCommit,
75-
validate: validate,
76-
isFusionCommit: isFusion,
77-
useGitOnBorgFallback: true,
78-
);
72+
final totYaml = await getTipOfTreeCiYaml(slug: commit.slug);
7973
return _getCiYaml(
8074
commit: commit,
8175
validate: validate,
@@ -85,6 +79,16 @@ interface class CiYamlFetcher {
8579
);
8680
}
8781

82+
/// Returns the latest `ci.yaml`(s) for the default branch of [slug].
83+
Future<CiYamlSet> getTipOfTreeCiYaml({required RepositorySlug slug}) async {
84+
return await _getCiYaml(
85+
commit: await _fetchTipOfTreeCommit(slug: slug),
86+
validate: false,
87+
isFusionCommit: slug == Config.flutterSlug,
88+
useGitOnBorgFallback: true,
89+
);
90+
}
91+
8892
/// Creates and returns, using a cache, the `.ci.yaml` file for a commit.
8993
Future<CiYamlSet> _getCiYaml({
9094
required CommitRef commit,

app_dart/test/src/service/fake_ci_yaml_fetcher.dart

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,22 @@ import 'package:github/src/common/model/repos.dart';
1111
import 'package:yaml/yaml.dart';
1212

1313
final class FakeCiYamlFetcher implements CiYamlFetcher {
14-
FakeCiYamlFetcher({this.ciYaml, this.failCiYamlValidation = false});
14+
FakeCiYamlFetcher({
15+
this.ciYaml,
16+
this.totCiYaml,
17+
this.failCiYamlValidation = false,
18+
});
1519

1620
/// The value that should be returned as a canned response for [getCiYamlByCommit].
1721
///
1822
/// If omitted (`null`) defaults to a configuration with a single target.
1923
CiYamlSet? ciYaml;
2024

25+
/// The value that should be returned as a canned response for [getTipOfTreeCiYaml].
26+
///
27+
/// If omitted (`null`), defaults to the same response as [ciYaml].
28+
CiYamlSet? totCiYaml;
29+
2130
/// Sets [ciYaml] by loading a YAML document.
2231
///
2332
/// Optionally may also specify [engine] for a fusion [CiYamlSet].
@@ -59,6 +68,19 @@ final class FakeCiYamlFetcher implements CiYamlFetcher {
5968
);
6069
}
6170

71+
@override
72+
Future<CiYamlSet> getTipOfTreeCiYaml({required RepositorySlug slug}) async {
73+
final ci =
74+
totCiYaml ??
75+
ciYaml ??
76+
_createDefault(slug: slug, commitBranch: Config.defaultBranch(slug));
77+
return CiYamlSet(
78+
slug: slug,
79+
branch: Config.defaultBranch(slug),
80+
yamls: ci.configs.map((k, v) => MapEntry(k, v.config)),
81+
);
82+
}
83+
6284
static CiYamlSet _createDefault({
6385
required RepositorySlug slug,
6486
required String commitBranch,

0 commit comments

Comments
 (0)