Skip to content

Commit 60a4a71

Browse files
jason-simmonsjason-simmons
authored
Revert "Always set FLUTTER_PREBUILT_ENGINE_VERSION for framework-stage PRs. (flutter#4254)" (flutter#4259)
This reverts commit fc543f1. This is causing errors when presubmit CI jobs try to download the Dart SDK from a URL that is missing the "flutter_archives_v2" realm.
1 parent fc543f1 commit 60a4a71

File tree

6 files changed

+30
-182
lines changed

6 files changed

+30
-182
lines changed

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

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -461,9 +461,8 @@ class Scheduler {
461461
pullRequest: pullRequest,
462462
checkRunGuard: '$lock',
463463
logCrumb: logCrumb,
464-
465-
// The if-branch already skips the engine build phase.
466464
skipEngine: true,
465+
flutterPrebuiltEngineVersion: pullRequest.base!.sha,
467466
);
468467
break;
469468
}
@@ -1169,16 +1168,12 @@ $stackTrace
11691168
await unlockMergeQueueGuard(slug, sha, checkRunGuard);
11701169
}
11711170

1172-
/// Fetches, and schedules, tests that execute _after_ the engine is built.
1173-
///
1174-
/// If [skipEngine] is `true`, engine tests are not scheduled (it is
1175-
/// assumed that the engine has not changed in [pullRequest] so there are no
1176-
/// need to run them at this PR).
11771171
Future<void> _runCiTestingStage({
11781172
required PullRequest pullRequest,
11791173
required String checkRunGuard,
11801174
required String logCrumb,
11811175
bool skipEngine = false,
1176+
String? flutterPrebuiltEngineVersion,
11821177
}) async {
11831178
try {
11841179
// Both the author and label should be checked to make sure that no one is
@@ -1190,7 +1185,6 @@ $stackTrace
11901185
// Schedule the tests that would have run in a call to triggerPresubmitTargets - but for both the
11911186
// engine and the framework.
11921187
final presubmitTargets = await getTestsForStage(pullRequest, CiStage.fusionTests, skipEngine: skipEngine);
1193-
11941188
// Create the document for tracking test check runs.
11951189
await initializeCiStagingDocument(
11961190
firestoreService: firestoreService,
@@ -1201,22 +1195,6 @@ $stackTrace
12011195
checkRunGuard: checkRunGuard,
12021196
);
12031197

1204-
// Here is where it gets fun: how do framework tests* know what engine
1205-
// artifacts to fetch and use on CI? For presubmits on flutter/flutter;
1206-
// see https://github.com/flutter/flutter/issues/164031.
1207-
//
1208-
// *In theory, also engine tests, but engine tests build from the engine
1209-
// from source and rely on remote-build execution (RBE) for builds to
1210-
// fast and cached.
1211-
final String flutterPrebuiltEngineVersion;
1212-
if (skipEngine) {
1213-
// Use the engine that this PR was branched off of.
1214-
flutterPrebuiltEngineVersion = pullRequest.base!.sha!;
1215-
} else {
1216-
// Use the engine that was built from source *for* this PR.
1217-
flutterPrebuiltEngineVersion = pullRequest.head!.sha!;
1218-
}
1219-
12201198
await luciBuildService.scheduleTryBuilds(
12211199
targets: presubmitTargets,
12221200
pullRequest: pullRequest,

‎app_dart/test/model/firestore/pr_check_runs_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ void main() {
2727
generateCheckRun(1, name: 'check 1'),
2828
generateCheckRun(2, name: 'check 2'),
2929
];
30-
final pr = generatePullRequest(id: 11252024, repo: 'fluax', headSha: '1234abc');
30+
final pr = generatePullRequest(id: 11252024, repo: 'fluax', sha: '1234abc');
3131

3232
late MockProjectsDatabasesDocumentsResource docRes;
3333

‎app_dart/test/request_handlers/github/webhook_subscription_test.dart‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2499,7 +2499,7 @@ void foo() {
24992499
test('applies emergency label on approved PRs', () async {
25002500
final pullRequest = generatePullRequest(
25012501
number: 123,
2502-
headSha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
2502+
sha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
25032503
labels: [
25042504
IssueLabel(
25052505
name: 'emergency',
@@ -2545,7 +2545,7 @@ void foo() {
25452545
test('logs and gracefully skips emergency label on missing Merge Queue Guard', () async {
25462546
final pullRequest = generatePullRequest(
25472547
number: 123,
2548-
headSha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
2548+
sha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
25492549
labels: [
25502550
IssueLabel(
25512551
name: 'emergency',
@@ -2610,7 +2610,7 @@ void foo() {
26102610
test('leaves educational comment for new emergency PRs', () async {
26112611
final pullRequest = generatePullRequest(
26122612
number: 123,
2613-
headSha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
2613+
sha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
26142614
labels: [
26152615
IssueLabel(
26162616
name: 'emergency',
@@ -2656,7 +2656,7 @@ void foo() {
26562656
test('leaves only one educational comment for new emergency PRs', () async {
26572657
final pullRequest = generatePullRequest(
26582658
number: 123,
2659-
headSha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
2659+
sha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
26602660
labels: [
26612661
IssueLabel(
26622662
name: 'emergency',
@@ -2707,7 +2707,7 @@ void foo() {
27072707
test('does not leave educational comment for non-new emergency PRs', () async {
27082708
final pullRequest = generatePullRequest(
27092709
number: 123,
2710-
headSha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
2710+
sha: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
27112711
labels: [
27122712
IssueLabel(
27132713
name: 'emergency',

‎app_dart/test/request_handlers/push_build_status_to_github_test.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ void main() {
127127
});
128128

129129
test('if status has not changed since last update', () async {
130-
final PullRequest pr = generatePullRequest(id: 1, headSha: 'sha1');
130+
final PullRequest pr = generatePullRequest(id: 1, sha: 'sha1');
131131
when(pullRequestsService.list(any, base: anyNamed('base'))).thenAnswer((_) => Stream<PullRequest>.value(pr));
132132
buildStatusService.cumulativeStatus = BuildStatus.success();
133133
githubBuildStatus = generateFirestoreGithubBuildStatus(1);
@@ -137,7 +137,7 @@ void main() {
137137
});
138138

139139
test('if there is no pr found for a targeted branch', () async {
140-
final PullRequest pr = generatePullRequest(id: 1, headSha: 'sha1', branch: 'test_branch');
140+
final PullRequest pr = generatePullRequest(id: 1, sha: 'sha1', branch: 'test_branch');
141141
when(pullRequestsService.list(any, base: anyNamed('base'))).thenAnswer((_) => Stream<PullRequest>.value(pr));
142142
buildStatusService.cumulativeStatus = BuildStatus.success();
143143
githubBuildStatus = generateFirestoreGithubBuildStatus(1, status: GithubBuildStatus.statusFailure);
@@ -157,7 +157,7 @@ void main() {
157157
).thenAnswer((Invocation invocation) {
158158
return Future<BatchWriteResponse>.value(BatchWriteResponse());
159159
});
160-
final PullRequest pr = generatePullRequest(id: 1, headSha: 'sha1');
160+
final PullRequest pr = generatePullRequest(id: 1, sha: 'sha1');
161161
when(pullRequestsService.list(any, base: anyNamed('base'))).thenAnswer((_) => Stream<PullRequest>.value(pr));
162162
buildStatusService.cumulativeStatus = BuildStatus.success();
163163
githubBuildStatus = generateFirestoreGithubBuildStatus(1, status: GithubBuildStatus.statusFailure);
@@ -186,7 +186,7 @@ void main() {
186186
return Future<BatchWriteResponse>.value(BatchWriteResponse());
187187
});
188188
final PullRequest pr =
189-
generatePullRequest(id: 1, headSha: 'sha1', labels: [IssueLabel(name: Config.kEmergencyLabel)]);
189+
generatePullRequest(id: 1, sha: 'sha1', labels: [IssueLabel(name: Config.kEmergencyLabel)]);
190190
when(pullRequestsService.list(any, base: anyNamed('base'))).thenAnswer((_) => Stream<PullRequest>.value(pr));
191191
buildStatusService.cumulativeStatus = BuildStatus.failure(const ['all bad']);
192192
githubBuildStatus = generateFirestoreGithubBuildStatus(1, status: GithubBuildStatus.statusFailure);

‎app_dart/test/service/scheduler_test.dart‎

Lines changed: 15 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,7 @@ targets:
653653
final Commit commit = shaToCommit('1');
654654
db.values[commit.key] = commit;
655655

656-
final PullRequest alreadyLandedPr = generatePullRequest(headSha: '1');
656+
final PullRequest alreadyLandedPr = generatePullRequest(sha: '1');
657657
await scheduler.addPullRequest(alreadyLandedPr);
658658

659659
expect(db.values.values.whereType<Commit>().map<String>(toSha).length, 1);
@@ -1238,13 +1238,8 @@ targets:
12381238
throw Exception('Failed to find ${request.url.path}');
12391239
});
12401240
final luci = MockLuciBuildService();
1241-
when(
1242-
luci.scheduleTryBuilds(
1243-
targets: anyNamed('targets'),
1244-
pullRequest: anyNamed('pullRequest'),
1245-
flutterPrebuiltEngineVersion: anyNamed('flutterPrebuiltEngineVersion'),
1246-
),
1247-
).thenAnswer((inv) async {
1241+
when(luci.scheduleTryBuilds(targets: anyNamed('targets'), pullRequest: anyNamed('pullRequest')))
1242+
.thenAnswer((inv) async {
12481243
return [];
12491244
});
12501245

@@ -1338,7 +1333,6 @@ targets:
13381333
luci.scheduleTryBuilds(
13391334
targets: captureAnyNamed('targets'),
13401335
pullRequest: captureAnyNamed('pullRequest'),
1341-
flutterPrebuiltEngineVersion: anyNamed('flutterPrebuiltEngineVersion'),
13421336
),
13431337
);
13441338
expect(result.callCount, 1);
@@ -1520,106 +1514,6 @@ targets:
15201514
expect(captured.first.summary, 'A critical error occurred, preventing further CI testing.');
15211515
});
15221516

1523-
// Regression test for https://github.com/flutter/flutter/issues/164031.
1524-
test('uses the built-from-source engine artifacts', () async {
1525-
final githubService = config.githubService = MockGithubService();
1526-
final githubClient = MockGitHub();
1527-
final luci = MockLuciBuildService();
1528-
final gitHubChecksService = MockGithubChecksService();
1529-
1530-
when(githubService.github).thenReturn(githubClient);
1531-
when(gitHubChecksService.githubChecksUtil).thenReturn(mockGithubChecksUtil);
1532-
1533-
scheduler = Scheduler(
1534-
cache: cache,
1535-
config: config,
1536-
datastoreProvider: (DatastoreDB db) => DatastoreService(db, 2),
1537-
buildStatusProvider: (_, __) => buildStatusService,
1538-
getFilesChanged: getFilesChanged,
1539-
githubChecksService: gitHubChecksService,
1540-
httpClientProvider: () => httpClient,
1541-
luciBuildService: luci,
1542-
fusionTester: fakeFusion,
1543-
markCheckRunConclusion: callbacks.markCheckRunConclusion,
1544-
);
1545-
1546-
when(gitHubChecksService.findMatchingPullRequest(any, any, any)).thenAnswer((inv) async {
1547-
return pullRequest;
1548-
});
1549-
1550-
final checkRuns = <CheckRun>[];
1551-
when(
1552-
mockGithubChecksUtil.createCheckRun(
1553-
any,
1554-
any,
1555-
any,
1556-
any,
1557-
output: anyNamed('output'),
1558-
conclusion: anyNamed('conclusion'),
1559-
),
1560-
).thenAnswer((inv) async {
1561-
final slug = inv.positionalArguments[1] as RepositorySlug;
1562-
final sha = inv.positionalArguments[2];
1563-
final name = inv.positionalArguments[3];
1564-
checkRuns.add(createCheckRun(id: 1, owner: slug.owner, repo: slug.name, sha: sha, name: name));
1565-
return checkRuns.last;
1566-
});
1567-
1568-
when(mockFirestoreService.documentResource()).thenAnswer((_) async {
1569-
final resource = MockProjectsDatabasesDocumentsResource();
1570-
when(
1571-
resource.createDocument(
1572-
any,
1573-
any,
1574-
any,
1575-
documentId: argThat(anything, named: 'documentId'),
1576-
mask_fieldPaths: argThat(anything, named: 'mask_fieldPaths'),
1577-
$fields: argThat(anything, named: r'$fields'),
1578-
),
1579-
).thenAnswer((_) async {
1580-
return Document();
1581-
});
1582-
return resource;
1583-
});
1584-
1585-
String? flutterPrebuiltEngineVersion;
1586-
when(
1587-
luci.scheduleTryBuilds(
1588-
targets: anyNamed('targets'),
1589-
pullRequest: anyNamed('pullRequest'),
1590-
flutterPrebuiltEngineVersion: anyNamed('flutterPrebuiltEngineVersion'),
1591-
),
1592-
).thenAnswer((Invocation i) async {
1593-
flutterPrebuiltEngineVersion = i.namedArguments[#flutterPrebuiltEngineVersion] as String?;
1594-
return [];
1595-
});
1596-
1597-
final checkRunEvent = cocoon_checks.CheckRunEvent.fromJson(
1598-
jsonDecode(checkRunString) as Map<String, dynamic>,
1599-
);
1600-
1601-
await scheduler.proceedToCiTestingStage(
1602-
checkRun: checkRunEvent.checkRun!,
1603-
slug: RepositorySlug('flutter', 'flutter'),
1604-
sha: 'abc1234',
1605-
mergeQueueGuard: checkRunFor(name: 'merge queue guard'),
1606-
logCrumb: 'test',
1607-
);
1608-
1609-
// Ensure that we used the HEAD SHA as as FLUTTER_PREBUILT_ENGINE_VERSION,
1610-
// since the engine was built from source.
1611-
//
1612-
// See https://github.com/flutter/flutter/issues/164031.
1613-
expect(
1614-
flutterPrebuiltEngineVersion,
1615-
allOf([
1616-
isNotNull,
1617-
pullRequest.head!.sha,
1618-
]),
1619-
reason: 'Should be set to HEAD (i.e. the current SHA), since the engine was built from source.',
1620-
);
1621-
});
1622-
16231517
test('does not fail the merge queue guard when a test check run fails', () async {
16241518
final githubService = config.githubService = MockGithubService();
16251519
final githubClient = MockGitHub();
@@ -1748,13 +1642,8 @@ targets:
17481642
throw Exception('Failed to find ${request.url.path}');
17491643
});
17501644
final luci = MockLuciBuildService();
1751-
when(
1752-
luci.scheduleTryBuilds(
1753-
targets: anyNamed('targets'),
1754-
pullRequest: anyNamed('pullRequest'),
1755-
flutterPrebuiltEngineVersion: anyNamed('flutterPrebuiltEngineVersion'),
1756-
),
1757-
).thenAnswer((inv) async {
1645+
when(luci.scheduleTryBuilds(targets: anyNamed('targets'), pullRequest: anyNamed('pullRequest')))
1646+
.thenAnswer((inv) async {
17581647
return [];
17591648
});
17601649

@@ -1847,7 +1736,6 @@ targets:
18471736
luci.scheduleTryBuilds(
18481737
targets: captureAnyNamed('targets'),
18491738
pullRequest: captureAnyNamed('pullRequest'),
1850-
flutterPrebuiltEngineVersion: anyNamed('flutterPrebuiltEngineVersion'),
18511739
),
18521740
);
18531741
expect(result.callCount, 1);
@@ -2603,13 +2491,8 @@ targets:
26032491
throw Exception('Failed to find ${request.url.path}');
26042492
});
26052493
final luci = MockLuciBuildService();
2606-
when(
2607-
luci.scheduleTryBuilds(
2608-
targets: anyNamed('targets'),
2609-
pullRequest: anyNamed('pullRequest'),
2610-
flutterPrebuiltEngineVersion: anyNamed('flutterPrebuiltEngineVersion'),
2611-
),
2612-
).thenAnswer((inv) async {
2494+
when(luci.scheduleTryBuilds(targets: anyNamed('targets'), pullRequest: anyNamed('pullRequest')))
2495+
.thenAnswer((inv) async {
26132496
return [];
26142497
});
26152498
final MockGithubService mockGithubService = MockGithubService();
@@ -2670,7 +2553,6 @@ targets:
26702553
luci.scheduleTryBuilds(
26712554
targets: captureAnyNamed('targets'),
26722555
pullRequest: anyNamed('pullRequest'),
2673-
flutterPrebuiltEngineVersion: anyNamed('flutterPrebuiltEngineVersion'),
26742556
),
26752557
);
26762558
expect(result.callCount, 1);
@@ -2731,13 +2613,8 @@ targets:
27312613
'Linux engine_build',
27322614
};
27332615
});
2734-
when(
2735-
luci.scheduleTryBuilds(
2736-
targets: anyNamed('targets'),
2737-
pullRequest: anyNamed('pullRequest'),
2738-
flutterPrebuiltEngineVersion: anyNamed('flutterPrebuiltEngineVersion'),
2739-
),
2740-
).thenAnswer((inv) async {
2616+
when(luci.scheduleTryBuilds(targets: anyNamed('targets'), pullRequest: anyNamed('pullRequest')))
2617+
.thenAnswer((inv) async {
27412618
return [];
27422619
});
27432620
final MockGithubService mockGithubService = MockGithubService();
@@ -2851,13 +2728,8 @@ targets:
28512728
'Mac engine_build',
28522729
};
28532730
});
2854-
when(
2855-
luci.scheduleTryBuilds(
2856-
targets: anyNamed('targets'),
2857-
pullRequest: anyNamed('pullRequest'),
2858-
flutterPrebuiltEngineVersion: anyNamed('flutterPrebuiltEngineVersion'),
2859-
),
2860-
).thenAnswer((inv) async {
2731+
when(luci.scheduleTryBuilds(targets: anyNamed('targets'), pullRequest: anyNamed('pullRequest')))
2732+
.thenAnswer((inv) async {
28612733
return [];
28622734
});
28632735
final MockGithubService mockGithubService = MockGithubService();
@@ -3247,8 +3119,8 @@ targets:
32473119
await scheduler.triggerPresubmitTargets(pullRequest: pullRequest);
32483120
expect(
32493121
fakeLuciBuildService.flutterPrebuiltEngineVersion,
3250-
isNull,
3251-
reason: 'When scheduling engine builds, there is no concept of an engine prebuilt.',
3122+
pullRequest.base!.sha,
3123+
reason: 'Should use the base ref for the engine artifacts',
32523124
);
32533125
expect(
32543126
fakeLuciBuildService.scheduledTryBuilds.map((t) => t.value.name),
@@ -3261,8 +3133,8 @@ targets:
32613133
}
32623134

32633135
final class _CapturingFakeLuciBuildService extends Fake implements LuciBuildService {
3264-
List<Target> scheduledTryBuilds = [];
3265-
String? flutterPrebuiltEngineVersion;
3136+
late List<Target> scheduledTryBuilds;
3137+
late String? flutterPrebuiltEngineVersion;
32663138

32673139
@override
32683140
Future<List<Target>> scheduleTryBuilds({

0 commit comments

Comments
 (0)