Skip to content

simplify tests #908

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 1 commit into from
Apr 5, 2016
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// rustfmt-write_mode: coverage
/// Here's a doc comment!
fn main() {
// foo is bar
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
XX XXXXXXXXXXXXXXXXXXX XXXXXXXX
/// Here's a doc comment!
fn main() {
XX XXX XX XXX
Expand Down
78 changes: 31 additions & 47 deletions tests/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::path::{Path, PathBuf};

use rustfmt::*;
use rustfmt::filemap::{write_system_newlines, FileMap};
use rustfmt::config::{Config, ReportTactic, WriteMode};
use rustfmt::config::{Config, ReportTactic};
use rustfmt::rustfmt_diff::*;

const DIFF_CONTEXT_SIZE: usize = 3;
Expand All @@ -44,7 +44,7 @@ fn system_tests() {
// Turn a DirEntry into a String that represents the relative path to the
// file.
let files = files.map(get_path_string);
let (_reports, count, fails) = check_files(files, None);
let (_reports, count, fails) = check_files(files);

// Display results.
println!("Ran {} system tests.", count);
Expand All @@ -55,26 +55,26 @@ fn system_tests() {
// the only difference is the coverage mode
#[test]
fn coverage_tests() {
let files = fs::read_dir("tests/coverage-source").expect("Couldn't read source dir");
let files = fs::read_dir("tests/coverage/source").expect("Couldn't read source dir");
let files = files.map(get_path_string);
let (_reports, count, fails) = check_files(files, Some(WriteMode::Coverage));
let (_reports, count, fails) = check_files(files);

println!("Ran {} tests in coverage mode.", count);
assert!(fails == 0, "{} tests failed", fails);
}

#[test]
fn checkstyle_test() {
let filename = "tests/source/fn-single-line.rs";
let expected_filename = "tests/writemode/checkstyle.xml";
assert_output(filename, expected_filename, Some(WriteMode::Checkstyle));
let filename = "tests/writemode/source/fn-single-line.rs";
let expected_filename = "tests/writemode/target/checkstyle.xml";
assert_output(filename, expected_filename);
}


// Helper function for comparing the results of rustfmt
// to a known output file generated by one of the write modes.
fn assert_output(source: &str, expected_filename: &str, write_mode: Option<WriteMode>) {
let config = read_config(&source, write_mode);
fn assert_output(source: &str, expected_filename: &str) {
let config = read_config(&source);
let (file_map, _report) = format_file(source, &config);

// Populate output by writing to a vec.
Expand Down Expand Up @@ -104,7 +104,7 @@ fn idempotence_tests() {
let files = fs::read_dir("tests/target")
.expect("Couldn't read target dir")
.map(get_path_string);
let (_reports, count, fails) = check_files(files, None);
let (_reports, count, fails) = check_files(files);

// Display results.
println!("Ran {} idempotent tests.", count);
Expand All @@ -122,7 +122,7 @@ fn self_tests() {
// Hack because there's no `IntoIterator` impl for `[T; N]`.
let files = files.chain(Some("src/lib.rs".to_owned()).into_iter());

let (reports, count, fails) = check_files(files, None);
let (reports, count, fails) = check_files(files);
let mut warnings = 0;

// Display results.
Expand All @@ -141,7 +141,7 @@ fn self_tests() {

// For each file, run rustfmt and collect the output.
// Returns the number of files checked and the number of failures.
fn check_files<I>(files: I, write_mode: Option<WriteMode>) -> (Vec<FormatReport>, u32, u32)
fn check_files<I>(files: I) -> (Vec<FormatReport>, u32, u32)
where I: Iterator<Item = String>
{
let mut count = 0;
Expand All @@ -151,7 +151,7 @@ fn check_files<I>(files: I, write_mode: Option<WriteMode>) -> (Vec<FormatReport>
for file_name in files.filter(|f| f.ends_with(".rs")) {
println!("Testing '{}'...", file_name);

match idempotent_check(file_name, write_mode) {
match idempotent_check(file_name) {
Ok(report) => reports.push(report),
Err(msg) => {
print_mismatches(msg);
Expand All @@ -176,7 +176,7 @@ fn print_mismatches(result: HashMap<String, Vec<Mismatch>>) {
assert!(t.reset().unwrap());
}

fn read_config(filename: &str, write_mode: Option<WriteMode>) -> Config {
fn read_config(filename: &str) -> Config {
let sig_comments = read_significant_comments(&filename);
let mut config = get_config(sig_comments.get("config").map(|x| &(*x)[..]));

Expand All @@ -189,10 +189,6 @@ fn read_config(filename: &str, write_mode: Option<WriteMode>) -> Config {
// Don't generate warnings for to-do items.
config.report_todo = ReportTactic::Never;

if let Some(mode) = write_mode {
config.write_mode = mode
}

config
}

Expand All @@ -201,11 +197,9 @@ fn format_file<P: Into<PathBuf>>(filename: P, config: &Config) -> (FileMap, Form
format_input(input, &config)
}

pub fn idempotent_check(filename: String,
write_mode: Option<WriteMode>)
-> Result<FormatReport, HashMap<String, Vec<Mismatch>>> {
pub fn idempotent_check(filename: String) -> Result<FormatReport, HashMap<String, Vec<Mismatch>>> {
let sig_comments = read_significant_comments(&filename);
let config = read_config(&filename, write_mode);
let config = read_config(&filename);
let (file_map, format_report) = format_file(filename, &config);

let mut write_result = HashMap::new();
Expand All @@ -220,7 +214,7 @@ pub fn idempotent_check(filename: String,

let target = sig_comments.get("target").map(|x| &(*x)[..]);

handle_result(write_result, target, write_mode).map(|_| format_report)
handle_result(write_result, target).map(|_| format_report)
}

// Reads test config file from comments and reads its contents.
Expand Down Expand Up @@ -268,14 +262,13 @@ fn read_significant_comments(file_name: &str) -> HashMap<String, String> {
// Compare output to input.
// TODO: needs a better name, more explanation.
fn handle_result(result: HashMap<String, String>,
target: Option<&str>,
write_mode: Option<WriteMode>)
target: Option<&str>)
-> Result<(), HashMap<String, Vec<Mismatch>>> {
let mut failures = HashMap::new();

for (file_name, fmt_text) in result {
// If file is in tests/source, compare to file with same name in tests/target.
let target = get_target(&file_name, target, write_mode);
let target = get_target(&file_name, target);
let mut f = fs::File::open(&target).expect("Couldn't open target");

let mut text = String::new();
Expand All @@ -297,29 +290,20 @@ fn handle_result(result: HashMap<String, String>,
}

// Map source file paths to their target paths.
fn get_target(file_name: &str, target: Option<&str>, write_mode: Option<WriteMode>) -> String {
let file_path = Path::new(file_name);
let (source_path_prefix, target_path_prefix) = match write_mode {
Some(WriteMode::Coverage) => {
(Path::new("tests/coverage-source/"), "tests/coverage-target/")
fn get_target(file_name: &str, target: Option<&str>) -> String {
if file_name.contains("source") {
let target_file_name = file_name.replace("source", "target");
if let Some(replace_name) = target {
Path::new(&target_file_name)
.with_file_name(replace_name)
.into_os_string()
.into_string()
.unwrap()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if branch does not work for all possible file names because it manipulates strings and not Paths, but I think this is OK: we are not evil enough to create edge cases for ourselves =)

} else {
target_file_name
}
_ => (Path::new("tests/source/"), "tests/target/"),
};

if file_path.starts_with(source_path_prefix) {
let mut components = file_path.components();
// Can't skip(2) as the resulting iterator can't as_path()
components.next();
components.next();

let new_target = match components.as_path().to_str() {
Some(string) => string,
None => file_name,
};
let base = target.unwrap_or(new_target);

format!("{}{}", target_path_prefix, base)
} else {
// This is either and idempotence check or a self check
file_name.to_owned()
}
}
Expand Down
2 changes: 0 additions & 2 deletions tests/writemode/checkstyle.xml

This file was deleted.

80 changes: 80 additions & 0 deletions tests/writemode/source/fn-single-line.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// rustfmt-fn_single_line: true
// rustfmt-write_mode: checkstyle
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a copy of usual test case file with write-mode option added

// Test single-line functions.

fn foo_expr() {
1
}

fn foo_stmt() {
foo();
}

fn foo_decl_local() {
let z = 5;
}

fn foo_decl_item(x: &mut i32) {
x = 3;
}

fn empty() {

}

fn foo_return() -> String {
"yay"
}

fn foo_where() -> T where T: Sync {
let x = 2;
}

fn fooblock() {
{
"inner-block"
}
}

fn fooblock2(x: i32) {
let z = match x {
_ => 2,
};
}

fn comment() {
// this is a test comment
1
}

fn comment2() {
// multi-line comment
let z = 2;
1
}

fn only_comment() {
// Keep this here
}

fn aaaaaaaaaaaaaaaaa_looooooooooooooooooooooong_name() {
let z = "aaaaaaawwwwwwwwwwwwwwwwwwwwwwwwwwww";
}

fn lots_of_space () {
1
}

fn mac() -> Vec<i32> { vec![] }

trait CoolTypes {
fn dummy(&self) {
}
}

trait CoolerTypes { fn dummy(&self) {
}
}

fn Foo<T>() where T: Bar {
}
2 changes: 2 additions & 0 deletions tests/writemode/target/checkstyle.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<?xml version="1.0" encoding="utf-8"?>
<checkstyle version="4.3"><file name="tests/writemode/source/fn-single-line.rs"><error line="2" severity="warning" message="Should be `fn foo_expr() { 1 }`" /><error line="2" severity="warning" message="Should be `fn foo_stmt() { foo(); }`" /><error line="2" severity="warning" message="Should be `fn foo_decl_local() { let z = 5; }`" /><error line="2" severity="warning" message="Should be `fn foo_decl_item(x: &amp;mut i32) { x = 3; }`" /><error line="2" severity="warning" message="Should be `fn empty() {}`" /><error line="2" severity="warning" message="Should be `fn foo_return() -&gt; String { &quot;yay&quot; }`" /><error line="2" severity="warning" message="Should be `fn foo_where() -&gt; T`" /><error line="2" severity="warning" message="Should be ` where T: Sync`" /><error line="2" severity="warning" message="Should be `{`" /><error line="51" severity="warning" message="Should be `fn lots_of_space() { 1 }`" /><error line="58" severity="warning" message="Should be ` fn dummy(&amp;self) {}`" /><error line="58" severity="warning" message="Should be `trait CoolerTypes {`" /><error line="58" severity="warning" message="Should be ` fn dummy(&amp;self) {}`" /><error line="58" severity="warning" message="Should be `fn Foo&lt;T&gt;() where T: Bar {}`" /></file></checkstyle>