From 7177fffd7b65c584b22abe42ec9845ec0a70565c Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 27 Jan 2019 02:03:52 +0300 Subject: fix verification on CI remove `--verify` flag from the binaries: we have tests for this! --- .travis.yml | 2 - crates/ra_syntax/src/ast/generated.rs | 44 ------------ crates/tools/src/bin/pre-commit.rs | 7 +- crates/tools/src/lib.rs | 106 +++++++++++++++++++++++++++- crates/tools/src/main.rs | 127 ++-------------------------------- crates/tools/tests/cli.rs | 16 +++-- 6 files changed, 123 insertions(+), 179 deletions(-) diff --git a/.travis.yml b/.travis.yml index bddf1bebb..8d10a43f0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,8 +10,6 @@ build: &rust_build script: - rustup component add rustfmt - rustup component add rust-src - - cargo gen-tests --verify - - cargo gen-syntax --verify - cargo test --no-run # let's measure compile time separately - cargo test env: diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs index 4f8723ae7..3ace6533c 100644 --- a/crates/ra_syntax/src/ast/generated.rs +++ b/crates/ra_syntax/src/ast/generated.rs @@ -660,50 +660,6 @@ impl ToOwned for DynTraitType { impl DynTraitType {} -// ElseBranch -#[derive(Debug, PartialEq, Eq, Hash)] -#[repr(transparent)] -pub struct ElseBranch { - pub(crate) syntax: SyntaxNode, -} -unsafe impl TransparentNewType for ElseBranch { - type Repr = rowan::SyntaxNode; -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ElseBranchKind<'a> { - Block(&'a Block), - IfExpr(&'a IfExpr), -} - -impl AstNode for ElseBranch { - fn cast(syntax: &SyntaxNode) -> Option<&Self> { - match syntax.kind() { - | BLOCK - | IF_EXPR => Some(ElseBranch::from_repr(syntax.into_repr())), - _ => None, - } - } - fn syntax(&self) -> &SyntaxNode { &self.syntax } -} - -impl ToOwned for ElseBranch { - type Owned = TreeArc; - fn to_owned(&self) -> TreeArc { TreeArc::cast(self.syntax.to_owned()) } -} - -impl ElseBranch { - pub fn kind(&self) -> ElseBranchKind { - match self.syntax.kind() { - BLOCK => ElseBranchKind::Block(Block::cast(&self.syntax).unwrap()), - IF_EXPR => ElseBranchKind::IfExpr(IfExpr::cast(&self.syntax).unwrap()), - _ => unreachable!(), - } - } -} - -impl ElseBranch {} - // EnumDef #[derive(Debug, PartialEq, Eq, Hash)] #[repr(transparent)] diff --git a/crates/tools/src/bin/pre-commit.rs b/crates/tools/src/bin/pre-commit.rs index bae3b26d3..e00bd0d3d 100644 --- a/crates/tools/src/bin/pre-commit.rs +++ b/crates/tools/src/bin/pre-commit.rs @@ -1,10 +1,9 @@ -use std::{ - process::{Command}, -}; +use std::process::Command; -use tools::{Result, run_rustfmt, run, project_root}; use failure::bail; +use tools::{Result, run_rustfmt, run, project_root}; + fn main() -> tools::Result<()> { run_rustfmt(tools::Overwrite)?; update_staged() diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index d404db214..311bcb4d8 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -1,7 +1,8 @@ use std::{ + fs, + collections::HashMap, path::{Path, PathBuf}, process::{Command, Stdio}, - fs::copy, io::{Error, ErrorKind} }; @@ -13,6 +14,10 @@ pub use teraron::{Mode, Overwrite, Verify}; pub type Result = std::result::Result; pub const GRAMMAR: &str = "crates/ra_syntax/src/grammar.ron"; +const GRAMMAR_DIR: &str = "crates/ra_syntax/src/grammar"; +const OK_INLINE_TESTS_DIR: &str = "crates/ra_syntax/tests/data/parser/inline/ok"; +const ERR_INLINE_TESTS_DIR: &str = "crates/ra_syntax/tests/data/parser/inline/err"; + pub const SYNTAX_KINDS: &str = "crates/ra_syntax/src/syntax_kinds/generated.rs.tera"; pub const AST: &str = "crates/ra_syntax/src/ast/generated.rs.tera"; const TOOLCHAIN: &str = "stable"; @@ -130,9 +135,9 @@ pub fn install_format_hook() -> Result<()> { if !result_path.exists() { run("cargo build --package tools --bin pre-commit", ".")?; if cfg!(windows) { - copy("./target/debug/pre-commit.exe", result_path)?; + fs::copy("./target/debug/pre-commit.exe", result_path)?; } else { - copy("./target/debug/pre-commit", result_path)?; + fs::copy("./target/debug/pre-commit", result_path)?; } } else { return Err(Error::new(ErrorKind::AlreadyExists, "Git hook already created").into()); @@ -156,3 +161,98 @@ pub fn run_fuzzer() -> Result<()> { "./crates/ra_syntax", ) } + +pub fn gen_tests(mode: Mode) -> Result<()> { + let tests = tests_from_dir(&project_root().join(Path::new(GRAMMAR_DIR)))?; + fn install_tests(tests: &HashMap, into: &str, mode: Mode) -> Result<()> { + let tests_dir = project_root().join(into); + if !tests_dir.is_dir() { + fs::create_dir_all(&tests_dir)?; + } + // ok is never actually read, but it needs to be specified to create a Test in existing_tests + let existing = existing_tests(&tests_dir, true)?; + for t in existing.keys().filter(|&t| !tests.contains_key(t)) { + panic!("Test is deleted: {}", t); + } + + let mut new_idx = existing.len() + 1; + for (name, test) in tests { + let path = match existing.get(name) { + Some((path, _test)) => path.clone(), + None => { + let file_name = format!("{:04}_{}.rs", new_idx, name); + new_idx += 1; + tests_dir.join(file_name) + } + }; + teraron::update(&path, &test.text, mode)?; + } + Ok(()) + } + install_tests(&tests.ok, OK_INLINE_TESTS_DIR, mode)?; + install_tests(&tests.err, ERR_INLINE_TESTS_DIR, mode) +} + +#[derive(Default, Debug)] +struct Tests { + pub ok: HashMap, + pub err: HashMap, +} + +fn tests_from_dir(dir: &Path) -> Result { + let mut res = Tests::default(); + for entry in ::walkdir::WalkDir::new(dir) { + let entry = entry.unwrap(); + if !entry.file_type().is_file() { + continue; + } + if entry.path().extension().unwrap_or_default() != "rs" { + continue; + } + process_file(&mut res, entry.path())?; + } + let grammar_rs = dir.parent().unwrap().join("grammar.rs"); + process_file(&mut res, &grammar_rs)?; + return Ok(res); + fn process_file(res: &mut Tests, path: &Path) -> Result<()> { + let text = fs::read_to_string(path)?; + + for (_, test) in collect_tests(&text) { + if test.ok { + if let Some(old_test) = res.ok.insert(test.name.clone(), test) { + bail!("Duplicate test: {}", old_test.name) + } + } else { + if let Some(old_test) = res.err.insert(test.name.clone(), test) { + bail!("Duplicate test: {}", old_test.name) + } + } + } + Ok(()) + } +} + +fn existing_tests(dir: &Path, ok: bool) -> Result> { + let mut res = HashMap::new(); + for file in fs::read_dir(dir)? { + let file = file?; + let path = file.path(); + if path.extension().unwrap_or_default() != "rs" { + continue; + } + let name = { + let file_name = path.file_name().unwrap().to_str().unwrap(); + file_name[5..file_name.len() - 3].to_string() + }; + let text = fs::read_to_string(&path)?; + let test = Test { + name: name.clone(), + text, + ok, + }; + if let Some(old) = res.insert(name, (path, test)) { + println!("Duplicate test: {:?}", old); + } + } + Ok(res) +} diff --git a/crates/tools/src/main.rs b/crates/tools/src/main.rs index d6eabce6c..c3e293911 100644 --- a/crates/tools/src/main.rs +++ b/crates/tools/src/main.rs @@ -1,30 +1,13 @@ -use std::{ - collections::HashMap, - fs, - path::{Path, PathBuf}, -}; - -use clap::{App, Arg, SubCommand}; -use failure::bail; +use clap::{App, SubCommand}; use tools::{ - collect_tests, generate,install_format_hook, run, run_rustfmt, - Mode, Overwrite, Result, Test, Verify, project_root, run_fuzzer + generate, gen_tests, install_format_hook, run, run_rustfmt, + Overwrite, Result, run_fuzzer, }; -const GRAMMAR_DIR: &str = "crates/ra_syntax/src/grammar"; -const OK_INLINE_TESTS_DIR: &str = "crates/ra_syntax/tests/data/parser/inline/ok"; -const ERR_INLINE_TESTS_DIR: &str = "crates/ra_syntax/tests/data/parser/inline/err"; - fn main() -> Result<()> { let matches = App::new("tasks") .setting(clap::AppSettings::SubcommandRequiredElseHelp) - .arg( - Arg::with_name("verify") - .long("--verify") - .help("Verify that generated code is up-to-date") - .global(true), - ) .subcommand(SubCommand::with_name("gen-syntax")) .subcommand(SubCommand::with_name("gen-tests")) .subcommand(SubCommand::with_name("install-code")) @@ -32,19 +15,14 @@ fn main() -> Result<()> { .subcommand(SubCommand::with_name("format-hook")) .subcommand(SubCommand::with_name("fuzz-tests")) .get_matches(); - let mode = if matches.is_present("verify") { - Verify - } else { - Overwrite - }; match matches .subcommand_name() .expect("Subcommand must be specified") { "install-code" => install_code_extension()?, - "gen-tests" => gen_tests(mode)?, + "gen-tests" => gen_tests(Overwrite)?, "gen-syntax" => generate(Overwrite)?, - "format" => run_rustfmt(mode)?, + "format" => run_rustfmt(Overwrite)?, "format-hook" => install_format_hook()?, "fuzz-tests" => run_fuzzer()?, _ => unreachable!(), @@ -52,101 +30,6 @@ fn main() -> Result<()> { Ok(()) } -fn gen_tests(mode: Mode) -> Result<()> { - let tests = tests_from_dir(Path::new(GRAMMAR_DIR))?; - fn install_tests(tests: &HashMap, into: &str, mode: Mode) -> Result<()> { - let tests_dir = project_root().join(into); - if !tests_dir.is_dir() { - fs::create_dir_all(&tests_dir)?; - } - // ok is never actually read, but it needs to be specified to create a Test in existing_tests - let existing = existing_tests(&tests_dir, true)?; - for t in existing.keys().filter(|&t| !tests.contains_key(t)) { - panic!("Test is deleted: {}", t); - } - - let mut new_idx = existing.len() + 1; - for (name, test) in tests { - let path = match existing.get(name) { - Some((path, _test)) => path.clone(), - None => { - let file_name = format!("{:04}_{}.rs", new_idx, name); - new_idx += 1; - tests_dir.join(file_name) - } - }; - teraron::update(&path, &test.text, mode)?; - } - Ok(()) - } - install_tests(&tests.ok, OK_INLINE_TESTS_DIR, mode)?; - install_tests(&tests.err, ERR_INLINE_TESTS_DIR, mode) -} - -#[derive(Default, Debug)] -struct Tests { - pub ok: HashMap, - pub err: HashMap, -} - -fn tests_from_dir(dir: &Path) -> Result { - let mut res = Tests::default(); - for entry in ::walkdir::WalkDir::new(dir) { - let entry = entry.unwrap(); - if !entry.file_type().is_file() { - continue; - } - if entry.path().extension().unwrap_or_default() != "rs" { - continue; - } - process_file(&mut res, entry.path())?; - } - let grammar_rs = dir.parent().unwrap().join("grammar.rs"); - process_file(&mut res, &grammar_rs)?; - return Ok(res); - fn process_file(res: &mut Tests, path: &Path) -> Result<()> { - let text = fs::read_to_string(path)?; - - for (_, test) in collect_tests(&text) { - if test.ok { - if let Some(old_test) = res.ok.insert(test.name.clone(), test) { - bail!("Duplicate test: {}", old_test.name) - } - } else { - if let Some(old_test) = res.err.insert(test.name.clone(), test) { - bail!("Duplicate test: {}", old_test.name) - } - } - } - Ok(()) - } -} - -fn existing_tests(dir: &Path, ok: bool) -> Result> { - let mut res = HashMap::new(); - for file in fs::read_dir(dir)? { - let file = file?; - let path = file.path(); - if path.extension().unwrap_or_default() != "rs" { - continue; - } - let name = { - let file_name = path.file_name().unwrap().to_str().unwrap(); - file_name[5..file_name.len() - 3].to_string() - }; - let text = fs::read_to_string(&path)?; - let test = Test { - name: name.clone(), - text, - ok, - }; - if let Some(old) = res.insert(name, (path, test)) { - println!("Duplicate test: {:?}", old); - } - } - Ok(res) -} - fn install_code_extension() -> Result<()> { run("cargo install --path crates/ra_lsp_server --force", ".")?; if cfg!(windows) { diff --git a/crates/tools/tests/cli.rs b/crates/tools/tests/cli.rs index 2d238d9ea..2ee4b5223 100644 --- a/crates/tools/tests/cli.rs +++ b/crates/tools/tests/cli.rs @@ -1,14 +1,22 @@ -extern crate tools; - -use tools::{generate, run_rustfmt, Verify}; +use tools::{generate, gen_tests, run_rustfmt, Verify}; #[test] -fn verify_template_generation() { +fn generated_grammar_is_fresh() { if let Err(error) = generate(Verify) { panic!("{}. Please update it by running `cargo gen-syntax`", error); } } +#[test] +fn generated_tests_are_fresh() { + if let Err(error) = gen_tests(Verify) { + panic!( + "{}. Please update tests by running `cargo gen-tests`", + error + ); + } +} + #[test] fn check_code_formatting() { if let Err(error) = run_rustfmt(Verify) { -- cgit v1.2.3