Special case Linux analyze for CHANGELOG.md (#4521)

Closes a loop hole found in
flutter/flutter#167225.

---------

Co-authored-by: John McDole <codefu@google.com>

Author: matanlurey

app_dart/lib/src/service/scheduler.dart

@@ -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'),
@@ -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,
           )) {
@@ -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,
         );
 
@@ -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!,
@@ -1682,5 +1697,5 @@ enum _FlutterRepoTestsToRun {
   frameworkTestsOnly,
 
   /// No tests.
-  noTests,
+  frameworkFlutterAnalyzeOnly,
 }

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

@@ -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/')) {
@@ -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;

app_dart/test/service/scheduler/files_changed_optimization_test.dart

@@ -131,7 +131,7 @@ void main() {
       optimizer.checkPullRequest(
         generatePullRequest(repo: 'flutter', changedFilesCount: 1, number: 123),
       ),
-      completion(FilesChangedOptimization.skipPresubmitAll),
+      completion(FilesChangedOptimization.skipPresubmitAllExceptFlutterAnalyze),
     );
   });
 

app_dart/test/service/scheduler_test.dart

@@ -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
@@ -3403,7 +3425,10 @@ targets:
       setUp(() {
         mockGithubService = MockGithubService();
         fakeLuciBuildService = _CapturingFakeLuciBuildService();
-        ciYamlFetcher.setCiYamlFrom(singleCiYaml, engine: fusionCiYaml);
+        ciYamlFetcher.setCiYamlFrom(
+          singleCiYamlWithLinuxAnalyze,
+          engine: fusionCiYaml,
+        );
 
         scheduler = Scheduler(
           cache: cache,
@@ -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
@@ -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',
         );
       });
 

test_utilities/bin/dart_test_runner.sh

@@ -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