#[test] attribute and related changes #218
Conversation
| @@ -174,6 +178,10 @@ async fn try_main() -> Result<ExitCode> { | |||
| options.parse_option(opt)?; | |||
| } | |||
|
|
|||
| if args.test { | |||
| options.test(true); | |||
| } | |||
There was a problem hiding this comment.
options.test(args.test)
| Err(e) => { | ||
| // TODO: store output here | ||
| failures.insert(test.1.item.item.clone(), FailureReason::Crash(e)); | ||
| writeln!(out, "crashed")?; |
| println!("Return value: {:?}\n", e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I think moving all of this information into a test-case struct would make this clearer.
| tests.len(), | ||
| failure_count, | ||
| elapsed.as_secs_f64() | ||
| ); |
There was a problem hiding this comment.
Should this go above or below the detailed output?
| format!("type mismatch for operation `{}`", op), | ||
| vec![ | ||
| format!("left hand side has type `{}`", lhs), | ||
| format!("rhs hand side has type `{}`", rhs), |
| let line_range = files.line_range(frame.source_id, line).expect("a range"); | ||
| let source = files.get(frame.source_id)?; | ||
| let name = source.name(); | ||
| let slice = &source.source()[line_range]; |
There was a problem hiding this comment.
cleanup this with a helper function
There was a problem hiding this comment.
Maybe this should take the span from the inst instead... This is a bit odd right now with closures, you can end up repeating the same multiple times.
| // span, | ||
| // "#[test] is not supported on nested", | ||
| // )); | ||
| // } |
There was a problem hiding this comment.
remove this commented code
| @@ -712,7 +740,7 @@ impl Index for ast::ItemFn { | |||
| }; | |||
|
|
|||
| idx.query.insert_meta(span, meta)?; | |||
| } else if is_toplevel && item.visibility.is_public() { | |||
| } else if is_toplevel && item.visibility.is_public() || is_test { | |||
There was a problem hiding this comment.
this isn't enough to make #[test] in mods auto-discovered, we need to be even more eager about them.
| @@ -194,6 +197,9 @@ pub enum CompileMetaKind { | |||
| ConstFn { | |||
| /// Opaque identifier for the constant function. | |||
| id: Id, | |||
|
|
|||
| /// Whether this function has a test annotation | |||
| is_test: bool, | |||
There was a problem hiding this comment.
@udoprog I think you had opinions on this. See rust-lang/rust#51999 and related threads for rust background on static contexts and asserts. Seems like a reduced feature set is supported, which allows implementing tests as const fn. Not sure whether it is required to interact with the test system - I guess by definition any assert in const code blocks the build, which precludes testing it. It'd just become weird if you want to test the code and you end failing tests during compilation...
| } else { | ||
| write!(f, "{{root}}") | ||
| formatter.pad("{root}") |
There was a problem hiding this comment.
This is ugly, but I found no better way - this makes sure padding specifiers in format strings work as intended.
|
Ok, merging it in since it's fairly isolated and patching it up in follow ups. Thanks! |
udoprog
added
changelog
This does the following:
I don't think there are any major issues with the current impl so I'd like to merge this (after some cleanup) and open a bunch of issues for follow-up work. If you think any of these are blockers for merging (or something else) I'm open to discussion! I'm starting to shoehorn more and more things into this PR because the main feature is done, which makes it harder to work with and review.
These are good topics for follow-up work:
testas a subcommand instead of a flag (and addrune runas the default command)--shortmode to tests (...F..N.E...Fstyle)--no-capture