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.

Sign up for GitHub

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

libtest: add --show-output flag to print stdout of successful tests #62600

Merged
merged 2 commits into from
Aug 27, 2019
Merged

libtest: add --show-output flag to print stdout of successful tests #62600

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1add949
libtest: add --show-output option
Jul 11, 2019
409a41d
libtest: support display_output in JSON formatter
Jul 11, 2019
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
2 changes: 1 addition & 1 deletion src/librustdoc/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,6 @@ pub fn test(mut options: Options, diag: &errors::Handler) -> i32 {

options.test_args.insert(0, "rustdoctest".to_string());
testing::test_main(&options.test_args, collector.tests,
testing::Options::new().display_output(options.display_warnings));
Some(testing::Options::new().display_output(options.display_warnings)));
0
}
2 changes: 1 addition & 1 deletion src/librustdoc/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub fn run(options: Options) -> i32 {
testing::test_main(
&test_args,
tests,
testing::Options::new().display_output(display_warnings)
Some(testing::Options::new().display_output(display_warnings))
);

0
Expand Down
16 changes: 11 additions & 5 deletions src/libtest/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ impl Options {

// The default console test runner. It accepts the command line
// arguments and a vector of test_descs.
pub fn test_main(args: &[String], tests: Vec<TestDescAndFn>, options: Options) {
pub fn test_main(args: &[String], tests: Vec<TestDescAndFn>, options: Option<Options>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really sure why this change is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two places that call this function internally are:

  • librustdoc, which gets the display_output options from some config somewhere else, so we don't want to override it from the commandline parameter here (at least not flag not present overring the config), so can't override display_output
  • test_main_static, which wants to get display_output from the not yet parsed args list, so we need to set it here from the command line

I guess it would be possible to change librustdoc to add/remove the --show-output flag in the args array when its internal config option is present/not present, but i think having options as an Option to override this is nicer than fiddling with cli args.

So the actual problem is that there are basically two structs storing test options, it's overall messy. I didn't want to make a big change to that logic here

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. I just took a look and unifying these isn't trivial.

let mut opts = match parse_opts(args) {
Some(Ok(o)) => o,
Some(Err(msg)) => {
Expand All @@ -281,8 +281,9 @@ pub fn test_main(args: &[String], tests: Vec<TestDescAndFn>, options: Options) {
}
None => return,
};

opts.options = options;
if let Some(options) = options {
opts.options = options;
}
if opts.list {
if let Err(e) = list_tests_console(&opts, tests) {
eprintln!("error: io error when listing tests: {:?}", e);
Expand Down Expand Up @@ -323,7 +324,7 @@ pub fn test_main_static(tests: &[&TestDescAndFn]) {
_ => panic!("non-static tests passed to test::test_main_static"),
})
.collect();
test_main(&args, owned_tests, Options::new())
test_main(&args, owned_tests, None)
}

/// Invoked when unit tests terminate. Should panic if the unit
Expand Down Expand Up @@ -468,6 +469,11 @@ fn optgroups() -> getopts::Options {
json = Output a json document",
"pretty|terse|json",
)
.optflag(
"",
"show-output",
"Show captured stdout of successful tests"
)
.optopt(
"Z",
"",
Expand Down Expand Up @@ -667,7 +673,7 @@ pub fn parse_opts(args: &[String]) -> Option<OptRes> {
format,
test_threads,
skip: matches.opt_strs("skip"),
options: Options::new(),
options: Options::new().display_output(matches.opt_present("show-output")),
};

Some(Ok(test_opts))
Expand Down
11 changes: 11 additions & 0 deletions src/libtest/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,17 @@ fn parse_ignored_flag() {
assert_eq!(opts.run_ignored, RunIgnored::Only);
}

#[test]
fn parse_show_output_flag() {
let args = vec![
"progname".to_string(),
"filter".to_string(),
"--show-output".to_string(),
];
let opts = parse_opts(&args).unwrap().unwrap();
assert!(opts.options.display_output);
}

#[test]
fn parse_include_ignored_flag() {
let args = vec![
Expand Down