DAG-1987: Remove _misc sub-task creation for generated tasks (#50)

zamj · web-flow · commit a6b5ab1916fe · 2022-10-10T15:14:09.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+## 0.6.3 - 2022-10-07
+* Remove _misc task generation.
+
 ## 0.6.2 - 2022-09-14
 * Propogate up errors when calling resmoke.
 
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "mongo-task-generator"
-version = "0.6.2"
+version = "0.6.3"
 repository = "https://github.com/mongodb/mongo-task-generator"
 authors = ["Decision Automation Group <dev-prod-dag@mongodb.com>"]
 edition = "2018"
diff --git a/src/main.rs b/src/main.rs
@@ -129,7 +129,7 @@ async fn main() {
     let result = generate_configuration(&deps, &args.target_directory).await;
     event!(
         Level::INFO,
-        "generation completed: {} seconds",
+        "generation completed: {duration_secs} seconds",
         duration_secs = start.elapsed().as_secs()
     );
     if let Err(err) = result {
diff --git a/src/task_types/burn_in_tests.rs b/src/task_types/burn_in_tests.rs
@@ -235,7 +235,7 @@ impl BurnInServiceImpl {
         let origin_suite = suite_info.build_origin_suite(&params.suite_name);
 
         let sub_suite = SubSuite {
-            index: Some(index),
+            index,
             name: suite_info.build_display_name(),
             test_list: vec![test.to_string()],
             exclude_test_list: None,
diff --git a/src/task_types/fuzzer_tasks.rs b/src/task_types/fuzzer_tasks.rs
@@ -272,7 +272,7 @@ fn build_fuzzer_sub_task(
 ) -> EvgTask {
     let sub_task_name = name_generated_task(
         display_name,
-        Some(sub_task_index),
+        sub_task_index,
         params.num_tasks as usize,
         params.is_enterprise,
         params.platform.as_deref(),
diff --git a/src/task_types/resmoke_config_writer.rs b/src/task_types/resmoke_config_writer.rs
@@ -121,9 +121,6 @@ impl WriteConfigActorImpl {
         // Create suite files for all the sub-suites.
         self.write_sub_suites(&suite_info.sub_suites, &mut resmoke_config_cache)?;
 
-        // Create a suite file for the '_misc' sub-task.
-        self.write_misc_suites(&suite_info.sub_suites, &mut resmoke_config_cache)?;
-
         Ok(())
     }
 
@@ -166,46 +163,6 @@ impl WriteConfigActorImpl {
         results?;
         Ok(())
     }
-
-    /// Write resmoke configurations for a "_misc" suite.
-    ///
-    /// # Arguments
-    ///
-    /// * `sub_suites` - List of sub-suites comprising the generated suite.
-    /// * `resmoke_config_cache` - Cache to get resmoke suite configurations.
-    fn write_misc_suites(
-        &self,
-        sub_suites: &[SubSuite],
-        resmoke_config_cache: &mut ResmokeConfigCache,
-    ) -> Result<()> {
-        let total_tasks = sub_suites.len();
-        let results: Result<Vec<()>> = sub_suites
-            .iter()
-            .filter(|s| s.exclude_test_list.is_some())
-            .map(|s| {
-                let origin_config = resmoke_config_cache.get_config(&s.origin_suite)?;
-                let test_list = s.exclude_test_list.clone().unwrap();
-                let misc_config = origin_config.with_new_tests(None, Some(&test_list));
-                let filename = format!(
-                    "{}.yml",
-                    name_generated_task(
-                        &s.name,
-                        s.index,
-                        total_tasks,
-                        s.is_enterprise,
-                        s.platform.as_deref()
-                    )
-                );
-                let mut path = PathBuf::from(&self.target_dir);
-                path.push(filename);
-                self.fs_service
-                    .write_file(&path, &misc_config.to_string())?;
-                Ok(())
-            })
-            .collect();
-        results?;
-        Ok(())
-    }
 }
 
 #[async_trait]
@@ -446,35 +403,26 @@ mod tests {
             generate_multiversion_combos: false,
             sub_suites: vec![
                 SubSuite {
-                    index: Some(0),
+                    index: 0,
                     name: "suite_name".to_string(),
                     origin_suite: "suite".to_string(),
                     test_list: vec!["test_0.js".to_string(), "test_1.js".to_string()],
                     ..Default::default()
                 },
                 SubSuite {
-                    index: Some(1),
+                    index: 1,
                     name: "suite_name".to_string(),
                     origin_suite: "suite".to_string(),
                     test_list: vec!["test_2.js".to_string(), "test_3.js".to_string()],
                     ..Default::default()
                 },
-                SubSuite {
-                    index: None,
-                    name: "suite_name".to_string(),
-                    origin_suite: "suite".to_string(),
-                    test_list: vec![],
-                    exclude_test_list: Some((0..4).map(|i| format!("test_{}.js", i)).collect()),
-                    ..Default::default()
-                },
             ],
         };
 
         resmoke_config_actor.write_suite_files(suite_info);
 
         assert_eq!(fs_service.get_call_counts("target/suite_name_0.yml"), 1);
         assert_eq!(fs_service.get_call_counts("target/suite_name_1.yml"), 1);
-        assert_eq!(fs_service.get_call_counts("target/suite_name_misc.yml"), 1);
     }
 
     #[tokio::test]
@@ -489,14 +437,14 @@ mod tests {
             generate_multiversion_combos: false,
             sub_suites: vec![
                 SubSuite {
-                    index: Some(0),
+                    index: 0,
                     name: "suite".to_string(),
                     origin_suite: "suite".to_string(),
                     test_list: vec!["test_0.js".to_string(), "test_1.js".to_string()],
                     ..Default::default()
                 },
                 SubSuite {
-                    index: Some(1),
+                    index: 1,
                     name: "suite".to_string(),
                     origin_suite: "suite".to_string(),
                     test_list: vec!["test_2.js".to_string(), "test_3.js".to_string()],
diff --git a/src/task_types/resmoke_tasks.rs b/src/task_types/resmoke_tasks.rs
@@ -3,10 +3,7 @@
 //! This service will query the historic runtime of tests in the given task and then
 //! use that information to divide the tests into sub-suites that can be run in parallel.
 //!
-//! Each task will contain the generated sub-suites and a '_misc' suite. The '_misc' suite
-//! tries to run all the tests for the original suite minus tests that were added to generated
-//! suites. This catches tests that were not included in the historic runtime data. For example,
-//! newly added tests that have not yet be run.
+//! Each task will contain the generated sub-suites.
 use std::{cmp::min, collections::HashMap, sync::Arc};
 
 use anyhow::Result;
@@ -181,8 +178,8 @@ impl ResmokeGenParams {
 /// Representation of generated sub-suite.
 #[derive(Clone, Debug, Default)]
 pub struct SubSuite {
-    /// Index value of generated suite (None for the '_misc' sub-task).
-    pub index: Option<usize>,
+    /// Index value of generated suite.
+    pub index: usize,
 
     /// Name of generated sub-suite.
     pub name: String,
@@ -443,7 +440,7 @@ impl GenResmokeTaskServiceImpl {
         let mut sub_suites = vec![];
         for (i, slice) in running_tests.iter().enumerate() {
             sub_suites.push(SubSuite {
-                index: Some(i),
+                index: i,
                 name: multiversion_name.unwrap_or(&params.task_name).to_string(),
                 test_list: slice.clone(),
                 origin_suite: origin_suite.to_string(),
@@ -481,10 +478,7 @@ impl GenResmokeTaskServiceImpl {
 
         if !params.is_enterprise {
             if let Some(enterprise_dir) = &self.config.enterprise_dir {
-                test_list = test_list
-                    .into_iter()
-                    .filter(|s| !s.starts_with(enterprise_dir))
-                    .collect();
+                test_list.retain(|s| !s.starts_with(enterprise_dir));
             }
         }
 
@@ -525,7 +519,7 @@ impl GenResmokeTaskServiceImpl {
             current_tests.push(test);
             if current_tests.len() >= tasks_per_suite {
                 sub_suites.push(SubSuite {
-                    index: Some(i),
+                    index: i,
                     name: multiversion_name.unwrap_or(&params.task_name).to_string(),
                     test_list: current_tests,
                     origin_suite: origin_suite.to_string(),
@@ -541,7 +535,7 @@ impl GenResmokeTaskServiceImpl {
 
         if !current_tests.is_empty() {
             sub_suites.push(SubSuite {
-                index: Some(i),
+                index: i,
                 name: multiversion_name.unwrap_or(&params.task_name).to_string(),
                 test_list: current_tests,
                 origin_suite: origin_suite.to_string(),
@@ -614,8 +608,7 @@ impl GenResmokeTaskServiceImpl {
         multiversion_name: Option<&str>,
         multiversion_tags: Option<String>,
     ) -> Result<Vec<SubSuite>> {
-        let origin_suite = multiversion_name.unwrap_or(&params.suite_name);
-        let mut sub_suites = if self.config.use_task_split_fallback {
+        let sub_suites = if self.config.use_task_split_fallback {
             self.split_task_fallback(params, multiversion_name, multiversion_tags.clone())?
         } else {
             let task_history = self
@@ -643,22 +636,6 @@ impl GenResmokeTaskServiceImpl {
             }
         };
 
-        // Add a `_misc` sub-task to the list of tasks.
-        let full_test_list: Vec<String> = sub_suites
-            .iter()
-            .flat_map(|s| s.test_list.clone())
-            .collect();
-        sub_suites.push(SubSuite {
-            index: None,
-            name: multiversion_name.unwrap_or(&params.task_name).to_string(),
-            test_list: vec![],
-            origin_suite: origin_suite.to_string(),
-            exclude_test_list: Some(full_test_list),
-            mv_exclude_tags: multiversion_tags,
-            is_enterprise: params.is_enterprise,
-            platform: params.platform.clone(),
-        });
-
         Ok(sub_suites)
     }
 }
@@ -1408,14 +1385,14 @@ mod tests {
         let version_combos = vec!["new_new_new".to_string(), "old_new_old".to_string()];
         let sub_suites = vec![
             SubSuite {
-                index: Some(0),
+                index: 0,
                 name: "suite".to_string(),
                 origin_suite: "suite".to_string(),
                 test_list: vec!["test_0.js".to_string(), "test_1.js".to_string()],
                 ..Default::default()
             },
             SubSuite {
-                index: Some(1),
+                index: 1,
                 name: "suite".to_string(),
                 origin_suite: "suite".to_string(),
                 test_list: vec!["test_2.js".to_string(), "test_3.js".to_string()],
@@ -1431,7 +1408,12 @@ mod tests {
             test_map: hashmap! {},
         };
         let gen_resmoke_service = build_mocked_service(
-            vec![],
+            vec![
+                "test_0.js".to_string(),
+                "test_1.js".to_string(),
+                "test_2.js".to_string(),
+                "test_3.js".to_string(),
+            ],
             task_history,
             1,
             old_version.clone(),
@@ -1490,7 +1472,7 @@ mod tests {
             .unwrap();
 
         assert_eq!(suite.display_name(), "my_task".to_string());
-        assert_eq!(suite.sub_tasks().len(), n_suites + 1); // +1 for _misc suite.
+        assert_eq!(suite.sub_tasks().len(), n_suites);
     }
 
     // resmoke_commands tests.
diff --git a/src/utils/task_name.rs b/src/utils/task_name.rs
@@ -14,7 +14,7 @@ const GEN_SUFFIX: &str = "_gen";
 /// * `platform` - Platform that task will run on.
 pub fn name_generated_task(
     display_name: &str,
-    sub_task_index: Option<usize>,
+    sub_task_index: usize,
     total_tasks: usize,
     is_enterprise: bool,
     platform: Option<&str>,
@@ -29,18 +29,14 @@ pub fn name_generated_task(
         suffix = format!("-{}{}", platform, suffix)
     }
 
-    if let Some(index) = sub_task_index {
-        let alignment = (total_tasks as f64).log10().ceil() as usize;
-        format!(
-            "{}_{:0fill$}{}",
-            display_name,
-            index,
-            suffix,
-            fill = alignment
-        )
-    } else {
-        format!("{}_misc{}", display_name, suffix)
-    }
+    let alignment = (total_tasks as f64).log10().ceil() as usize;
+    format!(
+        "{}_{:0fill$}{}",
+        display_name,
+        sub_task_index,
+        suffix,
+        fill = alignment
+    )
 }
 
 /// Remove the '_gen' from end of the given task name if it exists.
@@ -67,32 +63,17 @@ mod tests {
     use rstest::*;
 
     #[rstest]
-    #[case("task", Some(0), 10, false, None, "task_0")]
-    #[case("task", Some(0), 10, false, Some("linux"), "task_0-linux")]
-    #[case("task", Some(42), 1001, false, None, "task_0042")]
-    #[case("task", Some(42), 1001, false, Some("linux"), "task_0042-linux")]
-    #[case("task", None, 1001, false, None, "task_misc")]
-    #[case("task", None, 1001, false, Some("linux"), "task_misc-linux")]
-    #[case("task", None, 0, false, None, "task_misc")]
-    #[case("task", None, 0, false, Some("linux"), "task_misc-linux")]
-    #[case("task", Some(0), 10, true, None, "task_0-enterprise")]
-    #[case("task", Some(0), 10, true, Some("linux"), "task_0-linux-enterprise")]
-    #[case("task", Some(42), 1001, true, None, "task_0042-enterprise")]
-    #[case(
-        "task",
-        Some(42),
-        1001,
-        true,
-        Some("linux"),
-        "task_0042-linux-enterprise"
-    )]
-    #[case("task", None, 1001, true, None, "task_misc-enterprise")]
-    #[case("task", None, 1001, true, Some("linux"), "task_misc-linux-enterprise")]
-    #[case("task", None, 0, true, None, "task_misc-enterprise")]
-    #[case("task", None, 0, true, Some("linux"), "task_misc-linux-enterprise")]
+    #[case("task", 0, 10, false, None, "task_0")]
+    #[case("task", 0, 10, false, Some("linux"), "task_0-linux")]
+    #[case("task", 42, 1001, false, None, "task_0042")]
+    #[case("task", 42, 1001, false, Some("linux"), "task_0042-linux")]
+    #[case("task", 0, 10, true, None, "task_0-enterprise")]
+    #[case("task", 0, 10, true, Some("linux"), "task_0-linux-enterprise")]
+    #[case("task", 42, 1001, true, None, "task_0042-enterprise")]
+    #[case("task", 42, 1001, true, Some("linux"), "task_0042-linux-enterprise")]
     fn test_name_generated_task_should_not_include_suffix(
         #[case] name: &str,
-        #[case] index: Option<usize>,
+        #[case] index: usize,
         #[case] total: usize,
         #[case] is_enterprise: bool,
         #[case] platform: Option<&str>,
diff --git a/tests/integration_test.rs b/tests/integration_test.rs
@@ -29,7 +29,7 @@ fn test_end2end_execution() {
     assert!(tmp_dir_path.exists());
 
     let files = std::fs::read_dir(tmp_dir_path).unwrap();
-    assert_eq!(2647, files.into_iter().collect::<Vec<_>>().len());
+    assert_eq!(2206, files.into_iter().collect::<Vec<_>>().len());
 }
 
 #[test]

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ fn test_end2end_execution() {`
`29`	`29`	`assert!(tmp_dir_path.exists());`
`30`	`30`
`31`	`31`	`let files = std::fs::read_dir(tmp_dir_path).unwrap();`
`32`		`- assert_eq!(2647, files.into_iter().collect::<Vec<_>>().len());`
	`32`	`+ assert_eq!(2206, files.into_iter().collect::<Vec<_>>().len());`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`#[test]`