From 61aee79c4cac47f23f1e79574f37721cc995e5bf Mon Sep 17 00:00:00 2001
From: Casey Hillers <chillers@google.com>
Date: Tue, 12 Dec 2023 09:32:18 -0800
Subject: [PATCH] [scheduler] Verify runIf includes foundational files (#3333)

Occasionally bugs come in where targets should run, but the runIf is missing critical files (like ci.yaml). Adding this assertion here, and will update the downstream ci.yamls to be fixed.

Fixed the integration test to use the ci_yaml.dart validation suite.
---
 app_dart/integration_test/common.dart         |  2 +-
 .../validate_all_ci_configs_test.dart         | 19 ++++++++++---------
 app_dart/lib/src/model/ci_yaml/ci_yaml.dart   | 10 ++++++++++
 app_dart/test/service/scheduler_test.dart     |  3 +++
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/app_dart/integration_test/common.dart b/app_dart/integration_test/common.dart
index a2668516a..92e3f45e7 100644
--- a/app_dart/integration_test/common.dart
+++ b/app_dart/integration_test/common.dart
@@ -18,7 +18,7 @@ void expectNoDiff(String path) {
 
 /// Wrapper class to make it easy to add new repos + branches to the validation suite.
 class SupportedConfig {
-  SupportedConfig(this.slug, [this.branch = 'master']);
+  SupportedConfig(this.slug, [this.branch = 'main']);
 
   final RepositorySlug slug;
   final String branch;
diff --git a/app_dart/integration_test/validate_all_ci_configs_test.dart b/app_dart/integration_test/validate_all_ci_configs_test.dart
index 2032f5606..a5f79d683 100644
--- a/app_dart/integration_test/validate_all_ci_configs_test.dart
+++ b/app_dart/integration_test/validate_all_ci_configs_test.dart
@@ -18,10 +18,10 @@ import 'common.dart';
 
 /// List of repositories that have supported .ci.yaml config files.
 final List<SupportedConfig> configs = <SupportedConfig>[
-  SupportedConfig(RepositorySlug('flutter', 'cocoon'), 'main'),
-  SupportedConfig(RepositorySlug('flutter', 'engine'), 'main'),
+  SupportedConfig(RepositorySlug('flutter', 'cocoon')),
+  SupportedConfig(RepositorySlug('flutter', 'engine')),
   SupportedConfig(RepositorySlug('flutter', 'flutter')),
-  SupportedConfig(RepositorySlug('flutter', 'packages'), 'main'),
+  SupportedConfig(RepositorySlug('flutter', 'packages')),
 ];
 
 Future<void> main() async {
@@ -57,6 +57,13 @@ Future<void> main() async {
         );
         final YamlMap configYaml = loadYaml(configContent) as YamlMap;
         final pb.SchedulerConfig schedulerConfig = pb.SchedulerConfig()..mergeFromProto3Json(configYaml);
+        // Validate using the existing CiYaml logic.
+        CiYaml(
+          slug: config.slug,
+          branch: config.branch,
+          config: schedulerConfig,
+          validate: true,
+        );
 
         final List<String> githubBranches = getBranchesForRepository(config.slug);
 
@@ -81,11 +88,6 @@ Future<void> main() async {
           }
         }
 
-        if (config.slug.name == 'engine') {
-          print(githubBranches);
-          print(validEnabledBranches);
-        }
-
         // Verify the enabled branches
         for (String enabledBranch in validEnabledBranches.keys) {
           expect(
@@ -95,7 +97,6 @@ Future<void> main() async {
           );
         }
       },
-      skip: config.slug.name == 'flutter',
     );
   }
 }
diff --git a/app_dart/lib/src/model/ci_yaml/ci_yaml.dart b/app_dart/lib/src/model/ci_yaml/ci_yaml.dart
index 46f18321b..38cc3fda1 100644
--- a/app_dart/lib/src/model/ci_yaml/ci_yaml.dart
+++ b/app_dart/lib/src/model/ci_yaml/ci_yaml.dart
@@ -204,6 +204,7 @@ class CiYaml {
   ///   5. [pb.Target] should not depend on self
   ///   6. [pb.Target] cannot have more than 1 dependency
   ///   7. [pb.Target] should depend on target that already exist in depedency graph, and already recorded in map [targetGraph]
+  ///   8. [pb.Target] has an empty runIf or the runIf includes `.ci.yaml` and `DEPS if on the engine repo.
   void _validate(pb.SchedulerConfig schedulerConfig, String branch, {pb.SchedulerConfig? totSchedulerConfig}) {
     if (schedulerConfig.targets.isEmpty) {
       throw const FormatException('Scheduler config must have at least 1 target');
@@ -251,6 +252,15 @@ class CiYaml {
             }
           }
         }
+        // Verify runIf includes foundational files.
+        if (target.runIf.isNotEmpty) {
+          if (!target.runIf.contains('.ci.yaml')) {
+            exceptions.add('ERROR: ${target.name} is missing `.ci.yaml` in runIf');
+          }
+          if (slug == Config.engineSlug && !target.runIf.contains('DEPS')) {
+            exceptions.add('ERROR: ${target.name} is missing `DEPS` in runIf');
+          }
+        }
       }
 
       /// Check the dependencies for the current target if it is viable and to
diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart
index fdc460416..53dd4b0be 100644
--- a/app_dart/test/service/scheduler_test.dart
+++ b/app_dart/test/service/scheduler_test.dart
@@ -55,6 +55,7 @@ targets:
     scheduler: luci
   - name: Linux runIf
     runIf:
+      - .ci.yaml
       - dev/**
   - name: Google Internal Roll
     postsubmit: true
@@ -708,6 +709,8 @@ targets:
       presubmit: true
       scheduler: luci
       runIf:
+        - .ci.yaml
+        - DEPS
         - dev/run_if/**
     - name: Linux Conditional Presubmit (runIfNot)
       presubmit: true