From 9b3d806b0dbfcf76ff707aa86daba83454227720 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 3 Feb 2018 22:34:35 +0300 Subject: [PATCH 1/4] Add infra for inline tests --- .cargo/config | 1 + appveyor.yml | 1 + src/parser/event_parser/grammar/items/mod.rs | 4 + .../parser/inline/0001_const_unsafe_fn.rs | 1 + .../parser/inline/0001_const_unsafe_fn.txt | 15 ++ tests/data/parser/inline/0002_const_fn.rs | 1 + tests/data/parser/inline/0002_const_fn.txt | 13 ++ tests/data/parser/ok/0024_const_fn.rs | 5 - tests/data/parser/ok/0024_const_fn.txt | 29 ---- ...{0025_const_item.rs => 0024_const_item.rs} | 0 ...025_const_item.txt => 0024_const_item.txt} | 0 tests/parser.rs | 2 +- tools/Cargo.toml | 2 + tools/src/bin/collect-tests.rs | 134 ++++++++++++++++++ 14 files changed, 173 insertions(+), 35 deletions(-) create mode 100644 tests/data/parser/inline/0001_const_unsafe_fn.rs create mode 100644 tests/data/parser/inline/0001_const_unsafe_fn.txt create mode 100644 tests/data/parser/inline/0002_const_fn.rs create mode 100644 tests/data/parser/inline/0002_const_fn.txt delete mode 100644 tests/data/parser/ok/0024_const_fn.rs delete mode 100644 tests/data/parser/ok/0024_const_fn.txt rename tests/data/parser/ok/{0025_const_item.rs => 0024_const_item.rs} (100%) rename tests/data/parser/ok/{0025_const_item.txt => 0024_const_item.txt} (100%) create mode 100644 tools/src/bin/collect-tests.rs diff --git a/.cargo/config b/.cargo/config index 1ebc0f748cb4..7d89cf49041e 100644 --- a/.cargo/config +++ b/.cargo/config @@ -1,3 +1,4 @@ [alias] parse = "run --package tools --bin parse" gen = "run --package tools --bin gen" +collect-tests = "run --package tools --bin collect-tests --" diff --git a/appveyor.yml b/appveyor.yml index a6ba3b0e153a..8c7d118c82ed 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -10,6 +10,7 @@ install: build: false test_script: + - cargo collect-tests --verify - cargo test branches: diff --git a/src/parser/event_parser/grammar/items/mod.rs b/src/parser/event_parser/grammar/items/mod.rs index 8ccf8f90f9a2..5cf2fc39a496 100644 --- a/src/parser/event_parser/grammar/items/mod.rs +++ b/src/parser/event_parser/grammar/items/mod.rs @@ -52,11 +52,15 @@ fn item(p: &mut Parser) { STATIC_ITEM } CONST_KW => match p.nth(1) { + // test const_fn + // const fn foo() {} FN_KW => { p.bump(); fn_item(p); FN_ITEM } + // test const_unsafe_fn + // const unsafe fn foo() {} UNSAFE_KW if p.nth(2) == FN_KW => { p.bump(); p.bump(); diff --git a/tests/data/parser/inline/0001_const_unsafe_fn.rs b/tests/data/parser/inline/0001_const_unsafe_fn.rs new file mode 100644 index 000000000000..31a1e435f555 --- /dev/null +++ b/tests/data/parser/inline/0001_const_unsafe_fn.rs @@ -0,0 +1 @@ +const unsafe fn foo() {} diff --git a/tests/data/parser/inline/0001_const_unsafe_fn.txt b/tests/data/parser/inline/0001_const_unsafe_fn.txt new file mode 100644 index 000000000000..1f0865cb013d --- /dev/null +++ b/tests/data/parser/inline/0001_const_unsafe_fn.txt @@ -0,0 +1,15 @@ +FILE@[0; 25) + FN_ITEM@[0; 25) + CONST_KW@[0; 5) + WHITESPACE@[5; 6) + UNSAFE_KW@[6; 12) + WHITESPACE@[12; 13) + FN_KW@[13; 15) + WHITESPACE@[15; 16) + IDENT@[16; 19) "foo" + L_PAREN@[19; 20) + R_PAREN@[20; 21) + WHITESPACE@[21; 22) + L_CURLY@[22; 23) + R_CURLY@[23; 24) + WHITESPACE@[24; 25) diff --git a/tests/data/parser/inline/0002_const_fn.rs b/tests/data/parser/inline/0002_const_fn.rs new file mode 100644 index 000000000000..8c84d9cd7c42 --- /dev/null +++ b/tests/data/parser/inline/0002_const_fn.rs @@ -0,0 +1 @@ +const fn foo() {} diff --git a/tests/data/parser/inline/0002_const_fn.txt b/tests/data/parser/inline/0002_const_fn.txt new file mode 100644 index 000000000000..2d360d78bc43 --- /dev/null +++ b/tests/data/parser/inline/0002_const_fn.txt @@ -0,0 +1,13 @@ +FILE@[0; 18) + FN_ITEM@[0; 18) + CONST_KW@[0; 5) + WHITESPACE@[5; 6) + FN_KW@[6; 8) + WHITESPACE@[8; 9) + IDENT@[9; 12) "foo" + L_PAREN@[12; 13) + R_PAREN@[13; 14) + WHITESPACE@[14; 15) + L_CURLY@[15; 16) + R_CURLY@[16; 17) + WHITESPACE@[17; 18) diff --git a/tests/data/parser/ok/0024_const_fn.rs b/tests/data/parser/ok/0024_const_fn.rs deleted file mode 100644 index eba9322a1cac..000000000000 --- a/tests/data/parser/ok/0024_const_fn.rs +++ /dev/null @@ -1,5 +0,0 @@ -const fn foo() { -} - -const unsafe fn foo() { -} diff --git a/tests/data/parser/ok/0024_const_fn.txt b/tests/data/parser/ok/0024_const_fn.txt deleted file mode 100644 index 0fd485997295..000000000000 --- a/tests/data/parser/ok/0024_const_fn.txt +++ /dev/null @@ -1,29 +0,0 @@ -FILE@[0; 46) - FN_ITEM@[0; 20) - CONST_KW@[0; 5) - WHITESPACE@[5; 6) - FN_KW@[6; 8) - WHITESPACE@[8; 9) - IDENT@[9; 12) "foo" - L_PAREN@[12; 13) - R_PAREN@[13; 14) - WHITESPACE@[14; 15) - L_CURLY@[15; 16) - WHITESPACE@[16; 17) - R_CURLY@[17; 18) - WHITESPACE@[18; 20) - FN_ITEM@[20; 46) - CONST_KW@[20; 25) - WHITESPACE@[25; 26) - UNSAFE_KW@[26; 32) - WHITESPACE@[32; 33) - FN_KW@[33; 35) - WHITESPACE@[35; 36) - IDENT@[36; 39) "foo" - L_PAREN@[39; 40) - R_PAREN@[40; 41) - WHITESPACE@[41; 42) - L_CURLY@[42; 43) - WHITESPACE@[43; 44) - R_CURLY@[44; 45) - WHITESPACE@[45; 46) diff --git a/tests/data/parser/ok/0025_const_item.rs b/tests/data/parser/ok/0024_const_item.rs similarity index 100% rename from tests/data/parser/ok/0025_const_item.rs rename to tests/data/parser/ok/0024_const_item.rs diff --git a/tests/data/parser/ok/0025_const_item.txt b/tests/data/parser/ok/0024_const_item.txt similarity index 100% rename from tests/data/parser/ok/0025_const_item.txt rename to tests/data/parser/ok/0024_const_item.txt diff --git a/tests/parser.rs b/tests/parser.rs index f681c066f99c..68a6434bec19 100644 --- a/tests/parser.rs +++ b/tests/parser.rs @@ -7,7 +7,7 @@ use testutils::dir_tests; #[test] fn parser_tests() { - dir_tests(&["parser/ok", "parser/err"], |text| { + dir_tests(&["parser/inline", "parser/ok", "parser/err"], |text| { let tokens = tokenize(text); let file = parse(text.to_string(), &tokens); dump_tree(&file) diff --git a/tools/Cargo.toml b/tools/Cargo.toml index e46874929168..8cbc2fc93b05 100644 --- a/tools/Cargo.toml +++ b/tools/Cargo.toml @@ -9,4 +9,6 @@ serde = "1.0.26" serde_derive = "1.0.26" file = "1.1.1" ron = "0.1.5" +walkdir = "2" +itertools = "0.7" libsyntax2 = { path = "../" } diff --git a/tools/src/bin/collect-tests.rs b/tools/src/bin/collect-tests.rs new file mode 100644 index 000000000000..c54059e79609 --- /dev/null +++ b/tools/src/bin/collect-tests.rs @@ -0,0 +1,134 @@ +extern crate file; +extern crate walkdir; +extern crate itertools; + +use walkdir::WalkDir; +use itertools::Itertools; + +use std::path::{PathBuf, Path}; +use std::collections::HashSet; +use std::fs; + +fn main() { + let verify = ::std::env::args().any(|arg| arg == "--verify"); + + let d = grammar_dir(); + let tests = tests_from_dir(&d); + let existing = existing_tests(); + + for t in existing.difference(&tests) { + panic!("Test is deleted: {}\n{}", t.name, t.text); + } + + let new_tests = tests.difference(&existing); + for (i, t) in new_tests.enumerate() { + if verify { + panic!("Inline test is not recorded: {}", t.name); + } + + let name = format!("{:04}_{}.rs", existing.len() + i + 1, t.name); + println!("Creating {}", name); + let path = inline_tests_dir().join(name); + file::put_text(&path, &t.text).unwrap(); + } +} + + +#[derive(Debug, Eq)] +struct Test { + name: String, + text: String, +} + +impl PartialEq for Test { + fn eq(&self, other: &Test) -> bool { + self.name.eq(&other.name) + } +} + +impl ::std::hash::Hash for Test { + fn hash(&self, state: &mut H) { + self.name.hash(state) + } +} + +fn tests_from_dir(dir: &Path) -> HashSet { + let mut res = HashSet::new(); + for entry in WalkDir::new(dir) { + let entry = entry.unwrap(); + if !entry.file_type().is_file() { + continue + } + if entry.path().extension().unwrap_or_default() != "rs" { + continue + } + let text = file::get_text(entry.path()) + .unwrap(); + + for test in collect_tests(&text) { + if let Some(old_test) = res.replace(test) { + panic!("Duplicate test: {}", old_test.name) + } + } + } + res +} + +fn collect_tests(s: &str) -> Vec { + let mut res = vec![]; + let prefix = "// "; + let comment_blocks = s.lines() + .map(str::trim_left) + .group_by(|line| line.starts_with(prefix)); + + for (is_comment, block) in comment_blocks.into_iter() { + if !is_comment { + continue; + } + let mut block = block.map(|line| &line[prefix.len()..]); + let first = block.next().unwrap(); + if !first.starts_with("test ") { + continue + } + let name = first["test ".len()..].to_string(); + let text: String = itertools::join(block.chain(::std::iter::once("")), "\n"); + assert!(!text.trim().is_empty() && text.ends_with("\n")); + res.push(Test { name, text }) + } + res +} + +fn existing_tests() -> HashSet { + let mut res = HashSet::new(); + for file in fs::read_dir(&inline_tests_dir()).unwrap() { + let file = file.unwrap(); + let path = file.path(); + if path.extension().unwrap_or_default() != "rs" { + continue + } + let name = path.file_name().unwrap().to_str().unwrap(); + let name = name["0000_".len()..name.len() - 3].to_string(); + let text = file::get_text(&path).unwrap(); + res.insert(Test { name, text }); + } + res +} + +fn inline_tests_dir() -> PathBuf { + let res = base_dir().join("tests/data/parser/inline"); + if !res.is_dir() { + fs::create_dir_all(&res).unwrap(); + } + res +} + +fn grammar_dir() -> PathBuf { + base_dir().join("src/parser/event_parser/grammar") +} + +fn base_dir() -> PathBuf { + let dir = env!("CARGO_MANIFEST_DIR"); + PathBuf::from(dir).parent().unwrap().to_owned() +} + + From 2fd3228525858f594361eba345ff31087f8a917a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 3 Feb 2018 22:39:01 +0300 Subject: [PATCH 2/4] Document inline tests infra --- docs/TESTS.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/TESTS.md b/docs/TESTS.md index 8005ec9da62a..db06dbebc989 100644 --- a/docs/TESTS.md +++ b/docs/TESTS.md @@ -19,12 +19,26 @@ files to have the same name except for the leading number. In general, test suite should be append-only: old tests should not be modified, new tests should be created instead. - Note that only `ok` tests are normative: `err` tests test error recovery and it is totally ok for a parser to not implement any error recovery at all. However, for libsyntax2.0 we do care about error recovery, and we do care about precise and useful error messages. +There are also so-called "inline tests". They appear as the comments +with a `test` header in the source code, like this: + +```rust +// test fn_basic +// fn foo() {} +fn fn_item(p: &mut Parser) { + // ... +} +``` + +You can run `cargo collect-tests` command to collect all inline tests +into `tests/data/inline` directory. The main advantage of inline tests +is that they help to illustrate what the relevant code is doing. + Contribution opportunity: design and implement testing infrastructure for validators. From a5a6973df68a99a19176c717a337d5cdbf7c5629 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 3 Feb 2018 22:41:52 +0300 Subject: [PATCH 3/4] fmt --- tools/src/bin/collect-tests.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/tools/src/bin/collect-tests.rs b/tools/src/bin/collect-tests.rs index c54059e79609..df9d2db81583 100644 --- a/tools/src/bin/collect-tests.rs +++ b/tools/src/bin/collect-tests.rs @@ -1,11 +1,11 @@ extern crate file; -extern crate walkdir; extern crate itertools; +extern crate walkdir; use walkdir::WalkDir; use itertools::Itertools; -use std::path::{PathBuf, Path}; +use std::path::{Path, PathBuf}; use std::collections::HashSet; use std::fs; @@ -33,7 +33,6 @@ fn main() { } } - #[derive(Debug, Eq)] struct Test { name: String, @@ -57,13 +56,12 @@ fn tests_from_dir(dir: &Path) -> HashSet { for entry in WalkDir::new(dir) { let entry = entry.unwrap(); if !entry.file_type().is_file() { - continue + continue; } if entry.path().extension().unwrap_or_default() != "rs" { - continue + continue; } - let text = file::get_text(entry.path()) - .unwrap(); + let text = file::get_text(entry.path()).unwrap(); for test in collect_tests(&text) { if let Some(old_test) = res.replace(test) { @@ -88,7 +86,7 @@ fn collect_tests(s: &str) -> Vec { let mut block = block.map(|line| &line[prefix.len()..]); let first = block.next().unwrap(); if !first.starts_with("test ") { - continue + continue; } let name = first["test ".len()..].to_string(); let text: String = itertools::join(block.chain(::std::iter::once("")), "\n"); @@ -104,7 +102,7 @@ fn existing_tests() -> HashSet { let file = file.unwrap(); let path = file.path(); if path.extension().unwrap_or_default() != "rs" { - continue + continue; } let name = path.file_name().unwrap().to_str().unwrap(); let name = name["0000_".len()..name.len() - 3].to_string(); @@ -130,5 +128,3 @@ fn base_dir() -> PathBuf { let dir = env!("CARGO_MANIFEST_DIR"); PathBuf::from(dir).parent().unwrap().to_owned() } - - From b072e68ad5bf1687aebd2ff1c7bf327d38a6a2f2 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 3 Feb 2018 22:44:17 +0300 Subject: [PATCH 4/4] More docs --- docs/TOOLS.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/TOOLS.md b/docs/TOOLS.md index 1fcfa2dec89e..f8754c06fe42 100644 --- a/docs/TOOLS.md +++ b/docs/TOOLS.md @@ -17,14 +17,20 @@ cargo tool ``` -# Tool: `gen` +## Tool: `gen` This tool reads a "grammar" from [grammar.ron](../grammar.ron) and generates the `syntax_kinds.rs` file. You should run this tool if you add new keywords or syntax elements. -# Tool: 'parse' +## Tool: `parse` This tool reads rust source code from the standard input, parses it, and prints the result to stdout. + + +## Tool: `collect-tests` + +This tools collect inline tests from comments in libsyntax2 source code +and places them into `tests/data/inline` directory.