Skip to content
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

Add a new clippy mode to crater to test lint regressions #391

Merged
merged 4 commits into from
Jan 26, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions local-crates/clippy-warn/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "clippy-warn"
version = "0.1.0"
authors = ["Dylan MacKenzie <[email protected]>"]
edition = "2018"

[dependencies]
4 changes: 4 additions & 0 deletions local-crates/clippy-warn/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main() {
// Trigger `clippy::print_with_newline`
print!("Hello, world!\n");
}
2 changes: 1 addition & 1 deletion src/dirs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,6 @@ pub(crate) fn crate_source_dir(ex: &Experiment, tc: &Toolchain, krate: &Crate) -
EXPERIMENT_DIR
.join(&ex.name)
.join("sources")
.join(tc.to_string())
.join(tc.to_path_component())
.join(krate.id())
}
1 change: 1 addition & 0 deletions src/experiments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ string_enum!(pub enum Mode {
BuildAndTest => "build-and-test",
BuildOnly => "build-only",
CheckOnly => "check-only",
Clippy => "clippy",
Rustdoc => "rustdoc",
UnstableFeatures => "unstable-features",
});
Expand Down
4 changes: 4 additions & 0 deletions src/runner/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ pub(super) fn build_graph(ex: &Experiment, config: &Config) -> TasksGraph {
tc: tc.clone(),
quiet,
},
Mode::Clippy => TaskStep::Clippy {
tc: tc.clone(),
quiet,
},
Mode::Rustdoc => TaskStep::Rustdoc {
tc: tc.clone(),
quiet,
Expand Down
5 changes: 4 additions & 1 deletion src/runner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ mod unstable_features;
use crate::config::Config;
use crate::crates::Crate;
use crate::docker::DockerEnv;
use crate::experiments::Experiment;
use crate::experiments::{Experiment, Mode};
use crate::logs::LogStorage;
use crate::prelude::*;
use crate::results::{FailureReason, TestResult, WriteResults};
Expand Down Expand Up @@ -87,6 +87,9 @@ fn run_ex_inner<DB: WriteResults + Sync>(
info!("preparing the execution...");
for tc in &ex.toolchains {
tc.prepare()?;
if ex.mode == Mode::Clippy {
tc.install_rustup_component("clippy")?;
}
}

info!("running tasks in {} threads...", threads_count);
Expand Down
54 changes: 24 additions & 30 deletions src/runner/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,42 +54,30 @@ pub(super) enum TaskStep {
BuildAndTest { tc: Toolchain, quiet: bool },
BuildOnly { tc: Toolchain, quiet: bool },
CheckOnly { tc: Toolchain, quiet: bool },
Clippy { tc: Toolchain, quiet: bool },
Rustdoc { tc: Toolchain, quiet: bool },
UnstableFeatures { tc: Toolchain },
}

impl fmt::Debug for TaskStep {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
TaskStep::Prepare => write!(f, "prepare")?,
TaskStep::Cleanup => write!(f, "cleanup")?,
TaskStep::BuildAndTest { ref tc, quiet } => {
write!(f, "build and test {}", tc.to_string())?;
if quiet {
write!(f, " (quiet)")?;
}
}
TaskStep::BuildOnly { ref tc, quiet } => {
write!(f, "build {}", tc.to_string())?;
if quiet {
write!(f, " (quiet)")?;
}
}
TaskStep::CheckOnly { ref tc, quiet } => {
write!(f, "check {}", tc.to_string())?;
if quiet {
write!(f, " (quiet)")?;
}
}
TaskStep::Rustdoc { ref tc, quiet } => {
write!(f, "doc {}", tc.to_string())?;
if quiet {
write!(f, " (quiet)")?;
}
}
TaskStep::UnstableFeatures { ref tc } => {
write!(f, "find unstable features on {}", tc.to_string())?;
}
let (name, quiet, tc) = match *self {
TaskStep::Prepare => ("prepare", false, None),
TaskStep::Cleanup => ("cleanup", false, None),
TaskStep::BuildAndTest { ref tc, quiet } => ("build and test", quiet, Some(tc)),
TaskStep::BuildOnly { ref tc, quiet } => ("build", quiet, Some(tc)),
TaskStep::CheckOnly { ref tc, quiet } => ("check", quiet, Some(tc)),
TaskStep::Clippy { ref tc, quiet } => ("clippy", quiet, Some(tc)),
TaskStep::Rustdoc { ref tc, quiet } => ("doc", quiet, Some(tc)),
TaskStep::UnstableFeatures { ref tc } => ("find unstable features on", false, Some(tc)),
};

write!(f, "{}", name)?;
if let Some(tc) = tc {
write!(f, " {}", tc.to_string())?;
}
if quiet {
write!(f, " (quiet)")?;
}
Ok(())
}
Expand Down Expand Up @@ -120,6 +108,7 @@ impl Task {
TaskStep::BuildAndTest { ref tc, .. }
| TaskStep::BuildOnly { ref tc, .. }
| TaskStep::CheckOnly { ref tc, .. }
| TaskStep::Clippy { ref tc, .. }
| TaskStep::Rustdoc { ref tc, .. }
| TaskStep::UnstableFeatures { ref tc } => {
db.get_result(ex, tc, &self.krate).unwrap_or(None).is_none()
Expand All @@ -141,6 +130,7 @@ impl Task {
TaskStep::BuildAndTest { ref tc, .. }
| TaskStep::BuildOnly { ref tc, .. }
| TaskStep::CheckOnly { ref tc, .. }
| TaskStep::Clippy { ref tc, .. }
| TaskStep::Rustdoc { ref tc, .. }
| TaskStep::UnstableFeatures { ref tc } => {
let log_storage = state
Expand Down Expand Up @@ -199,6 +189,10 @@ impl Task {
let ctx = TaskCtx::new(config, db, ex, tc, &self.krate, docker_env, state, quiet);
test::run_test("checking", &ctx, test::test_check_only)?;
}
TaskStep::Clippy { ref tc, quiet } => {
let ctx = TaskCtx::new(config, db, ex, tc, &self.krate, docker_env, state, quiet);
test::run_test("linting", &ctx, test::test_clippy_only)?;
}
TaskStep::Rustdoc { ref tc, quiet } => {
let ctx = TaskCtx::new(config, db, ex, tc, &self.krate, docker_env, state, quiet);
test::run_test("documenting", &ctx, test::test_rustdoc)?;
Expand Down
15 changes: 15 additions & 0 deletions src/runner/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,21 @@ pub(super) fn test_check_only<DB: WriteResults>(
}
}

pub(super) fn test_clippy_only<DB: WriteResults>(
ctx: &TaskCtx<DB>,
source_path: &Path,
) -> Fallible<TestResult> {
if let Err(err) = run_cargo(
ctx,
source_path,
&["clippy", "--frozen", "--all", "--all-targets"],
) {
Ok(TestResult::BuildFail(failure_reason(&err)))
} else {
Ok(TestResult::TestPass)
}
}

pub(super) fn test_rustdoc<DB: WriteResults>(
ctx: &TaskCtx<DB>,
source_path: &Path,
Expand Down
1 change: 1 addition & 0 deletions src/server/routes/ui/experiments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ impl ExperimentData {
Mode::BuildAndTest => "cargo test",
Mode::BuildOnly => "cargo build",
Mode::CheckOnly => "cargo check",
Mode::Clippy => "cargo clippy",
Mode::Rustdoc => "cargo doc",
Mode::UnstableFeatures => "unstable features",
},
Expand Down
29 changes: 28 additions & 1 deletion src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,27 @@ impl Toolchain {
}
}

pub fn install_rustup_component(&self, component: &str) -> Fallible<()> {
let toolchain_name = &self.rustup_name();
info!(
"installing component {} for toolchain {}",
component, toolchain_name
);

utils::try_hard(|| {
RunCommand::new(&RUSTUP)
.args(&["component", "add", "--toolchain", toolchain_name, component])
.run()
.with_context(|_| {
format!(
"unable to install component {} for toolchain {} via rustup",
component, toolchain_name,
)
})
})?;
Ok(())
}

pub fn target_dir(&self, ex_name: &str) -> PathBuf {
let mut dir = ex_target_dir(ex_name);

Expand All @@ -80,7 +101,7 @@ impl Toolchain {
dir = dir.join("shared");
}

dir.join(self.to_string())
dir.join(self.to_path_component())
}

pub fn prep_offline_registry(&self) -> Fallible<()> {
Expand All @@ -100,6 +121,12 @@ impl Toolchain {
// is ready
Ok(())
}

pub fn to_path_component(&self) -> String {
use url::percent_encoding::utf8_percent_encode as encode;

encode(&self.to_string(), utils::fs::FILENAME_ENCODE_SET).to_string()
}
}

impl fmt::Display for Toolchain {
Expand Down
8 changes: 8 additions & 0 deletions src/utils/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,16 @@ use crate::prelude::*;
use crate::utils::try_hard_limit;
use std::fs;
use std::path::{Path, PathBuf};
use url::percent_encoding::SIMPLE_ENCODE_SET;
use walkdir::{DirEntry, WalkDir};

url::define_encode_set! {
/// The set of characters which cannot be used in a [filename on Windows][windows].
///
/// [windows]: https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file#naming-conventions
pub FILENAME_ENCODE_SET = [SIMPLE_ENCODE_SET] | { '<', '>', ':', '"', '/', '\\', '|', '?', '*' }
}

pub(crate) fn try_canonicalize<P: AsRef<Path>>(path: P) -> PathBuf {
fs::canonicalize(&path).unwrap_or_else(|_| path.as_ref().to_path_buf())
}
Expand Down
25 changes: 25 additions & 0 deletions tests/minicrater/clippy/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
[server]
bot-acl = [
"pietroalbini",
]

[server.labels]
remove = "^S-"
experiment-queued = "S-waiting-on-crater"
experiment-completed = "S-waiting-on-review"

[demo-crates]
crates = []
github-repos = []
local-crates = ["build-pass", "clippy-warn"]

[sandbox]
memory-limit = "512M"
build-log-max-size = "2M"
build-log-max-lines = 1000

[crates]

[github-repos]

[local-crates]
34 changes: 34 additions & 0 deletions tests/minicrater/clippy/results.expected.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"crates": [
{
"name": "build-pass (local)",
"res": "test-pass",
"runs": [
{
"log": "stable/local/build-pass",
"res": "test-pass"
},
{
"log": "stable%2Brustflags=-Dclippy::all/local/build-pass",
"res": "test-pass"
}
],
"url": "https://github.com/rust-lang-nursery/crater/tree/master/local-crates/build-pass"
},
{
"name": "clippy-warn (local)",
"res": "regressed",
"runs": [
{
"log": "stable/local/clippy-warn",
"res": "test-pass"
},
{
"log": "stable%2Brustflags=-Dclippy::all/local/clippy-warn",
"res": "build-fail:unknown"
}
],
"url": "https://github.com/rust-lang-nursery/crater/tree/master/local-crates/clippy-warn"
}
]
}
19 changes: 18 additions & 1 deletion tests/minicrater/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,21 @@ pub(super) struct MinicraterRun {
pub(super) crate_select: &'static str,
pub(super) multithread: bool,
pub(super) ignore_blacklist: bool,
pub(super) mode: &'static str,
pub(super) toolchains: &'static [&'static str],
}

impl Default for MinicraterRun {
fn default() -> Self {
MinicraterRun {
ex: "default",
crate_select: "demo",
multithread: false,
ignore_blacklist: false,
mode: "build-and-test",
toolchains: &["stable", "beta"],
}
}
}

impl MinicraterRun {
Expand Down Expand Up @@ -54,8 +69,10 @@ impl MinicraterRun {
.minicrater_exec();

// Define the experiment
let mode = format!("--mode={}", self.mode);
let crate_select = format!("--crate-select={}", self.crate_select);
let mut define_args = vec!["define-ex", &ex_arg, "stable", "beta", &crate_select];
let mut define_args = vec!["define-ex", &ex_arg, &crate_select, &mode];
define_args.extend(self.toolchains);
if self.ignore_blacklist {
define_args.push("--ignore-blacklist");
}
Expand Down
21 changes: 13 additions & 8 deletions tests/minicrater/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,40 @@ minicrater! {
single_thread_small {
ex: "small",
crate_select: "demo",
multithread: false,
ignore_blacklist: false,
..Default::default()
},

single_thread_full {
ex: "full",
crate_select: "local",
multithread: false,
ignore_blacklist: false,
..Default::default()
},

single_thread_blacklist {
ex: "blacklist",
crate_select: "demo",
multithread: false,
ignore_blacklist: false,
..Default::default()
},

single_thread_ignore_blacklist {
ex: "ignore-blacklist",
crate_select: "demo",
multithread: false,
ignore_blacklist: true,
..Default::default()
},

multi_thread_full {
ex: "full",
crate_select: "local",
multithread: true,
ignore_blacklist: false,
..Default::default()
},

clippy_small {
ex: "clippy",
crate_select: "demo",
mode: "clippy",
toolchains: &["stable", "stable+rustflags=-Dclippy::all"],
..Default::default()
},
}