Skip to content

Commit

Permalink
Auto merge of #385 - pietroalbini:action-trait, r=pietroalbini
Browse files Browse the repository at this point in the history 
Refactor actions to implement the Action trait
  • Loading branch information
@bors
bors committed Jan 1, 2019
2 parents ac183ef + 8346d3b commit b83e3c7
Show file tree
Hide file tree
Showing 12 changed files with 156 additions and 135 deletions.
37 changes: 21 additions & 16 deletions src/actions/experiments/create.rs
View file
@@ -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;
Expand Down Expand Up @@ -33,21 +32,23 @@ 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
if self.toolchains[0] == self.toolchains[1] {
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, \
Expand All @@ -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],
Expand All @@ -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};
Expand All @@ -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();

Expand All @@ -118,7 +120,7 @@ mod tests {
}),
ignore_blacklist: true,
}
.apply(&db, &config)
.apply(&ctx)
.unwrap();

let ex = Experiment::get(&db, "foo").unwrap().unwrap();
Expand Down Expand Up @@ -182,22 +184,23 @@ mod tests {
broken: false,
},
);
let ctx = ActionsCtx::new(&db, &config);

crate::crates::lists::setup_test_lists(&db, &config).unwrap();

CreateExperiment {
ignore_blacklist: false,
..CreateExperiment::dummy("foo")
}
.apply(&db, &config)
.apply(&ctx)
.unwrap();
assert!(is_skipped(&db, "foo", "build-pass"));

CreateExperiment {
ignore_blacklist: true,
..CreateExperiment::dummy("bar")
}
.apply(&db, &config)
.apply(&ctx)
.unwrap();
assert!(!is_skipped(&db, "bar", "build-pass"));
}
Expand All @@ -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();

Expand All @@ -220,7 +224,7 @@ mod tests {
github_issue: None,
ignore_blacklist: false,
}
.apply(&db, &config)
.apply(&ctx)
.unwrap_err();

assert_eq!(
Expand All @@ -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();

Expand All @@ -247,7 +252,7 @@ mod tests {
github_issue: None,
ignore_blacklist: false,
}
.apply(&db, &config)
.apply(&ctx)
.unwrap();

// While the second one fails
Expand All @@ -261,7 +266,7 @@ mod tests {
github_issue: None,
ignore_blacklist: false,
}
.apply(&db, &config)
.apply(&ctx)
.unwrap_err();

assert_eq!(
Expand Down
26 changes: 13 additions & 13 deletions src/actions/experiments/delete.rs
View file
@@ -1,22 +1,22 @@
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::*;

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(())
}
Expand All @@ -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;
Expand All @@ -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!(
Expand All @@ -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());
}
Expand Down

0 comments on commit b83e3c7

Please sign in to comment.