From 8346d3b61004cb9a99cef917fbabc9cdc370f6c3 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Mon, 31 Dec 2018 13:58:31 +0100 Subject: [PATCH] actions: refactor to use a trait --- src/actions/experiments/create.rs | 37 +++++++----- src/actions/experiments/delete.rs | 26 ++++---- src/actions/experiments/edit.rs | 82 +++++++++++--------------- src/actions/lists/update.rs | 13 ++-- src/actions/mod.rs | 19 ++++++ src/cli.rs | 17 ++++-- src/crates/lists.rs | 6 +- src/experiments.rs | 7 ++- src/report/archives.rs | 6 +- src/results/db.rs | 17 +++--- src/server/agents.rs | 8 +-- src/server/routes/webhooks/commands.rs | 53 +++++++++-------- 12 files changed, 156 insertions(+), 135 deletions(-) diff --git a/src/actions/experiments/create.rs b/src/actions/experiments/create.rs index 5f4791cfd..4fc5499e4 100644 --- a/src/actions/experiments/create.rs +++ b/src/actions/experiments/create.rs @@ -1,6 +1,5 @@ -use crate::actions::experiments::ExperimentError; -use crate::config::Config; -use crate::db::{Database, QueryUtils}; +use crate::actions::{experiments::ExperimentError, Action, ActionsCtx}; +use crate::db::QueryUtils; use crate::experiments::{CapLints, CrateSelect, Experiment, GitHubIssue, Mode, Status}; use crate::prelude::*; use crate::toolchain::Toolchain; @@ -33,11 +32,13 @@ impl CreateExperiment { ignore_blacklist: false, } } +} - pub fn apply(self, db: &Database, config: &Config) -> Fallible<()> { +impl Action for CreateExperiment { + fn apply(self, ctx: &ActionsCtx) -> Fallible<()> { // Ensure no duplicate experiments are created - if Experiment::exists(db, &self.name)? { - return Err(ExperimentError::AlreadyExists(self.name).into()); + if Experiment::exists(&ctx.db, &self.name)? { + return Err(ExperimentError::AlreadyExists(self.name.clone()).into()); } // Ensure no experiment with duplicate toolchains is created @@ -45,9 +46,9 @@ impl CreateExperiment { return Err(ExperimentError::DuplicateToolchains.into()); } - let crates = crate::crates::lists::get_crates(self.crates, db, config)?; + let crates = crate::crates::lists::get_crates(self.crates, &ctx.db, &ctx.config)?; - db.transaction(|transaction| { + ctx.db.transaction(|transaction| { transaction.execute( "INSERT INTO experiments \ (name, mode, cap_lints, toolchain_start, toolchain_end, priority, created_at, \ @@ -70,7 +71,7 @@ impl CreateExperiment { )?; for krate in &crates { - let skipped = !self.ignore_blacklist && config.should_skip(krate); + let skipped = !self.ignore_blacklist && ctx.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], @@ -87,7 +88,7 @@ impl CreateExperiment { #[cfg(test)] mod tests { use super::CreateExperiment; - use crate::actions::ExperimentError; + use crate::actions::{Action, ActionsCtx, ExperimentError}; use crate::config::{Config, CrateConfig}; use crate::crates::Crate; use crate::db::{Database, QueryUtils}; @@ -98,6 +99,7 @@ mod tests { fn test_creation() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); @@ -118,7 +120,7 @@ mod tests { }), ignore_blacklist: true, } - .apply(&db, &config) + .apply(&ctx) .unwrap(); let ex = Experiment::get(&db, "foo").unwrap().unwrap(); @@ -182,6 +184,7 @@ mod tests { broken: false, }, ); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); @@ -189,7 +192,7 @@ mod tests { ignore_blacklist: false, ..CreateExperiment::dummy("foo") } - .apply(&db, &config) + .apply(&ctx) .unwrap(); assert!(is_skipped(&db, "foo", "build-pass")); @@ -197,7 +200,7 @@ mod tests { ignore_blacklist: true, ..CreateExperiment::dummy("bar") } - .apply(&db, &config) + .apply(&ctx) .unwrap(); assert!(!is_skipped(&db, "bar", "build-pass")); } @@ -206,6 +209,7 @@ mod tests { fn test_duplicate_toolchains() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); @@ -220,7 +224,7 @@ mod tests { github_issue: None, ignore_blacklist: false, } - .apply(&db, &config) + .apply(&ctx) .unwrap_err(); assert_eq!( @@ -233,6 +237,7 @@ mod tests { fn test_duplicate_name() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); @@ -247,7 +252,7 @@ mod tests { github_issue: None, ignore_blacklist: false, } - .apply(&db, &config) + .apply(&ctx) .unwrap(); // While the second one fails @@ -261,7 +266,7 @@ mod tests { github_issue: None, ignore_blacklist: false, } - .apply(&db, &config) + .apply(&ctx) .unwrap_err(); assert_eq!( diff --git a/src/actions/experiments/delete.rs b/src/actions/experiments/delete.rs index 61d8af3b6..d22ab5606 100644 --- a/src/actions/experiments/delete.rs +++ b/src/actions/experiments/delete.rs @@ -1,6 +1,5 @@ -use crate::actions::experiments::ExperimentError; -use crate::config::Config; -use crate::db::{Database, QueryUtils}; +use crate::actions::{experiments::ExperimentError, Action, ActionsCtx}; +use crate::db::QueryUtils; use crate::experiments::Experiment; use crate::prelude::*; @@ -8,15 +7,16 @@ pub struct DeleteExperiment { pub name: String, } -impl DeleteExperiment { - pub fn apply(self, db: &Database, _config: &Config) -> Fallible<()> { - if !Experiment::exists(db, &self.name)? { +impl Action for DeleteExperiment { + fn apply(self, ctx: &ActionsCtx) -> Fallible<()> { + if !Experiment::exists(&ctx.db, &self.name)? { return Err(ExperimentError::NotFound(self.name).into()); } // This will also delete all the data related to this experiment, thanks to the foreign // keys in the SQLite database - db.execute("DELETE FROM experiments WHERE name = ?1;", &[&self.name])?; + ctx.db + .execute("DELETE FROM experiments WHERE name = ?1;", &[&self.name])?; Ok(()) } @@ -25,7 +25,7 @@ impl DeleteExperiment { #[cfg(test)] mod tests { use super::DeleteExperiment; - use crate::actions::{CreateExperiment, ExperimentError}; + use crate::actions::{Action, ActionsCtx, CreateExperiment, ExperimentError}; use crate::config::Config; use crate::db::Database; use crate::experiments::Experiment; @@ -34,11 +34,12 @@ mod tests { fn test_delete_missing_experiment() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); let err = DeleteExperiment { name: "dummy".to_string(), } - .apply(&db, &config) + .apply(&ctx) .unwrap_err(); assert_eq!( @@ -51,20 +52,19 @@ mod tests { fn test_delete_experiment() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); // Create a dummy experiment and make sure it exists - CreateExperiment::dummy("dummy") - .apply(&db, &config) - .unwrap(); + CreateExperiment::dummy("dummy").apply(&ctx).unwrap(); assert!(Experiment::exists(&db, "dummy").unwrap()); // Delete it and make sure it doesn't exist anymore DeleteExperiment { name: "dummy".to_string(), } - .apply(&db, &config) + .apply(&ctx) .unwrap(); assert!(!Experiment::exists(&db, "dummy").unwrap()); } diff --git a/src/actions/experiments/edit.rs b/src/actions/experiments/edit.rs index e30fd23a1..e25de8339 100644 --- a/src/actions/experiments/edit.rs +++ b/src/actions/experiments/edit.rs @@ -1,6 +1,5 @@ -use crate::actions::experiments::ExperimentError; -use crate::config::Config; -use crate::db::{Database, QueryUtils}; +use crate::actions::{experiments::ExperimentError, Action, ActionsCtx}; +use crate::db::QueryUtils; use crate::experiments::{CapLints, CrateSelect, Experiment, Mode, Status}; use crate::prelude::*; use crate::toolchain::Toolchain; @@ -28,11 +27,13 @@ impl EditExperiment { ignore_blacklist: None, } } +} - pub fn apply(mut self, db: &Database, config: &Config) -> Fallible { - let mut ex = match Experiment::get(db, &self.name)? { +impl Action for EditExperiment { + fn apply(mut self, ctx: &ActionsCtx) -> Fallible<()> { + let mut ex = match Experiment::get(&ctx.db, &self.name)? { Some(ex) => ex, - None => return Err(ExperimentError::NotFound(self.name).into()), + None => return Err(ExperimentError::NotFound(self.name.clone()).into()), }; // Ensure no change is made to running or complete experiments @@ -40,9 +41,7 @@ impl EditExperiment { return Err(ExperimentError::CanOnlyEditQueuedExperiments.into()); } - Ok(db.transaction(|t| { - let mut has_changed = false; - + ctx.db.transaction(|t| { // Try to update both toolchains for (i, col) in ["toolchain_start", "toolchain_end"].iter().enumerate() { if let Some(tc) = self.toolchains[i].take() { @@ -58,8 +57,6 @@ impl EditExperiment { &[&ex.toolchains[i].to_string(), &self.name], )?; assert_eq!(changes, 1); - - has_changed = true; } } @@ -71,15 +68,17 @@ impl EditExperiment { &[&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)?) + Some(crate::crates::lists::get_crates( + crates, + &ctx.db, + &ctx.config, + )?) } else if self.ignore_blacklist.is_some() { Some(ex.crates.clone()) } else { @@ -99,13 +98,11 @@ impl EditExperiment { &[ &self.name, &::serde_json::to_string(&krate)?, - &(!ex.ignore_blacklist && config.should_skip(krate)), + &(!ex.ignore_blacklist && ctx.config.should_skip(krate)), ], )?; } - ex.crates = crates_vec; - has_changed = true; } // Try to update the mode @@ -115,9 +112,7 @@ impl EditExperiment { &[&mode.to_str(), &self.name], )?; assert_eq!(changes, 1); - ex.mode = mode; - has_changed = true; } // Try to update the cap_lints @@ -127,9 +122,7 @@ impl EditExperiment { &[&cap_lints.to_str(), &self.name], )?; assert_eq!(changes, 1); - ex.cap_lints = cap_lints; - has_changed = true; } // Try to update the priority @@ -139,20 +132,19 @@ impl EditExperiment { &[&priority, &self.name], )?; assert_eq!(changes, 1); - ex.priority = priority; - has_changed = true; } - Ok(has_changed) - })?) + Ok(()) + })?; + Ok(()) } } #[cfg(test)] mod tests { use super::EditExperiment; - use crate::actions::{CreateExperiment, ExperimentError}; + use crate::actions::{Action, ActionsCtx, CreateExperiment, ExperimentError}; use crate::config::{Config, CrateConfig}; use crate::crates::Crate; use crate::db::{Database, QueryUtils}; @@ -163,21 +155,19 @@ mod tests { fn test_edit_with_no_changes() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); - // Create a dummy experiment to edit - CreateExperiment::dummy("foo").apply(&db, &config).unwrap(); - - // Ensure no changes are applied when no changes are defined - let has_changed = EditExperiment::dummy("foo").apply(&db, &config).unwrap(); - assert!(!has_changed); + CreateExperiment::dummy("foo").apply(&ctx).unwrap(); + EditExperiment::dummy("foo").apply(&ctx).unwrap(); } #[test] fn test_edit_with_every_change() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); @@ -192,7 +182,7 @@ mod tests { github_issue: None, ignore_blacklist: false, } - .apply(&db, &config) + .apply(&ctx) .unwrap(); // Change everything! @@ -208,7 +198,7 @@ mod tests { priority: Some(10), ignore_blacklist: Some(true), } - .apply(&db, &config) + .apply(&ctx) .unwrap(); // And get the experiment to make sure data is changed @@ -264,6 +254,7 @@ mod tests { broken: false, }, ); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); @@ -271,7 +262,7 @@ mod tests { ignore_blacklist: false, ..CreateExperiment::dummy("foo") } - .apply(&db, &config) + .apply(&ctx) .unwrap(); assert!(is_skipped(&db, "foo", "build-pass")); @@ -279,7 +270,7 @@ mod tests { ignore_blacklist: Some(true), ..EditExperiment::dummy("foo") } - .apply(&db, &config) + .apply(&ctx) .unwrap(); assert!(!is_skipped(&db, "foo", "build-pass")); @@ -287,7 +278,7 @@ mod tests { ignore_blacklist: Some(false), ..EditExperiment::dummy("foo") } - .apply(&db, &config) + .apply(&ctx) .unwrap(); assert!(is_skipped(&db, "foo", "build-pass")); } @@ -296,19 +287,20 @@ mod tests { fn test_duplicate_toolchains() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); // First create an experiment let mut dummy = CreateExperiment::dummy("foo"); dummy.toolchains = [MAIN_TOOLCHAIN.clone(), TEST_TOOLCHAIN.clone()]; - dummy.apply(&db, &config).unwrap(); + dummy.apply(&ctx).unwrap(); // Then try to switch the second toolchain to MAIN_TOOLCHAIN let mut edit = EditExperiment::dummy("foo"); edit.toolchains[1] = Some(MAIN_TOOLCHAIN.clone()); - let err = edit.apply(&db, &config).unwrap_err(); + let err = edit.apply(&ctx).unwrap_err(); assert_eq!( err.downcast_ref(), Some(&ExperimentError::DuplicateToolchains) @@ -319,12 +311,11 @@ mod tests { fn test_editing_missing_experiment() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); - let err = EditExperiment::dummy("foo") - .apply(&db, &config) - .unwrap_err(); + let err = EditExperiment::dummy("foo").apply(&ctx).unwrap_err(); assert_eq!( err.downcast_ref(), Some(&ExperimentError::NotFound("foo".into())) @@ -335,18 +326,17 @@ mod tests { fn test_editing_running_experiment() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); // Create an experiment and set it to running - CreateExperiment::dummy("foo").apply(&db, &config).unwrap(); + CreateExperiment::dummy("foo").apply(&ctx).unwrap(); let mut ex = Experiment::get(&db, "foo").unwrap().unwrap(); ex.set_status(&db, Status::Running).unwrap(); // Try to edit it - let err = EditExperiment::dummy("foo") - .apply(&db, &config) - .unwrap_err(); + let err = EditExperiment::dummy("foo").apply(&ctx).unwrap_err(); assert_eq!( err.downcast_ref(), Some(&ExperimentError::CanOnlyEditQueuedExperiments) diff --git a/src/actions/lists/update.rs b/src/actions/lists/update.rs index 1e25971b8..5d98f835b 100644 --- a/src/actions/lists/update.rs +++ b/src/actions/lists/update.rs @@ -1,6 +1,5 @@ -use crate::config::Config; +use crate::actions::{Action, ActionsCtx}; use crate::crates::lists::{GitHubList, List, LocalList, RegistryList}; -use crate::db::Database; use crate::prelude::*; pub struct UpdateLists { @@ -19,21 +18,21 @@ impl Default for UpdateLists { } } -impl UpdateLists { - pub fn apply(self, db: &Database, _config: &Config) -> Fallible<()> { +impl Action for UpdateLists { + fn apply(self, ctx: &ActionsCtx) -> Fallible<()> { if self.github { info!("updating GitHub repositories list"); - GitHubList::default().update(db)?; + GitHubList::default().update(&ctx.db)?; } if self.registry { info!("updating crates.io crates list"); - RegistryList.update(db)?; + RegistryList.update(&ctx.db)?; } if self.local { info!("updating local crates list"); - LocalList::default().update(db)?; + LocalList::default().update(&ctx.db)?; } Ok(()) diff --git a/src/actions/mod.rs b/src/actions/mod.rs index 03aa2cb9a..e291581b9 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -3,3 +3,22 @@ mod lists; pub use self::experiments::*; pub use self::lists::*; + +use crate::config::Config; +use crate::db::Database; +use crate::prelude::*; + +pub trait Action { + fn apply(self, ctx: &ActionsCtx) -> Fallible<()>; +} + +pub struct ActionsCtx<'ctx> { + db: &'ctx Database, + config: &'ctx Config, +} + +impl<'ctx> ActionsCtx<'ctx> { + pub fn new(db: &'ctx Database, config: &'ctx Config) -> Self { + ActionsCtx { db, config } + } +} diff --git a/src/cli.rs b/src/cli.rs index 2e7b83505..d404c253f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -9,7 +9,7 @@ //! application state employs ownership techniques to ensure that //! parallel access is consistent and race-free. -use crater::actions; +use crater::actions::{self, Action, ActionsCtx}; use crater::agent; use crater::config::Config; use crater::crates::Crate; @@ -274,6 +274,7 @@ impl Crater { let config = Config::load()?; let db = Database::open()?; + let ctx = ActionsCtx::new(&db, &config); let action = if lists.is_empty() { actions::UpdateLists::default() @@ -288,13 +289,14 @@ impl Crater { if let Some(unknown) = lists.iter().next() { bail!("unknown list: {}", unknown); } else { - action.apply(&db, &config)?; + action.apply(&ctx)?; } } Crater::PrepareLocal => { let config = Config::load()?; let db = Database::open()?; - actions::UpdateLists::default().apply(&db, &config)?; + let ctx = ActionsCtx::new(&db, &config); + actions::UpdateLists::default().apply(&ctx)?; } Crater::DefineEx { ref ex, @@ -308,6 +310,7 @@ impl Crater { } => { let config = Config::load()?; let db = Database::open()?; + let ctx = ActionsCtx::new(&db, &config); actions::CreateExperiment { name: ex.0.clone(), @@ -319,7 +322,7 @@ impl Crater { github_issue: None, ignore_blacklist: *ignore_blacklist, } - .apply(&db, &config)?; + .apply(&ctx)?; } Crater::Edit { ref name, @@ -334,6 +337,7 @@ impl Crater { } => { let config = Config::load()?; let db = Database::open()?; + let ctx = ActionsCtx::new(&db, &config); let ignore_blacklist = if *ignore_blacklist { Some(true) @@ -352,13 +356,14 @@ impl Crater { priority: *priority, ignore_blacklist, } - .apply(&db, &config)?; + .apply(&ctx)?; } Crater::DeleteEx { ref ex } => { let config = Config::load()?; let db = Database::open()?; + let ctx = ActionsCtx::new(&db, &config); - actions::DeleteExperiment { name: ex.0.clone() }.apply(&db, &config)?; + actions::DeleteExperiment { name: ex.0.clone() }.apply(&ctx)?; } Crater::DeleteAllResults { ref ex } => { let db = Database::open()?; diff --git a/src/crates/lists.rs b/src/crates/lists.rs index 94d661f17..ef533f589 100644 --- a/src/crates/lists.rs +++ b/src/crates/lists.rs @@ -137,10 +137,12 @@ pub(crate) fn get_crates( #[cfg(test)] pub(crate) fn setup_test_lists(db: &Database, config: &Config) -> Fallible<()> { - crate::actions::UpdateLists { + use crate::actions::{Action, ActionsCtx, UpdateLists}; + + UpdateLists { github: false, registry: false, local: true, } - .apply(db, config) + .apply(&ActionsCtx::new(db, config)) } diff --git a/src/experiments.rs b/src/experiments.rs index 8309569a6..86c5594fd 100644 --- a/src/experiments.rs +++ b/src/experiments.rs @@ -406,7 +406,7 @@ impl ExperimentDBRecord { #[cfg(test)] mod tests { use super::{Assignee, AssigneeParseError, Experiment, Status}; - use crate::actions::CreateExperiment; + use crate::actions::{Action, ActionsCtx, CreateExperiment}; use crate::config::Config; use crate::db::Database; use crate::server::agents::Agents; @@ -461,12 +461,13 @@ mod tests { let _ = Agents::new(db.clone(), &tokens).unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); - CreateExperiment::dummy("test").apply(&db, &config).unwrap(); + CreateExperiment::dummy("test").apply(&ctx).unwrap(); let mut create_important = CreateExperiment::dummy("important"); create_important.priority = 10; - create_important.apply(&db, &config).unwrap(); + create_important.apply(&ctx).unwrap(); // Test the important experiment is correctly assigned let (new, ex) = Experiment::next(&db, &agent1).unwrap().unwrap(); diff --git a/src/report/archives.rs b/src/report/archives.rs index 72edd75fb..489543066 100644 --- a/src/report/archives.rs +++ b/src/report/archives.rs @@ -95,6 +95,7 @@ pub fn write_logs_archives( #[cfg(test)] mod tests { use super::write_logs_archives; + use crate::actions::{Action, ActionsCtx, CreateExperiment}; use crate::config::Config; use crate::db::Database; use crate::experiments::Experiment; @@ -113,13 +114,12 @@ mod tests { let config = Config::default(); let db = Database::temp().unwrap(); let writer = DummyWriter::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); // Create a dummy experiment - crate::actions::CreateExperiment::dummy("dummy") - .apply(&db, &config) - .unwrap(); + CreateExperiment::dummy("dummy").apply(&ctx).unwrap(); let ex = Experiment::get(&db, "dummy").unwrap().unwrap(); let crate1 = ex.crates[0].clone(); let crate2 = ex.crates[1].clone(); diff --git a/src/results/db.rs b/src/results/db.rs index 888d61ef6..77a17c6eb 100644 --- a/src/results/db.rs +++ b/src/results/db.rs @@ -207,7 +207,7 @@ impl<'a> DeleteResults for DatabaseDB<'a> { #[cfg(test)] mod tests { use super::{DatabaseDB, ProgressData, TaskResult}; - use crate::actions::CreateExperiment; + use crate::actions::{Action, ActionsCtx, CreateExperiment}; use crate::config::Config; use crate::crates::{Crate, GitHubRepo, RegistryCrate}; use crate::db::Database; @@ -222,13 +222,12 @@ mod tests { let db = Database::temp().unwrap(); let results = DatabaseDB::new(&db); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); // Create a dummy experiment to attach the results to - CreateExperiment::dummy("dummy") - .apply(&db, &config) - .unwrap(); + CreateExperiment::dummy("dummy").apply(&ctx).unwrap(); let ex = Experiment::get(&db, "dummy").unwrap().unwrap(); // Define some dummy GitHub repositories @@ -285,13 +284,12 @@ mod tests { let db = Database::temp().unwrap(); let results = DatabaseDB::new(&db); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); // Create a dummy experiment to attach the results to - CreateExperiment::dummy("dummy") - .apply(&db, &config) - .unwrap(); + CreateExperiment::dummy("dummy").apply(&ctx).unwrap(); let ex = Experiment::get(&db, "dummy").unwrap().unwrap(); let krate = Crate::Registry(RegistryCrate { @@ -372,13 +370,12 @@ mod tests { let db = Database::temp().unwrap(); let results = DatabaseDB::new(&db); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); crate::crates::lists::setup_test_lists(&db, &config).unwrap(); // Create a dummy experiment to attach the results to - CreateExperiment::dummy("dummy") - .apply(&db, &config) - .unwrap(); + CreateExperiment::dummy("dummy").apply(&ctx).unwrap(); let ex = Experiment::get(&db, "dummy").unwrap().unwrap(); let krate = Crate::Registry(RegistryCrate { diff --git a/src/server/agents.rs b/src/server/agents.rs index 9f58aafbf..948e00426 100644 --- a/src/server/agents.rs +++ b/src/server/agents.rs @@ -151,7 +151,7 @@ impl Agents { #[cfg(test)] mod tests { use super::{AgentStatus, Agents}; - use crate::actions::CreateExperiment; + use crate::actions::{Action, ActionsCtx, CreateExperiment}; use crate::config::Config; use crate::db::Database; use crate::experiments::{Assignee, Experiment}; @@ -217,6 +217,8 @@ mod tests { fn test_agent_status() { let db = Database::temp().unwrap(); let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); + let mut tokens = Tokens::default(); tokens.agents.insert("token".into(), "agent".into()); let agents = Agents::new(db.clone(), &tokens).unwrap(); @@ -233,9 +235,7 @@ mod tests { assert_eq!(agent.status(), AgentStatus::Idle); // Create a new experiment and assign it to the agent - CreateExperiment::dummy("dummy") - .apply(&db, &config) - .unwrap(); + CreateExperiment::dummy("dummy").apply(&ctx).unwrap(); Experiment::next(&db, &Assignee::Agent("agent".to_string())).unwrap(); // After an experiment is assigned to the agent, the agent is working diff --git a/src/server/routes/webhooks/commands.rs b/src/server/routes/webhooks/commands.rs index f00e7d5ed..da93b9615 100644 --- a/src/server/routes/webhooks/commands.rs +++ b/src/server/routes/webhooks/commands.rs @@ -1,3 +1,4 @@ +use crate::actions::{self, Action, ActionsCtx}; use crate::db::{Database, QueryUtils}; use crate::experiments::{CapLints, CrateSelect, Experiment, GitHubIssue, Mode, Status}; use crate::prelude::*; @@ -17,7 +18,7 @@ pub fn ping(data: &Data, issue: &Issue) -> Fallible<()> { pub fn run(host: &str, data: &Data, issue: &Issue, args: RunArgs) -> Fallible<()> { let name = setup_run_name(&data.db, issue, args.name)?; - crate::actions::CreateExperiment { + actions::CreateExperiment { name: name.clone(), toolchains: [ args.start @@ -35,7 +36,7 @@ pub fn run(host: &str, data: &Data, issue: &Issue, args: RunArgs) -> Fallible<() }), ignore_blacklist: args.ignore_blacklist.unwrap_or(false), } - .apply(&data.db, &data.config)?; + .apply(&ActionsCtx::new(&data.db, &data.config))?; Message::new() .line( @@ -58,7 +59,7 @@ pub fn run(host: &str, data: &Data, issue: &Issue, args: RunArgs) -> Fallible<() pub fn edit(data: &Data, issue: &Issue, args: EditArgs) -> Fallible<()> { let name = get_name(&data.db, issue, args.name)?; - let changed = crate::actions::EditExperiment { + actions::EditExperiment { name: name.clone(), toolchains: [args.start, args.end], crates: args.crates, @@ -67,20 +68,14 @@ pub fn edit(data: &Data, issue: &Issue, args: EditArgs) -> Fallible<()> { priority: args.priority, ignore_blacklist: args.ignore_blacklist, } - .apply(&data.db, &data.config)?; + .apply(&ActionsCtx::new(&data.db, &data.config))?; - if changed { - Message::new() - .line( - "memo", - format!("Configuration of the **`{}`** experiment changed.", name), - ) - .send(&issue.url, data)?; - } else { - Message::new() - .line("warning", "No changes requested.") - .send(&issue.url, data)?; - } + Message::new() + .line( + "memo", + format!("Configuration of the **`{}`** experiment changed.", name), + ) + .send(&issue.url, data)?; Ok(()) } @@ -116,7 +111,8 @@ pub fn retry_report(data: &Data, issue: &Issue, args: RetryReportArgs) -> Fallib pub fn abort(data: &Data, issue: &Issue, args: AbortArgs) -> Fallible<()> { let name = get_name(&data.db, issue, args.name)?; - crate::actions::DeleteExperiment { name: name.clone() }.apply(&data.db, &data.config)?; + actions::DeleteExperiment { name: name.clone() } + .apply(&ActionsCtx::new(&data.db, &data.config))?; Message::new() .line("wastebasket", format!("Experiment **`{}`** deleted!", name)) @@ -207,23 +203,27 @@ mod tests { default_experiment_name, generate_new_experiment_name, get_name, setup_run_name, store_experiment_name, }; + use crate::actions::{self, Action, ActionsCtx}; + use crate::config::Config; use crate::db::Database; use crate::prelude::*; use crate::server::github; /// Simulate to the `run` command, and return experiment name fn dummy_run(db: &Database, issue: &github::Issue, name: Option) -> Fallible { + let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); let name = setup_run_name(db, issue, name)?; - crate::actions::CreateExperiment::dummy(&name) - .apply(&db, &crate::config::Config::default())?; + actions::CreateExperiment::dummy(&name).apply(&ctx)?; Ok(name) } /// Simulate to the `edit` command, and return experiment name fn dummy_edit(db: &Database, issue: &github::Issue, name: Option) -> Fallible { + let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); let name = get_name(db, issue, name)?; - crate::actions::EditExperiment::dummy(&name) - .apply(&db, &crate::config::Config::default())?; + actions::EditExperiment::dummy(&name).apply(&ctx)?; Ok(name) } @@ -381,6 +381,9 @@ mod tests { #[test] fn test_generate_new_experiment_name() { let db = Database::temp().unwrap(); + let config = Config::default(); + let ctx = ActionsCtx::new(&db, &config); + let pr = github::Issue { number: 12345, url: String::new(), @@ -391,13 +394,13 @@ mod tests { }), }; - crate::actions::CreateExperiment::dummy("pr-12345") - .apply(&db, &crate::config::Config::default()) + actions::CreateExperiment::dummy("pr-12345") + .apply(&ctx) .expect("could not store dummy experiment"); let new_name = generate_new_experiment_name(&db, &pr).unwrap(); assert_eq!(new_name, "pr-12345-1"); - crate::actions::CreateExperiment::dummy("pr-12345-1") - .apply(&db, &crate::config::Config::default()) + actions::CreateExperiment::dummy("pr-12345-1") + .apply(&ctx) .expect("could not store dummy experiment");; assert_eq!( &generate_new_experiment_name(&db, &pr).unwrap(),