Auto merge of #384 - pietroalbini:ignore-blacklist, r=pietroalbini

Add a flag to ignore the blacklist for a run

This PR adds a flag to ignore the blacklist only for a run. The flag will be useful when maintenance runs are done, for example when we regenerate the list of automatically skipped crates.

It also adds minicrater runs with the blacklist and with the blacklist ignored.

Author: bors

docs/bot-usage.md

@@ -115,6 +115,7 @@ beta run you can use:
 * `mode`: the experiment mode (default: `build-and-test`)
 * `crates`: the selection of crates to use (default: `full`)
 * `cap-lints`: the lints cap (default: `forbid`, which means no cap)
+* `ignore-blacklist`: whether the blacklist should be ignored (default: `false`)
 * `p`: the priority of the run (default: `0`)
 
 [Go back to the TOC][h-toc]
@@ -140,6 +141,7 @@ priority of the `foo` experiment you can use:
 * `mode`: the experiment mode (default: `build-and-test`)
 * `crates`: the selection of crates to use (default: `full`)
 * `cap-lints`: the lints cap (default: `forbid`, which means no cap)
+* `ignore-blacklist`: whether the blacklist should be ignored (default: `false`)
 * `p`: the priority of the run (default: `0`)
 
 [Go back to the TOC][h-toc]

src/actions/experiments/create.rs

@@ -14,6 +14,7 @@ pub struct CreateExperiment {
     pub cap_lints: CapLints,
     pub priority: i32,
     pub github_issue: Option<GitHubIssue>,
+    pub ignore_blacklist: bool,
 }
 
 impl CreateExperiment {
@@ -29,6 +30,7 @@ impl CreateExperiment {
             cap_lints: CapLints::Forbid,
             priority: 0,
             github_issue: None,
+            ignore_blacklist: false,
         }
     }
 
@@ -49,8 +51,8 @@ impl CreateExperiment {
             transaction.execute(
                 "INSERT INTO experiments \
                  (name, mode, cap_lints, toolchain_start, toolchain_end, priority, created_at, \
-                 status, github_issue, github_issue_url, github_issue_number) \
-                 VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11);",
+                 status, github_issue, github_issue_url, github_issue_number, ignore_blacklist) \
+                 VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12);",
                 &[
                     &self.name,
                     &self.mode.to_str(),
@@ -63,11 +65,12 @@ impl CreateExperiment {
                     &self.github_issue.as_ref().map(|i| i.api_url.as_str()),
                     &self.github_issue.as_ref().map(|i| i.html_url.as_str()),
                     &self.github_issue.as_ref().map(|i| i.number),
+                    &self.ignore_blacklist,
                 ],
             )?;
 
             for krate in &crates {
-                let skipped = config.should_skip(krate) as i32;
+                let skipped = !self.ignore_blacklist && config.should_skip(krate);
                 transaction.execute(
                     "INSERT INTO experiment_crates (experiment, crate, skipped) VALUES (?1, ?2, ?3);",
                     &[&self.name, &::serde_json::to_string(&krate)?, &skipped],
@@ -85,8 +88,9 @@ impl CreateExperiment {
 mod tests {
     use super::CreateExperiment;
     use crate::actions::ExperimentError;
-    use crate::config::Config;
-    use crate::db::Database;
+    use crate::config::{Config, CrateConfig};
+    use crate::crates::Crate;
+    use crate::db::{Database, QueryUtils};
     use crate::experiments::{CapLints, CrateSelect, Experiment, GitHubIssue, Mode, Status};
     use crate::toolchain::{MAIN_TOOLCHAIN, TEST_TOOLCHAIN};
 
@@ -112,6 +116,7 @@ mod tests {
                 html_url: html_url.to_string(),
                 number: 10,
             }),
+            ignore_blacklist: true,
         }
         .apply(&db, &config)
         .unwrap();
@@ -137,6 +142,64 @@ mod tests {
         assert_eq!(ex.priority, 5);
         assert_eq!(ex.status, Status::Queued);
         assert!(ex.assigned_to.is_none());
+        assert!(ex.ignore_blacklist);
+    }
+
+    #[test]
+    fn test_ignore_blacklist() {
+        fn is_skipped(db: &Database, ex: &str, krate: &str) -> bool {
+            let crates: Vec<Crate> = db
+                .query(
+                    "SELECT crate FROM experiment_crates WHERE experiment = ?1 AND skipped = 0",
+                    &[&ex],
+                    |row| {
+                        let krate: String = row.get("crate");
+                        serde_json::from_str(&krate).unwrap()
+                    },
+                )
+                .unwrap();
+            crates
+                .iter()
+                .find(|c| {
+                    if let Crate::Local(name) = c {
+                        name == krate
+                    } else {
+                        panic!("there should be no non-local crates")
+                    }
+                })
+                .is_none()
+        }
+
+        let db = Database::temp().unwrap();
+        let mut config = Config::default();
+        config.local_crates.insert(
+            "build-pass".into(),
+            CrateConfig {
+                skip: true,
+                skip_tests: false,
+                quiet: false,
+                update_lockfile: false,
+                broken: false,
+            },
+        );
+
+        crate::crates::lists::setup_test_lists(&db, &config).unwrap();
+
+        CreateExperiment {
+            ignore_blacklist: false,
+            ..CreateExperiment::dummy("foo")
+        }
+        .apply(&db, &config)
+        .unwrap();
+        assert!(is_skipped(&db, "foo", "build-pass"));
+
+        CreateExperiment {
+            ignore_blacklist: true,
+            ..CreateExperiment::dummy("bar")
+        }
+        .apply(&db, &config)
+        .unwrap();
+        assert!(!is_skipped(&db, "bar", "build-pass"));
     }
 
     #[test]
@@ -155,6 +218,7 @@ mod tests {
             cap_lints: CapLints::Forbid,
             priority: 0,
             github_issue: None,
+            ignore_blacklist: false,
         }
         .apply(&db, &config)
         .unwrap_err();
@@ -181,6 +245,7 @@ mod tests {
             cap_lints: CapLints::Forbid,
             priority: 0,
             github_issue: None,
+            ignore_blacklist: false,
         }
         .apply(&db, &config)
         .unwrap();
@@ -194,6 +259,7 @@ mod tests {
             cap_lints: CapLints::Forbid,
             priority: 0,
             github_issue: None,
+            ignore_blacklist: false,
         }
         .apply(&db, &config)
         .unwrap_err();

src/actions/experiments/edit.rs

@@ -12,6 +12,7 @@ pub struct EditExperiment {
     pub mode: Option<Mode>,
     pub cap_lints: Option<CapLints>,
     pub priority: Option<i32>,
+    pub ignore_blacklist: Option<bool>,
 }
 
 impl EditExperiment {
@@ -24,6 +25,7 @@ impl EditExperiment {
             crates: None,
             cap_lints: None,
             priority: None,
+            ignore_blacklist: None,
         }
     }
 
@@ -61,10 +63,29 @@ impl EditExperiment {
                 }
             }
 
-            // Try to update the list of crates
-            if let Some(crates) = self.crates {
-                let crates_vec = crate::crates::lists::get_crates(crates, db, config)?;
+            // Try to update the ignore_blacklist field
+            // The list of skipped crates will be recalculated afterwards
+            if let Some(ignore_blacklist) = self.ignore_blacklist {
+                let changes = t.execute(
+                    "UPDATE experiments SET ignore_blacklist = ?1 WHERE name = ?2;",
+                    &[&ignore_blacklist, &self.name],
+                )?;
+                assert_eq!(changes, 1);
 
+                ex.ignore_blacklist = ignore_blacklist;
+                has_changed = true;
+            }
+
+            // Try to update the list of crates
+            // This is also done if ignore_blacklist is changed to recalculate the skipped crates
+            let new_crates = if let Some(crates) = self.crates {
+                Some(crate::crates::lists::get_crates(crates, db, config)?)
+            } else if self.ignore_blacklist.is_some() {
+                Some(ex.crates.clone())
+            } else {
+                None
+            };
+            if let Some(crates_vec) = new_crates {
                 // Recreate the list of crates without checking if it was the same
                 // This is done to allow reloading the list of crates in an existing experiment
                 t.execute(
@@ -78,7 +99,7 @@ impl EditExperiment {
                         &[
                             &self.name,
                             &::serde_json::to_string(&krate)?,
-                            &config.should_skip(krate),
+                            &(!ex.ignore_blacklist && config.should_skip(krate)),
                         ],
                     )?;
                 }
@@ -132,8 +153,9 @@ impl EditExperiment {
 mod tests {
     use super::EditExperiment;
     use crate::actions::{CreateExperiment, ExperimentError};
-    use crate::config::Config;
-    use crate::db::Database;
+    use crate::config::{Config, CrateConfig};
+    use crate::crates::Crate;
+    use crate::db::{Database, QueryUtils};
     use crate::experiments::{CapLints, CrateSelect, Experiment, Mode, Status};
     use crate::toolchain::{MAIN_TOOLCHAIN, TEST_TOOLCHAIN};
 
@@ -168,6 +190,7 @@ mod tests {
             cap_lints: CapLints::Forbid,
             priority: 0,
             github_issue: None,
+            ignore_blacklist: false,
         }
         .apply(&db, &config)
         .unwrap();
@@ -183,6 +206,7 @@ mod tests {
             crates: Some(CrateSelect::Local),
             cap_lints: Some(CapLints::Warn),
             priority: Some(10),
+            ignore_blacklist: Some(true),
         }
         .apply(&db, &config)
         .unwrap();
@@ -195,13 +219,79 @@ mod tests {
         assert_eq!(ex.mode, Mode::CheckOnly);
         assert_eq!(ex.cap_lints, CapLints::Warn);
         assert_eq!(ex.priority, 10);
+        assert_eq!(ex.ignore_blacklist, true);
 
         assert_eq!(
             ex.crates,
             crate::crates::lists::get_crates(CrateSelect::Local, &db, &config).unwrap()
         );
     }
 
+    #[test]
+    fn test_ignore_blacklist() {
+        fn is_skipped(db: &Database, ex: &str, krate: &str) -> bool {
+            let crates: Vec<Crate> = db
+                .query(
+                    "SELECT crate FROM experiment_crates WHERE experiment = ?1 AND skipped = 0",
+                    &[&ex],
+                    |row| {
+                        let krate: String = row.get("crate");
+                        serde_json::from_str(&krate).unwrap()
+                    },
+                )
+                .unwrap();
+            crates
+                .iter()
+                .find(|c| {
+                    if let Crate::Local(name) = c {
+                        name == krate
+                    } else {
+                        panic!("there should be no non-local crates")
+                    }
+                })
+                .is_none()
+        }
+
+        let db = Database::temp().unwrap();
+        let mut config = Config::default();
+        config.local_crates.insert(
+            "build-pass".into(),
+            CrateConfig {
+                skip: true,
+                skip_tests: false,
+                quiet: false,
+                update_lockfile: false,
+                broken: false,
+            },
+        );
+
+        crate::crates::lists::setup_test_lists(&db, &config).unwrap();
+
+        CreateExperiment {
+            ignore_blacklist: false,
+            ..CreateExperiment::dummy("foo")
+        }
+        .apply(&db, &config)
+        .unwrap();
+        assert!(is_skipped(&db, "foo", "build-pass"));
+
+        EditExperiment {
+            ignore_blacklist: Some(true),
+            ..EditExperiment::dummy("foo")
+        }
+        .apply(&db, &config)
+        .unwrap();
+        assert!(!is_skipped(&db, "foo", "build-pass"));
+
+        EditExperiment {
+            ignore_blacklist: Some(false),
+            ..EditExperiment::dummy("foo")
+        }
+        .apply(&db, &config)
+        .unwrap();
+        assert!(is_skipped(&db, "foo", "build-pass"));
+    }
+
     #[test]
     fn test_duplicate_toolchains() {
         let db = Database::temp().unwrap();