Skip to content

Extend CrateSelect to allow an arbitrary number or explicit list of crates #461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Oct 3, 2019
23 changes: 23 additions & 0 deletions docs/bot-usage.md
Original file line number Diff line number Diff line change
@@ -95,6 +95,29 @@ The mode you should use depends on what your experiment is testing:

[Go back to the TOC][h-toc]

## Available crate selections

By default, your experiment will be run on all crates known to Crater.
However, it is possible to run an experiment on a subset of the ecosystem by
passing a different value to `crates`. The following options are currently
available:

* `full`: run the experiment on every crate.
* `top-{n}`: run the experiment on the `n` most downloaded crates on
[crates.io](crates.io) (e.g. `top-100`).
* `random-{n}`: run the experiment on `n` randomly selected crates (e.g. `random-20`).
* `list:{...}`: run the experiment on the specified crates.

For `list:`, the value after the colon can either be a comma-separated list of
crates to run or a link to a newline-separated list of crates ([example][list]).
For example, `list:lazy_static,brson/hello-rs` and `list:https://git.io/Jes7o`
will both run an experiment on the `lazy_static` crate and the git repo at
`github.com/brson/hello-rs`. A link must begin with `http[s]://`.

[list]: https://gist.githubusercontent.com/ecstatic-morse/837c558b63fc73ab469bfbf4ad419a1f/raw/example-crate-list

[Go back to the TOC][h-toc]

## Automatic experiment names

[h-experiment-names]: #automatic-experiment-names
4 changes: 2 additions & 2 deletions src/actions/experiments/create.rs
Original file line number Diff line number Diff line change
@@ -50,7 +50,7 @@ impl Action for CreateExperiment {
return Err(ExperimentError::DuplicateToolchains.into());
}

let crates = crate::crates::lists::get_crates(self.crates, &ctx.db, &ctx.config)?;
let crates = crate::crates::lists::get_crates(&self.crates, &ctx.db, &ctx.config)?;

ctx.db.transaction(|transaction| {
transaction.execute(
@@ -143,7 +143,7 @@ mod tests {
assert_eq!(ex.mode, Mode::BuildAndTest);
assert_eq!(
ex.get_crates(&ctx.db).unwrap(),
crate::crates::lists::get_crates(CrateSelect::Local, &db, &config).unwrap()
crate::crates::lists::get_crates(&CrateSelect::Local, &db, &config).unwrap()
);
assert_eq!(ex.cap_lints, CapLints::Forbid);
assert_eq!(ex.github_issue.as_ref().unwrap().api_url.as_str(), api_url);
6 changes: 3 additions & 3 deletions src/actions/experiments/edit.rs
Original file line number Diff line number Diff line change
@@ -79,7 +79,7 @@ impl Action for EditExperiment {
// 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,
&crates,
&ctx.db,
&ctx.config,
)?)
@@ -199,7 +199,7 @@ mod tests {
name: "foo".to_string(),
toolchains: ["stable".parse().unwrap(), "beta".parse().unwrap()],
mode: Mode::BuildAndTest,
crates: CrateSelect::SmallRandom,
crates: CrateSelect::Random(20),
cap_lints: CapLints::Forbid,
priority: 0,
github_issue: None,
@@ -242,7 +242,7 @@ mod tests {

assert_eq!(
ex.get_crates(&ctx.db).unwrap(),
crate::crates::lists::get_crates(CrateSelect::Local, &db, &config).unwrap()
crate::crates::lists::get_crates(&CrateSelect::Local, &db, &config).unwrap()
);
}

26 changes: 16 additions & 10 deletions src/cli.rs
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@ use crater::agent::{self, Capabilities};
use crater::config::Config;
use crater::crates::Crate;
use crater::db::Database;
use crater::experiments::{Assignee, CapLints, CrateSelect, Experiment, Mode, Status};
use crater::experiments::{Assignee, CapLints, DeferredCrateSelect, Experiment, Mode, Status};
use crater::report;
use crater::results::{DatabaseDB, DeleteResults};
use crater::runner;
@@ -118,12 +118,14 @@ pub enum Crater {
#[structopt(
name = "crate-select",
long = "crate-select",
raw(
default_value = "CrateSelect::Demo.to_str()",
possible_values = "CrateSelect::possible_values()"
)
help = "The set of crates on which the experiment will run.",
long_help = "The set of crates on which the experiment will run.\n\n\
This can be one of (full, demo, random-{d}, top-{d}, local) \
where {d} is a positive integer, or \"list:\" followed \
by a comma-separated list of crates.",
raw(default_value = "\"demo\"",)
)]
crates: CrateSelect,
crates: DeferredCrateSelect,
#[structopt(
name = "level",
long = "cap-lints",
@@ -160,9 +162,13 @@ pub enum Crater {
#[structopt(
name = "crates",
long = "crates",
raw(possible_values = "CrateSelect::possible_values()")
help = "The set of crates on which the experiment will run.",
long_help = "The set of crates on which the experiment will run.\n\n\
This can be one of (full, demo, random-{d}, top-{d}, local) \
where {d} is a positive integer, or \"list:\" followed \
by a comma-separated list of crates."
)]
crates: Option<CrateSelect>,
crates: Option<DeferredCrateSelect>,
#[structopt(
name = "cap-lints",
long = "cap-lints",
@@ -360,7 +366,7 @@ impl Crater {
name: ex.0.clone(),
toolchains: [tc1.clone(), tc2.clone()],
mode: *mode,
crates: *crates,
crates: crates.clone().resolve()?,
cap_lints: *cap_lints,
priority: *priority,
github_issue: None,
@@ -399,7 +405,7 @@ impl Crater {
name: name.clone(),
toolchains: [tc1.clone(), tc2.clone()],
mode: *mode,
crates: *crates,
crates: crates.clone().map(|cs| cs.resolve()).transpose()?,
cap_lints: *cap_lints,
priority: *priority,
ignore_blacklist,
2 changes: 1 addition & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
@@ -146,7 +146,7 @@ impl Config {
let mut has_errors = Self::check_for_dup_keys(&buffer).is_err();
let cfg: Self = ::toml::from_str(&buffer)?;
let db = crate::db::Database::open()?;
let crates = crate::crates::lists::get_crates(CrateSelect::Full, &db, &cfg)?;
let crates = crate::crates::lists::get_crates(&CrateSelect::Full, &db, &cfg)?;
has_errors |= cfg.check_for_missing_crates(&crates).is_err();
has_errors |= cfg.check_for_missing_repos(&crates).is_err();
if has_errors {
37 changes: 30 additions & 7 deletions src/crates/lists.rs
Original file line number Diff line number Diff line change
@@ -12,8 +12,6 @@ pub(crate) use crate::crates::sources::{
github::GitHubList, local::LocalList, registry::RegistryList,
};

const SMALL_RANDOM_COUNT: usize = 20;

pub(crate) trait List {
const NAME: &'static str;

@@ -63,7 +61,7 @@ pub(crate) trait List {
}

pub(crate) fn get_crates(
select: CrateSelect,
select: &CrateSelect,
db: &Database,
config: &Config,
) -> Fallible<Vec<Crate>> {
@@ -74,6 +72,7 @@ pub(crate) fn get_crates(
crates.append(&mut RegistryList::get(db)?);
crates.append(&mut GitHubList::get(db)?);
}

CrateSelect::Demo => {
let mut demo_registry = config.demo_crates().crates.iter().collect::<HashSet<_>>();
let mut demo_github = config
@@ -115,17 +114,41 @@ pub(crate) fn get_crates(
bail!("missing demo local crates: {:?}", demo_local);
}
}
CrateSelect::SmallRandom => {
CrateSelect::List(list) => {
let mut desired = list.clone();

let mut all_crates = Vec::new();
all_crates.append(&mut RegistryList::get(db)?);
all_crates.append(&mut GitHubList::get(db)?);

for krate in all_crates {
let is_desired = match krate {
Crate::Registry(RegistryCrate { ref name, .. }) => desired.remove(name),
Crate::GitHub(ref repo) => desired.remove(&repo.slug()),
_ => unreachable!(),
};

if is_desired {
crates.push(krate);
}
}

if !desired.is_empty() {
bail!("missing desired crates: {:?}", desired);
}
}

CrateSelect::Random(n) => {
crates.append(&mut RegistryList::get(db)?);
crates.append(&mut GitHubList::get(db)?);

let mut rng = thread_rng();
rng.shuffle(&mut crates);
crates.truncate(SMALL_RANDOM_COUNT);
crates.truncate(*n as usize);
}
CrateSelect::Top100 => {
CrateSelect::Top(n) => {
crates.append(&mut RegistryList::get(db)?);
crates.truncate(100);
crates.truncate(*n as usize);
}
CrateSelect::Local => {
crates.append(&mut LocalList::get(db)?);
191 changes: 181 additions & 10 deletions src/experiments.rs
Original file line number Diff line number Diff line change
@@ -2,11 +2,14 @@ use crate::crates::Crate;
use crate::db::{Database, QueryUtils};
use crate::prelude::*;
use crate::toolchain::Toolchain;
use crate::utils;
use chrono::{DateTime, Utc};
use rusqlite::Row;
use serde_json;
use std::collections::HashSet;
use std::fmt;
use std::str::FromStr;
use url::Url;

string_enum!(pub enum Status {
Queued => "queued",
@@ -27,22 +30,140 @@ string_enum!(pub enum Mode {
UnstableFeatures => "unstable-features",
});

string_enum!(pub enum CrateSelect {
Full => "full",
Demo => "demo",
SmallRandom => "small-random",
Top100 => "top-100",
Local => "local",
Dummy => "dummy",
});

string_enum!(pub enum CapLints {
Allow => "allow",
Warn => "warn",
Deny => "deny",
Forbid => "forbid",
});

const SMALL_RANDOM_COUNT: u32 = 20;

#[derive(Debug, PartialEq, Eq, Clone)]
pub enum CrateSelect {
Full,
Demo,
Top(u32),
Local,
Dummy,
Random(u32),
List(HashSet<String>),
}

impl FromStr for CrateSelect {
type Err = failure::Error;

fn from_str(s: &str) -> failure::Fallible<Self> {
let ret = match s {
s if s.starts_with("top-") => {
let n: u32 = s["top-".len()..].parse()?;
CrateSelect::Top(n)
}

"small-random" => CrateSelect::Random(SMALL_RANDOM_COUNT),
s if s.starts_with("random-") => {
let n: u32 = s["random-".len()..].parse()?;
CrateSelect::Random(n)
}

s if s.starts_with("list:") => {
let list = s["list:".len()..]
.split(',')
.map(|s| s.to_owned())
.collect();

CrateSelect::List(list)
}

"full" => CrateSelect::Full,
"demo" => CrateSelect::Demo,
"local" => CrateSelect::Local,
"dummy" => CrateSelect::Dummy,
s => bail!("invalid CrateSelect: {}", s),
};

Ok(ret)
}
}

impl fmt::Display for CrateSelect {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
CrateSelect::Full => write!(f, "full"),
CrateSelect::Demo => write!(f, "demo"),
CrateSelect::Dummy => write!(f, "dummy"),
CrateSelect::Top(n) => write!(f, "top-{}", n),
CrateSelect::Local => write!(f, "local"),
CrateSelect::Random(n) => write!(f, "random-{}", n),
CrateSelect::List(list) => {
let mut first = true;
write!(f, "list:")?;

for krate in list {
if !first {
write!(f, ",")?;
}

write!(f, "{}", krate)?;
first = false;
}

Ok(())
}
}
}
}

impl CrateSelect {
fn from_newline_separated_list(s: &str) -> Fallible<CrateSelect> {
if s.contains(',') {
bail!("Crate identifiers must not contain a comma");
}

let crates = s.split_whitespace().map(|s| s.to_owned()).collect();
Ok(CrateSelect::List(crates))
}
}

/// Either a `CrateSelect` or `Url` pointing to a list of crates.
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum DeferredCrateSelect {
Direct(CrateSelect),
Indirect(Url),
}

impl From<CrateSelect> for DeferredCrateSelect {
fn from(v: CrateSelect) -> Self {
DeferredCrateSelect::Direct(v)
}
}

impl DeferredCrateSelect {
pub fn resolve(self) -> Fallible<CrateSelect> {
let url = match self {
DeferredCrateSelect::Direct(v) => return Ok(v),
DeferredCrateSelect::Indirect(url) => url,
};

let body = utils::http::get_sync(url.as_str())?.text()?;
CrateSelect::from_newline_separated_list(&body)
}
}

impl FromStr for DeferredCrateSelect {
type Err = failure::Error;

fn from_str(input: &str) -> Fallible<Self> {
if input.starts_with("https://") || input.starts_with("http://") {
Ok(DeferredCrateSelect::Indirect(input.parse()?))
} else {
Ok(DeferredCrateSelect::Direct(input.parse()?))
}
}
}

impl_serde_from_parse!(CrateSelect, expecting = "A valid value of `CrateSelect`");

#[cfg_attr(test, derive(Debug, PartialEq, Eq))]
#[derive(Clone, Serialize, Deserialize)]
pub enum Assignee {
@@ -446,15 +567,65 @@ impl ExperimentDBRecord {

#[cfg(test)]
mod tests {
use super::{Assignee, AssigneeParseError, Experiment, Status};
use super::{
Assignee, AssigneeParseError, CrateSelect, DeferredCrateSelect, Experiment, Status,
};
use crate::actions::{Action, ActionsCtx, CreateExperiment};
use crate::agent::Capabilities;
use crate::config::Config;
use crate::db::Database;
use crate::server::agents::Agents;
use crate::server::tokens::Tokens;
use std::collections::HashSet;
use std::str::FromStr;

#[test]
fn test_crate_select_parsing() {
let demo_crates: HashSet<_> = ["brson/hello-rs", "lazy_static"]
.into_iter()
.map(|s| s.to_string())
.collect();

let suite = vec![
("demo", CrateSelect::Demo),
("top-25", CrateSelect::Top(25)),
("random-87", CrateSelect::Random(87)),
("small-random", CrateSelect::Random(20)),
(
"list:brson/hello-rs,lazy_static",
CrateSelect::List(demo_crates.clone()),
),
];

for (s, output) in suite.into_iter() {
assert_eq!(CrateSelect::from_str(s).unwrap(), output);
assert_eq!(
DeferredCrateSelect::from_str(s).unwrap(),
DeferredCrateSelect::Direct(output),
);
}

assert_eq!(
DeferredCrateSelect::from_str("http://git.io/Jes7o").unwrap(),
DeferredCrateSelect::Indirect("http://git.io/Jes7o".parse().unwrap()),
);

assert_eq!(
DeferredCrateSelect::from_str("https://git.io/Jes7o").unwrap(),
DeferredCrateSelect::Indirect("https://git.io/Jes7o".parse().unwrap()),
);

let list = CrateSelect::from_newline_separated_list(
r"
brson/hello-rs
lazy_static",
)
.unwrap();

assert_eq!(list, CrateSelect::List(demo_crates));
}

#[test]
fn test_assignee_parsing() {
assert_eq!(
9 changes: 5 additions & 4 deletions src/server/routes/webhooks/args.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::experiments::{Assignee, CapLints, CrateSelect, Mode};
use crate::experiments::{Assignee, CapLints, DeferredCrateSelect, Mode};
use crate::toolchain::Toolchain;
use failure::{self, Fallible};

#[derive(Debug, Fail)]
#[cfg_attr(test, derive(PartialEq, Eq))]
@@ -107,7 +108,7 @@ generate_parser!(pub enum Command {
start: Option<Toolchain> = "start",
end: Option<Toolchain> = "end",
mode: Option<Mode> = "mode",
crates: Option<CrateSelect> = "crates",
crates: Option<DeferredCrateSelect> = "crates",
cap_lints: Option<CapLints> = "cap-lints",
priority: Option<i32> = "p",
ignore_blacklist: Option<bool> = "ignore-blacklist",
@@ -119,7 +120,7 @@ generate_parser!(pub enum Command {
name: Option<String> = "name",
start: Option<Toolchain> = "start",
end: Option<Toolchain> = "end",
crates: Option<CrateSelect> = "crates",
crates: Option<DeferredCrateSelect> = "crates",
cap_lints: Option<CapLints> = "cap-lints",
priority: Option<i32> = "p",
ignore_blacklist: Option<bool> = "ignore-blacklist",
@@ -148,7 +149,7 @@ generate_parser!(pub enum Command {
start: Option<Toolchain> = "start",
end: Option<Toolchain> = "end",
mode: Option<Mode> = "mode",
crates: Option<CrateSelect> = "crates",
crates: Option<DeferredCrateSelect> = "crates",
cap_lints: Option<CapLints> = "cap-lints",
priority: Option<i32> = "p",
ignore_blacklist: Option<bool> = "ignore-blacklist",
15 changes: 13 additions & 2 deletions src/server/routes/webhooks/commands.rs
Original file line number Diff line number Diff line change
@@ -83,6 +83,11 @@ pub fn run(

// Make crater runs created via webhook require linux by default.
let requirement = args.requirement.unwrap_or_else(|| "linux".to_string());
let crates = args
.crates
.map(|c| c.resolve())
.transpose()
.map_err(|e| e.context("Failed to resolve crate list"))?;

actions::CreateExperiment {
name: name.clone(),
@@ -95,7 +100,7 @@ pub fn run(
.ok_or_else(|| err_msg("missing end toolchain"))?,
],
mode: args.mode.unwrap_or(Mode::BuildAndTest),
crates: args.crates.unwrap_or(CrateSelect::Full),
crates: crates.unwrap_or(CrateSelect::Full),
cap_lints: args.cap_lints.unwrap_or(CapLints::Forbid),
priority: args.priority.unwrap_or(0),
github_issue: Some(GitHubIssue {
@@ -131,10 +136,16 @@ pub fn run(
pub fn edit(data: &Data, issue: &Issue, args: EditArgs) -> Fallible<()> {
let name = get_name(&data.db, issue, args.name)?;

let crates = args
.crates
.map(|c| c.resolve())
.transpose()
.map_err(|e| e.context("Failed to resolve crate list"))?;

actions::EditExperiment {
name: name.clone(),
toolchains: [args.start, args.end],
crates: args.crates,
crates,
mode: args.mode,
cap_lints: args.cap_lints,
priority: args.priority,