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

Implementation of #34168 #34186

Merged
merged 2 commits into from
Jun 21, 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
10 changes: 5 additions & 5 deletions src/librustc_privacy/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ E0445: r##"
A private trait was used on a public type parameter bound. Erroneous code
examples:
```compile_fail
```compile_fail,E0445
#![deny(private_in_public)]
trait Foo {
Expand Down Expand Up @@ -46,7 +46,7 @@ pub fn foo<T: Foo> (t: T) {} // ok!
E0446: r##"
A private type was used in a public type signature. Erroneous code example:
```compile_fail
```compile_fail,E0446
#![deny(private_in_public)]
mod Foo {
Expand Down Expand Up @@ -100,7 +100,7 @@ pub enum Foo {
Since the enum is already public, adding `pub` on one its elements is
unnecessary. Example:
```compile_fail
```compile_fail,
enum Foo {
pub Bar, // not ok!
}
Expand All @@ -119,7 +119,7 @@ E0450: r##"
A tuple constructor was invoked while some of its fields are private. Erroneous
code example:
```compile_fail
```compile_fail,E0450
mod Bar {
pub struct Foo(isize);
}
Expand Down Expand Up @@ -157,7 +157,7 @@ let f = bar::Foo::new(1);
E0451: r##"
A struct constructor with private fields was invoked. Erroneous code example:
```compile_fail
```compile_fail,E0451
mod Bar {
pub struct Foo {
pub a: isize,
Expand Down
55 changes: 37 additions & 18 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector) {
tests.add_test(text.to_owned(),
block_info.should_panic, block_info.no_run,
block_info.ignore, block_info.test_harness,
block_info.compile_fail);
block_info.compile_fail, block_info.error_codes);
}
}

Expand Down Expand Up @@ -454,6 +454,7 @@ struct LangString {
rust: bool,
test_harness: bool,
compile_fail: bool,
error_codes: Vec<String>,
}

impl LangString {
Expand All @@ -465,16 +466,22 @@ impl LangString {
rust: true, // NB This used to be `notrust = false`
test_harness: false,
compile_fail: false,
error_codes: Vec::new(),
}
}

fn parse(string: &str) -> LangString {
let mut seen_rust_tags = false;
let mut seen_other_tags = false;
let mut data = LangString::all_false();
let allow_compile_fail = match get_unstable_features_setting() {
UnstableFeatures::Allow | UnstableFeatures::Cheat=> true,
_ => false,
let mut allow_compile_fail = false;
let mut allow_error_code_check = false;
match get_unstable_features_setting() {
UnstableFeatures::Allow | UnstableFeatures::Cheat => {
allow_compile_fail = true;
allow_error_code_check = true;
}
_ => {},
};

let tokens = string.split(|c: char|
Expand All @@ -493,7 +500,15 @@ impl LangString {
data.compile_fail = true;
seen_rust_tags = true;
data.no_run = true;
},
}
x if allow_error_code_check && x.starts_with("E") && x.len() == 5 => {
if let Ok(_) = x[1..].parse::<u32>() {
data.error_codes.push(x.to_owned());
seen_rust_tags = true;
} else {
seen_other_tags = true;
}
}
_ => { seen_other_tags = true }
}
}
Expand Down Expand Up @@ -577,30 +592,34 @@ mod tests {
fn test_lang_string_parse() {
fn t(s: &str,
should_panic: bool, no_run: bool, ignore: bool, rust: bool, test_harness: bool,
compile_fail: bool) {
compile_fail: bool, error_codes: Vec<String>) {
assert_eq!(LangString::parse(s), LangString {
should_panic: should_panic,
no_run: no_run,
ignore: ignore,
rust: rust,
test_harness: test_harness,
compile_fail: compile_fail,
error_codes: error_codes,
})
}

// marker | should_panic| no_run| ignore| rust | test_harness| compile_fail
t("", false, false, false, true, false, false);
t("rust", false, false, false, true, false, false);
t("sh", false, false, false, false, false, false);
t("ignore", false, false, true, true, false, false);
t("should_panic", true, false, false, true, false, false);
t("no_run", false, true, false, true, false, false);
t("test_harness", false, false, false, true, true, false);
t("compile_fail", false, true, false, true, false, true);
t("{.no_run .example}", false, true, false, true, false, false);
t("{.sh .should_panic}", true, false, false, true, false, false);
t("{.example .rust}", false, false, false, true, false, false);
t("{.test_harness .rust}", false, false, false, true, true, false);
// | error_codes
t("", false, false, false, true, false, false, Vec::new());
Copy link
Member

Choose a reason for hiding this comment

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

Can't you leave these unchanged and always use vec![] in t? Or are there other calls I'm not seeing?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, you're absolutely right.

t("rust", false, false, false, true, false, false, Vec::new());
t("sh", false, false, false, false, false, false, Vec::new());
t("ignore", false, false, true, true, false, false, Vec::new());
t("should_panic", true, false, false, true, false, false, Vec::new());
t("no_run", false, true, false, true, false, false, Vec::new());
t("test_harness", false, false, false, true, true, false, Vec::new());
t("compile_fail", false, true, false, true, false, true, Vec::new());
t("E0450", false, false, false, true, false, false,
vec!("E0450".to_owned()));
t("{.no_run .example}", false, true, false, true, false, false, Vec::new());
t("{.sh .should_panic}", true, false, false, true, false, false, Vec::new());
t("{.example .rust}", false, false, false, true, false, false, Vec::new());
t("{.test_harness .rust}", false, false, false, true, true, false, Vec::new());
}

#[test]
Expand Down
26 changes: 21 additions & 5 deletions src/librustdoc/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ fn scrape_test_config(krate: &::rustc::hir::Crate) -> TestOptions {
fn runtest(test: &str, cratename: &str, cfgs: Vec<String>, libs: SearchPaths,
externs: core::Externs,
should_panic: bool, no_run: bool, as_test_harness: bool,
compile_fail: bool, opts: &TestOptions) {
compile_fail: bool, mut error_codes: Vec<String>, opts: &TestOptions) {
// the test harness wants its own `main` & top level functions, so
// never wrap the test in `fn main() { ... }`
let test = maketest(test, Some(cratename), as_test_harness, opts);
Expand Down Expand Up @@ -232,7 +232,7 @@ fn runtest(test: &str, cratename: &str, cfgs: Vec<String>, libs: SearchPaths,
None,
codemap.clone());
let old = io::set_panic(box Sink(data.clone()));
let _bomb = Bomb(data, old.unwrap_or(box io::stdout()));
let _bomb = Bomb(data.clone(), old.unwrap_or(box io::stdout()));

// Compile the code
let diagnostic_handler = errors::Handler::with_emitter(true, false, box emitter);
Expand Down Expand Up @@ -273,13 +273,28 @@ fn runtest(test: &str, cratename: &str, cfgs: Vec<String>, libs: SearchPaths,
} else if count == 0 && compile_fail == true {
panic!("test compiled while it wasn't supposed to")
}
if count > 0 && error_codes.len() > 0 {
let out = String::from_utf8(data.lock().unwrap().to_vec()).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

I think this may be the wrong approach for testing that error codes were emitted, we emit structured errors in the compiler, right? Couldn't we capture that and test against the error codes there directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

We do, though these errors tend to get emitted and forgotten. Unless you can think of a way I'm not thinking of?

I'm working on a refactor to move errors into their own crate and to be able to have multiple emits (default, expanded, json, etc). May make sense to let you buffer them up and test them, not sure.

error_codes.retain(|err| !out.contains(err));
}
}
Ok(()) if compile_fail => panic!("test compiled while it wasn't supposed to"),
_ => {}
}
}
Err(_) if compile_fail == false => panic!("couldn't compile the test"),
_ => {}
Err(_) => {
if compile_fail == false {
panic!("couldn't compile the test");
}
if error_codes.len() > 0 {
let out = String::from_utf8(data.lock().unwrap().to_vec()).unwrap();
error_codes.retain(|err| !out.contains(err));
}
}
}

if error_codes.len() > 0 {
panic!("Some expected error codes were not found: {:?}", error_codes);
}

if no_run { return }
Expand Down Expand Up @@ -411,7 +426,7 @@ impl Collector {

pub fn add_test(&mut self, test: String,
should_panic: bool, no_run: bool, should_ignore: bool,
as_test_harness: bool, compile_fail: bool) {
as_test_harness: bool, compile_fail: bool, error_codes: Vec<String>) {
let name = if self.use_headers {
let s = self.current_header.as_ref().map(|s| &**s).unwrap_or("");
format!("{}_{}", s, self.cnt)
Expand Down Expand Up @@ -442,6 +457,7 @@ impl Collector {
no_run,
as_test_harness,
compile_fail,
error_codes,
&opts);
})
});
Expand Down