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
37 changes: 26 additions & 11 deletions app_dart/lib/src/service/scheduler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -472,8 +472,8 @@ class Scheduler {

// The if-branch already skips the engine build phase.
testsToRun: switch (opt) {
FilesChangedOptimization.skipPresubmitAll =>
_FlutterRepoTestsToRun.noTests,
FilesChangedOptimization.skipPresubmitAllExceptFlutterAnalyze =>
_FlutterRepoTestsToRun.frameworkFlutterAnalyzeOnly,
FilesChangedOptimization.skipPresubmitEngine =>
_FlutterRepoTestsToRun.frameworkTestsOnly,
FilesChangedOptimization.none => throw StateError('Unreachable'),
Expand Down Expand Up @@ -1228,11 +1228,7 @@ $s
try {
// Both the author and label should be checked to make sure that no one is
// attempting to get a pull request without check through.
if (testsToRun == _FlutterRepoTestsToRun.noTests) {
log.info(
'$logCrumb: skipping generating the full set of checks: no tests required.',
);
} else if (pullRequest.user!.login == _config.autosubmitBot &&
if (pullRequest.user!.login == _config.autosubmitBot &&
pullRequest.labels!.any(
(element) => element.name == Config.revertOfLabel,
)) {
Expand All @@ -1242,20 +1238,39 @@ $s
} else {
// Schedule the tests that would have run in a call to triggerPresubmitTargets - but for both the
// engine and the framework.
final presubmitTargets = await _getTestsForStage(
var presubmitTargets = await _getTestsForStage(
pullRequest,
CiStage.fusionTests,
skipEngine:
testsToRun != _FlutterRepoTestsToRun.engineTestsAndFrameworkTests,
);

// Create the document for tracking test check runs.
final List<String> tasks;
if (testsToRun == _FlutterRepoTestsToRun.frameworkFlutterAnalyzeOnly) {
const linuxAnalyze = 'Linux analyze';
final singleTarget = presubmitTargets.firstWhereOrNull(
(t) => t.name == linuxAnalyze,
);
if (singleTarget == null) {
log.warn('No target found named "$linuxAnalyze"');
tasks = [];
presubmitTargets = [];
} else {
log.info('Only running target "$linuxAnalyze"');
tasks = [linuxAnalyze];
presubmitTargets = [singleTarget];
}
} else {
tasks = [...presubmitTargets.map((t) => t.name)];
}

await initializeCiStagingDocument(
firestoreService: await _config.createFirestoreService(),
slug: pullRequest.base!.repo!.slug(),
sha: pullRequest.head!.sha!,
stage: CiStage.fusionTests,
tasks: [...presubmitTargets.map((t) => t.name)],
tasks: tasks,
checkRunGuard: checkRunGuard,
);

Expand All @@ -1267,7 +1282,7 @@ $s
// from source and rely on remote-build execution (RBE) for builds to
// fast and cached.
final EngineArtifacts engineArtifacts;
if (testsToRun == _FlutterRepoTestsToRun.frameworkTestsOnly) {
if (testsToRun != _FlutterRepoTestsToRun.engineTestsAndFrameworkTests) {
// Use the engine that this PR was branched off of.
engineArtifacts = EngineArtifacts.usingExistingEngine(
commitSha: pullRequest.base!.sha!,
Expand Down Expand Up @@ -1682,5 +1697,5 @@ enum _FlutterRepoTestsToRun {
frameworkTestsOnly,

/// No tests.
noTests,
frameworkFlutterAnalyzeOnly,
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ final class FilesChangedOptimizer {
return FilesChangedOptimization.none;
case SuccessfulFilesChanged(:final filesChanged):
if (filesChanged.length == 1 && filesChanged.contains('CHANGELOG.md')) {
return FilesChangedOptimization.skipPresubmitAll;
return FilesChangedOptimization.skipPresubmitAllExceptFlutterAnalyze;
}
for (final file in filesChanged) {
if (file == 'DEPS' || file.startsWith('engine/')) {
Expand All @@ -98,8 +98,8 @@ enum FilesChangedOptimization {
/// Engine builds (and tests) can be skipped for this presubmit run.
skipPresubmitEngine,

/// All builds (and tests) can be skipped for this presubmit run.
skipPresubmitAll;
/// Almost all builds (and tests) can be skipped for this presubmit run.
skipPresubmitAllExceptFlutterAnalyze;

/// Whether the engine should be prebuilt.
bool get shouldUsePrebuiltEngine => !shouldBuildEngineFromSource;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ void main() {
optimizer.checkPullRequest(
generatePullRequest(repo: 'flutter', changedFilesCount: 1, number: 123),
),
completion(FilesChangedOptimization.skipPresubmitAll),
completion(FilesChangedOptimization.skipPresubmitAllExceptFlutterAnalyze),
);
});

Expand Down
41 changes: 34 additions & 7 deletions app_dart/test/service/scheduler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,28 @@ targets:
scheduler: google_internal
''';

const String singleCiYamlWithLinuxAnalyze = r'''
enabled_branches:
- master
- main
- flutter-\d+\.\d+-candidate\.\d+
targets:
- name: Linux A
properties:
custom: abc
- name: Linux B
enabled_branches:
- stable
scheduler: luci
- name: Linux runIf
runIf:
- .ci.yaml
- DEPS
- dev/**
- engine/**
- name: Linux analyze
''';

const String fusionCiYaml = r'''
enabled_branches:
- master
Expand Down Expand Up @@ -3403,7 +3425,10 @@ targets:
setUp(() {
mockGithubService = MockGithubService();
fakeLuciBuildService = _CapturingFakeLuciBuildService();
ciYamlFetcher.setCiYamlFrom(singleCiYaml, engine: fusionCiYaml);
ciYamlFetcher.setCiYamlFrom(
singleCiYamlWithLinuxAnalyze,
engine: fusionCiYaml,
);

scheduler = Scheduler(
cache: cache,
Expand Down Expand Up @@ -3492,7 +3517,7 @@ targets:
);
expect(
fakeLuciBuildService.scheduledTryBuilds.map((t) => t.name),
['Linux A'],
['Linux A', 'Linux analyze'],
reason: 'Should skip Linux engine_build',
);
// TODO(matanlurey): Refactoring should allow us to verify the first stage
Expand All @@ -3501,20 +3526,22 @@ targets:
});

// Regression test for https://github.com/flutter/flutter/issues/167124.
test('skips all builds', () async {
test('skips all tests except "Linux analyze"', () async {
getFilesChanged.cannedFiles = ['CHANGELOG.md'];
final pullRequest = generatePullRequest(authorLogin: allowListedUser);

await scheduler.triggerPresubmitTargets(pullRequest: pullRequest);
expect(
fakeLuciBuildService.engineArtifacts,
isNull,
reason: 'Did not schdule tests',
EngineArtifacts.usingExistingEngine(
commitSha: pullRequest.base!.sha!,
),
reason: 'Should use the base ref for the engine artifacts',
);
expect(
fakeLuciBuildService.scheduledTryBuilds.map((t) => t.name),
isEmpty,
reason: 'Did not schdule tests',
['Linux analyze'],
reason: 'Only scheduled a special-cased build',
);
});

Expand Down
9 changes: 5 additions & 4 deletions test_utilities/bin/dart_test_runner.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ dart format --set-exit-if-changed .
echo "########### analyze ###########"
dart analyze --fatal-infos

# TODO(matanlurey): Re-enable (https://github.com/flutter/flutter/issues/167229).
# agent doesn't use build_runner as of this writing.
if grep -lq "build_runner" pubspec.yaml; then
echo "############# build ###########"
dart run build_runner build --delete-conflicting-outputs
fi
# if grep -lq "build_runner" pubspec.yaml; then
# echo "############# build ###########"
# dart run build_runner build --delete-conflicting-outputs
# fi

# Only try tests if test folder exist.
if [ -d 'test' ]; then
Expand Down