Rollup merge of rust-lang#62600 - emmericp:libtest-add-show-output, r=gnzlbg

Centril · web-flow · commit 6d20265a9e8c · 2019-08-27T08:17:48.000+02:00
libtest: add --show-output flag to print stdout of successful tests This pull request adds a new flag `--show-output` for tests to show the output of successful tests. For most formatters this was already supported just not exposed via the CLI (apparently only used by `librustdoc`). I've also added support for this option in the JSON formatter. This kind of fixes rust-lang#54669 which wants `--format json` to work with `--nocapture`, which is... well, impossible. What this issue really calls for is `--show-output` as implemented here.
diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs
@@ -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
 }
diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs
@@ -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
diff --git a/src/libtest/formatters/json.rs b/src/libtest/formatters/json.rs
@@ -9,44 +9,57 @@ impl<T: Write> JsonFormatter<T> {
         Self { out }
     }
 
-    fn write_message(&mut self, s: &str) -> io::Result<()> {
+    fn writeln_message(&mut self, s: &str) -> io::Result<()> {
         assert!(!s.contains('\n'));
 
         self.out.write_all(s.as_ref())?;
         self.out.write_all(b"\n")
     }
 
+    fn write_message(&mut self, s: &str) -> io::Result<()> {
+        assert!(!s.contains('\n'));
+
+        self.out.write_all(s.as_ref())
+    }
+
     fn write_event(
         &mut self,
         ty: &str,
         name: &str,
         evt: &str,
-        extra: Option<String>,
+        stdout: Option<Cow<'_, str>>,
+        extra: Option<&str>,
     ) -> io::Result<()> {
-        if let Some(extras) = extra {
+        self.write_message(&*format!(
+            r#"{{ "type": "{}", "name": "{}", "event": "{}""#,
+            ty, name, evt
+        ))?;
+        if let Some(stdout) = stdout {
             self.write_message(&*format!(
-                r#"{{ "type": "{}", "name": "{}", "event": "{}", {} }}"#,
-                ty, name, evt, extras
-            ))
-        } else {
+                r#", "stdout": "{}""#,
+                EscapedString(stdout)
+            ))?;
+        }
+        if let Some(extra) = extra {
             self.write_message(&*format!(
-                r#"{{ "type": "{}", "name": "{}", "event": "{}" }}"#,
-                ty, name, evt
-            ))
+                r#", {}"#,
+                extra
+            ))?;
         }
+        self.writeln_message(" }")
     }
 }
 
 impl<T: Write> OutputFormatter for JsonFormatter<T> {
     fn write_run_start(&mut self, test_count: usize) -> io::Result<()> {
-        self.write_message(&*format!(
+        self.writeln_message(&*format!(
             r#"{{ "type": "suite", "event": "started", "test_count": {} }}"#,
             test_count
         ))
     }
 
     fn write_test_start(&mut self, desc: &TestDesc) -> io::Result<()> {
-        self.write_message(&*format!(
+        self.writeln_message(&*format!(
             r#"{{ "type": "test", "event": "started", "name": "{}" }}"#,
             desc.name
         ))
@@ -57,34 +70,30 @@ impl<T: Write> OutputFormatter for JsonFormatter<T> {
         desc: &TestDesc,
         result: &TestResult,
         stdout: &[u8],
+        state: &ConsoleTestState,
     ) -> io::Result<()> {
+        let stdout = if (state.options.display_output || *result != TrOk) && stdout.len() > 0 {
+            Some(String::from_utf8_lossy(stdout))
+        } else {
+            None
+        };
         match *result {
-            TrOk => self.write_event("test", desc.name.as_slice(), "ok", None),
-
-            TrFailed => {
-                let extra_data = if stdout.len() > 0 {
-                    Some(format!(
-                        r#""stdout": "{}""#,
-                        EscapedString(String::from_utf8_lossy(stdout))
-                    ))
-                } else {
-                    None
-                };
+            TrOk => self.write_event("test", desc.name.as_slice(), "ok", stdout, None),
 
-                self.write_event("test", desc.name.as_slice(), "failed", extra_data)
-            }
+            TrFailed => self.write_event("test", desc.name.as_slice(), "failed", stdout, None),
 
             TrFailedMsg(ref m) => self.write_event(
                 "test",
                 desc.name.as_slice(),
                 "failed",
-                Some(format!(r#""message": "{}""#, EscapedString(m))),
+                stdout,
+                Some(&*format!(r#""message": "{}""#, EscapedString(m))),
             ),
 
-            TrIgnored => self.write_event("test", desc.name.as_slice(), "ignored", None),
+            TrIgnored => self.write_event("test", desc.name.as_slice(), "ignored", stdout, None),
 
             TrAllowedFail => {
-                self.write_event("test", desc.name.as_slice(), "allowed_failure", None)
+                self.write_event("test", desc.name.as_slice(), "allowed_failure", stdout, None)
             }
 
             TrBench(ref bs) => {
@@ -105,20 +114,20 @@ impl<T: Write> OutputFormatter for JsonFormatter<T> {
                     desc.name, median, deviation, mbps
                 );
 
-                self.write_message(&*line)
+                self.writeln_message(&*line)
             }
         }
     }
 
     fn write_timeout(&mut self, desc: &TestDesc) -> io::Result<()> {
-        self.write_message(&*format!(
+        self.writeln_message(&*format!(
             r#"{{ "type": "test", "event": "timeout", "name": "{}" }}"#,
             desc.name
         ))
     }
 
     fn write_run_finish(&mut self, state: &ConsoleTestState) -> io::Result<bool> {
-        self.write_message(&*format!(
+        self.writeln_message(&*format!(
             "{{ \"type\": \"suite\", \
              \"event\": \"{}\", \
              \"passed\": {}, \
diff --git a/src/libtest/formatters/mod.rs b/src/libtest/formatters/mod.rs
@@ -17,6 +17,7 @@ pub(crate) trait OutputFormatter {
         desc: &TestDesc,
         result: &TestResult,
         stdout: &[u8],
+        state: &ConsoleTestState,
     ) -> io::Result<()>;
     fn write_run_finish(&mut self, state: &ConsoleTestState) -> io::Result<bool>;
 }
diff --git a/src/libtest/formatters/pretty.rs b/src/libtest/formatters/pretty.rs
@@ -162,7 +162,13 @@ impl<T: Write> OutputFormatter for PrettyFormatter<T> {
         Ok(())
     }
 
-    fn write_result(&mut self, desc: &TestDesc, result: &TestResult, _: &[u8]) -> io::Result<()> {
+    fn write_result(
+        &mut self,
+        desc: &TestDesc,
+        result: &TestResult,
+        _: &[u8],
+        _: &ConsoleTestState,
+    ) -> io::Result<()> {
         if self.is_multithreaded {
             self.write_test_name(desc)?;
         }
diff --git a/src/libtest/formatters/terse.rs b/src/libtest/formatters/terse.rs
@@ -170,7 +170,13 @@ impl<T: Write> OutputFormatter for TerseFormatter<T> {
         Ok(())
     }
 
-    fn write_result(&mut self, desc: &TestDesc, result: &TestResult, _: &[u8]) -> io::Result<()> {
+    fn write_result(
+        &mut self,
+        desc: &TestDesc,
+        result: &TestResult,
+        _: &[u8],
+        _: &ConsoleTestState,
+    ) -> io::Result<()> {
         match *result {
             TrOk => self.write_ok(),
             TrFailed | TrFailedMsg(_) => self.write_failed(),
diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs
@@ -274,7 +274,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>) {
     let mut opts = match parse_opts(args) {
         Some(Ok(o)) => o,
         Some(Err(msg)) => {
@@ -283,8 +283,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);
@@ -325,7 +326,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
@@ -448,6 +449,11 @@ fn optgroups() -> getopts::Options {
             json   = Output a json document",
             "pretty|terse|json",
         )
+        .optflag(
+            "",
+            "show-output",
+            "Show captured stdout of successful tests"
+        )
         .optopt(
             "Z",
             "",
@@ -647,7 +653,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))
@@ -880,7 +886,7 @@ pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Resu
             TeTimeout(ref test) => out.write_timeout(test),
             TeResult(test, result, stdout) => {
                 st.write_log_result(&test, &result)?;
-                out.write_result(&test, &result, &*stdout)?;
+                out.write_result(&test, &result, &*stdout, &st)?;
                 match result {
                     TrOk => {
                         st.passed += 1;
diff --git a/src/libtest/tests.rs b/src/libtest/tests.rs
@@ -180,6 +180,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![
diff --git a/src/test/run-make-fulldeps/libtest-json/Makefile b/src/test/run-make-fulldeps/libtest-json/Makefile
@@ -2,13 +2,17 @@
 
 # Test expected libtest's JSON output
 
-OUTPUT_FILE := $(TMPDIR)/libtest-json-output.json
+OUTPUT_FILE_DEFAULT := $(TMPDIR)/libtest-json-output-default.json
+OUTPUT_FILE_STDOUT_SUCCESS := $(TMPDIR)/libtest-json-output-stdout-success.json
 
 all:
 	$(RUSTC) --test f.rs
-	RUST_BACKTRACE=0 $(call RUN,f) -Z unstable-options --test-threads=1 --format=json > $(OUTPUT_FILE) || true
+	RUST_BACKTRACE=0 $(call RUN,f) -Z unstable-options --test-threads=1 --format=json > $(OUTPUT_FILE_DEFAULT) || true
+	RUST_BACKTRACE=0 $(call RUN,f) -Z unstable-options --test-threads=1 --format=json --show-output > $(OUTPUT_FILE_STDOUT_SUCCESS) || true
 
-	cat $(OUTPUT_FILE) | "$(PYTHON)" validate_json.py
+	cat $(OUTPUT_FILE_DEFAULT) | "$(PYTHON)" validate_json.py
+	cat $(OUTPUT_FILE_STDOUT_SUCCESS) | "$(PYTHON)" validate_json.py
 
 	# Compare to output file
-	diff output.json $(OUTPUT_FILE)
+	diff output-default.json $(OUTPUT_FILE_DEFAULT)
+	diff output-stdout-success.json $(OUTPUT_FILE_STDOUT_SUCCESS)
diff --git a/src/test/run-make-fulldeps/libtest-json/f.rs b/src/test/run-make-fulldeps/libtest-json/f.rs
@@ -1,11 +1,12 @@
 #[test]
 fn a() {
+    println!("print from successful test");
     // Should pass
 }
 
 #[test]
 fn b() {
-    assert!(false)
+    assert!(false);
 }
 
 #[test]
diff --git a/src/test/run-make-fulldeps/libtest-json/output-default.json b/src/test/run-make-fulldeps/libtest-json/output-default.json
@@ -2,7 +2,7 @@
 { "type": "test", "event": "started", "name": "a" }
 { "type": "test", "name": "a", "event": "ok" }
 { "type": "test", "event": "started", "name": "b" }
-{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:8:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.\n" }
+{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.\n" }
 { "type": "test", "event": "started", "name": "c" }
 { "type": "test", "name": "c", "event": "ok" }
 { "type": "test", "event": "started", "name": "d" }
diff --git a/src/test/run-make-fulldeps/libtest-json/output-stdout-success.json b/src/test/run-make-fulldeps/libtest-json/output-stdout-success.json
@@ -0,0 +1,10 @@
+{ "type": "suite", "event": "started", "test_count": 4 }
+{ "type": "test", "event": "started", "name": "a" }
+{ "type": "test", "name": "a", "event": "ok", "stdout": "print from successful test\n" }
+{ "type": "test", "event": "started", "name": "b" }
+{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.\n" }
+{ "type": "test", "event": "started", "name": "c" }
+{ "type": "test", "name": "c", "event": "ok", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:15:5\n" }
+{ "type": "test", "event": "started", "name": "d" }
+{ "type": "test", "name": "d", "event": "ignored" }
+{ "type": "suite", "event": "failed", "passed": 2, "failed": 1, "allowed_fail": 0, "ignored": 1, "measured": 0, "filtered_out": 0 }

Original file line number	Diff line number	Diff line change
`@@ -154,6 +154,6 @@ pub fn test(mut options: Options, diag: &errors::Handler) -> i32 {`
`154`	`154`
`155`	`155`	`options.test_args.insert(0, "rustdoctest".to_string());`
`156`	`156`	`testing::test_main(&options.test_args, collector.tests,`
`157`		`- testing::Options::new().display_output(options.display_warnings));`
	`157`	`+ Some(testing::Options::new().display_output(options.display_warnings)));`
`158`	`158`	`0`
`159`	`159`	`}`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ pub fn run(options: Options) -> i32 {`
`120`	`120`	`testing::test_main(`
`121`	`121`	`&test_args,`
`122`	`122`	`tests,`
`123`		`- testing::Options::new().display_output(display_warnings)`
	`123`	`+ Some(testing::Options::new().display_output(display_warnings))`
`124`	`124`	`);`
`125`	`125`
`126`	`126`	`0`
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@ pub(crate) trait OutputFormatter {`
`17`	`17`	`desc: &TestDesc,`
`18`	`18`	`result: &TestResult,`
`19`	`19`	`stdout: &[u8],`
	`20`	`+ state: &ConsoleTestState,`
`20`	`21`	`) -> io::Result<()>;`
`21`	`22`	`fn write_run_finish(&mut self, state: &ConsoleTestState) -> io::Result<bool>;`
`22`	`23`	`}`
Original file line number	Diff line number	Diff line change
`@@ -162,7 +162,13 @@ impl<T: Write> OutputFormatter for PrettyFormatter<T> {`
`162`	`162`	`Ok(())`
`163`	`163`	`}`
`164`	`164`
`165`		`- fn write_result(&mut self, desc: &TestDesc, result: &TestResult, _: &[u8]) -> io::Result<()> {`
	`165`	`+ fn write_result(`
	`166`	`+ &mut self,`
	`167`	`+ desc: &TestDesc,`
	`168`	`+ result: &TestResult,`
	`169`	`+ _: &[u8],`
	`170`	`+ _: &ConsoleTestState,`
	`171`	`+ ) -> io::Result<()> {`
`166`	`172`	`if self.is_multithreaded {`
`167`	`173`	`self.write_test_name(desc)?;`
`168`	`174`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,12 @@`
`1`	`1`	`#[test]`
`2`	`2`	`fn a() {`
	`3`	`+ println!("print from successful test");`
`3`	`4`	`// Should pass`
`4`	`5`	`}`
`5`	`6`
`6`	`7`	`#[test]`
`7`	`8`	`fn b() {`
`8`		`- assert!(false)`
	`9`	`+ assert!(false);`
`9`	`10`	`}`
`10`	`11`
`11`	`12`	`#[test]`