From 9275ae554c9f52c06406a56a36006699e5ccd53b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 11 Jun 2024 15:42:07 +1000 Subject: [PATCH 1/5] compiletest: Allow escaped `"` in `//@ normalize-*` headers This makes it easier to normalize output containing double-quotes, such as JSON strings. --- src/tools/compiletest/src/header.rs | 13 +++++++------ src/tools/compiletest/src/header/tests.rs | 5 +++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 5469b9f1a0a2c..22428ad5144fd 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -1317,20 +1317,21 @@ fn expand_variables(mut value: String, config: &Config) -> String { fn parse_normalize_rule(header: &str) -> Option<(String, String)> { // FIXME(#126370): A colon after the header name should be mandatory, but // currently is not, and there are many tests that lack the colon. - // FIXME: Support escaped double-quotes in strings. let captures = static_regex!( r#"(?x) # (verbose mode regex) ^ - [^:\s]+:?\s* # (header name followed by optional colon) - "(?[^"]*)" # "REGEX" - \s+->\s+ # -> - "(?[^"]*)" # "REPLACEMENT" + [^:\s]+:?\s* # (header name followed by optional colon) + "(?(?:\\"|[^"])*)" # "REGEX" + \s+->\s+ # -> + "(?(?:\\"|[^"])*)" # "REPLACEMENT" $ "# ) .captures(header)?; + // The regex engine will unescape `\"` to `"`. let regex = captures["regex"].to_owned(); - let replacement = captures["replacement"].to_owned(); + // Unescape any escaped double-quotes in the replacement string. + let replacement = captures["replacement"].replace(r#"\""#, r#"""#); Some((regex, replacement)) } diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 61a85b84ad64c..4054e5410f50c 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -46,6 +46,11 @@ fn test_parse_normalize_rule() { "something (32 bits)", "something ($WORD bits)", ), + ( + r#"normalize-stout-test: "\"json\"key\"" -> "\"json\"value\"""#, + r#"\"json\"key\""#, + r#""json"value""#, + ), ]; for &(input, expected_regex, expected_replacement) in good_data { From ca4befbbda4a0edd06ac2f5bc918d5e143a24829 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 11 Jun 2024 13:32:47 +1000 Subject: [PATCH 2/5] compiletest: Don't panic when trying to print unknown JSON after failure If we're printing process output after some other check has failed, and the output is JSON-like, we shouldn't necessarily assume that it is valid JSON produced by the compiler. If it isn't, printing it as-is is more helpful than printing a less-relevant error message. --- src/tools/compiletest/src/json.rs | 22 ++++++++++++++++------ src/tools/compiletest/src/runtest.rs | 6 +++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/tools/compiletest/src/json.rs b/src/tools/compiletest/src/json.rs index 29e8809e5bd66..d778bf301e15c 100644 --- a/src/tools/compiletest/src/json.rs +++ b/src/tools/compiletest/src/json.rs @@ -91,7 +91,12 @@ pub fn rustfix_diagnostics_only(output: &str) -> String { .collect() } -pub fn extract_rendered(output: &str) -> String { +pub enum OnUnknownJson { + Error, + Print, +} + +pub fn extract_rendered(output: &str, on_unknown_json: OnUnknownJson) -> String { output .lines() .filter_map(|line| { @@ -125,11 +130,16 @@ pub fn extract_rendered(output: &str) -> String { // Ignore the notification. None } else { - print!( - "failed to decode compiler output as json: line: {}\noutput: {}", - line, output - ); - panic!() + match on_unknown_json { + OnUnknownJson::Error => { + print!( + "failed to decode compiler output as json: line: {}\noutput: {}", + line, output + ); + panic!() + } + OnUnknownJson::Print => Some(format!("{}\n", line)), + } } } else { // preserve non-JSON lines, such as ICEs diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 72b57d91c234e..c695da8152a27 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -31,7 +31,7 @@ use std::io::{self, BufReader}; use std::iter; use std::path::{Path, PathBuf}; use std::process::{Child, Command, ExitStatus, Output, Stdio}; -use std::str; +use std::str::{self, FromStr}; use std::sync::Arc; use anyhow::Context; @@ -3783,7 +3783,7 @@ impl<'test> TestCx<'test> { } else if explicit_format { proc_res.stderr.clone() } else { - json::extract_rendered(&proc_res.stderr) + json::extract_rendered(&proc_res.stderr, json::OnUnknownJson::Error) }; let normalized_stderr = self.normalize_output(&stderr, &self.props.normalize_stderr); @@ -4564,7 +4564,7 @@ pub struct ProcRes { impl ProcRes { pub fn print_info(&self) { fn render(name: &str, contents: &str) -> String { - let contents = json::extract_rendered(contents); + let contents = json::extract_rendered(contents, json::OnUnknownJson::Print); let contents = contents.trim_end(); if contents.is_empty() { format!("{name}: none") From 137908ccbb1c395f21ced1305a3477c90995efd9 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 11 Jun 2024 13:36:44 +1000 Subject: [PATCH 3/5] compiletest: Add support for `//@ check-run-stdout-is-json-lines` --- src/tools/compiletest/src/header.rs | 13 +++++++ src/tools/compiletest/src/runtest.rs | 11 ++++++ .../ui/meta/check-run-stdout-is-json-lines.rs | 34 +++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 tests/ui/meta/check-run-stdout-is-json-lines.rs diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 22428ad5144fd..dc0c143f9f64f 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -129,6 +129,11 @@ pub struct TestProps { pub check_stdout: bool, // Check stdout & stderr for output of run-pass test pub check_run_results: bool, + /// Check that stdout from running the binary is legal JSON Lines + /// (i.e. each line is well-formed JSON). + /// + /// Has no effect in tests that don't run the compiled binary. + pub check_run_stdout_is_json_lines: bool, // For UI tests, allows compiler to generate arbitrary output to stdout pub dont_check_compiler_stdout: bool, // For UI tests, allows compiler to generate arbitrary output to stderr @@ -226,6 +231,7 @@ mod directives { pub const FORCE_HOST: &'static str = "force-host"; pub const CHECK_STDOUT: &'static str = "check-stdout"; pub const CHECK_RUN_RESULTS: &'static str = "check-run-results"; + pub const CHECK_RUN_STDOUT_IS_JSON_LINES: &'static str = "check-run-stdout-is-json-lines"; pub const DONT_CHECK_COMPILER_STDOUT: &'static str = "dont-check-compiler-stdout"; pub const DONT_CHECK_COMPILER_STDERR: &'static str = "dont-check-compiler-stderr"; pub const NO_PREFER_DYNAMIC: &'static str = "no-prefer-dynamic"; @@ -285,6 +291,7 @@ impl TestProps { force_host: false, check_stdout: false, check_run_results: false, + check_run_stdout_is_json_lines: false, dont_check_compiler_stdout: false, dont_check_compiler_stderr: false, compare_output_lines_by_subset: false, @@ -423,6 +430,11 @@ impl TestProps { DONT_CHECK_COMPILER_STDOUT, &mut self.dont_check_compiler_stdout, ); + config.set_name_directive( + ln, + CHECK_RUN_STDOUT_IS_JSON_LINES, + &mut self.check_run_stdout_is_json_lines, + ); config.set_name_directive( ln, DONT_CHECK_COMPILER_STDERR, @@ -739,6 +751,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "check-fail", "check-pass", "check-run-results", + "check-run-stdout-is-json-lines", "check-stdout", "check-test-line-numbers-match", "compare-output-lines-by-subset", diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index c695da8152a27..9a0b44954c08c 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3940,6 +3940,17 @@ impl<'test> TestCx<'test> { self.fatal_proc_rec("test run succeeded!", &proc_res); } + if self.props.check_run_stdout_is_json_lines { + for (line, n) in proc_res.stdout.lines().zip(1..) { + if serde_json::Value::from_str(line).is_err() { + self.fatal_proc_rec( + &format!("invalid JSON on line {n} of stdout: {line:?}"), + &proc_res, + ); + } + } + } + if !self.props.error_patterns.is_empty() || !self.props.regex_error_patterns.is_empty() { // "// error-pattern" comments diff --git a/tests/ui/meta/check-run-stdout-is-json-lines.rs b/tests/ui/meta/check-run-stdout-is-json-lines.rs new file mode 100644 index 0000000000000..fb2eb0d314eeb --- /dev/null +++ b/tests/ui/meta/check-run-stdout-is-json-lines.rs @@ -0,0 +1,34 @@ +//@ run-pass +//@ ignore-pass (JSON checks don't run under --check=pass) +//@ check-run-stdout-is-json-lines + +//@ revisions: good bad_list bad_obj bad_empty bad_ws +//@ [bad_list] should-fail +//@ [bad_obj] should-fail +//@ [bad_empty] should-fail +//@ [bad_ws] should-fail + +// Check that `//@ check-run-stdout-is-json-lines` allows valid JSON lines and +// rejects invalid JSON lines, even without `//@ check-run-results`. + +fn main() { + println!("true"); + println!(r#"[ "this is valid json" ]"#); + println!(r#"{{ "key": "this is valid json" }}"#); + + if cfg!(bad_list) { + println!(r#"[ "this is invalid json", ]"#); + } + if cfg!(bad_obj) { + println!(r#"{{ "key": "this is invalid json", }}"#); + } + + // Every line must be valid JSON, and a blank or whitespace-only string is + // not valid JSON. + if cfg!(bad_empty) { + println!(); + } + if cfg!(bad_ws) { + println!(" \t \t "); + } +} From ec1d131987d8db5330650ced5e0c423bd78f8f0f Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 11 Jun 2024 13:51:22 +1000 Subject: [PATCH 4/5] Port `tests/run-make/libtest-json` to `tests/ui` --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/libtest-json/Makefile | 20 ---------- tests/run-make/libtest-json/f.rs | 22 ----------- tests/run-make/libtest-json/validate_json.py | 8 ---- .../test-attrs/format-json.normal.run.stdout} | 7 +++- tests/ui/test-attrs/format-json.rs | 39 +++++++++++++++++++ .../format-json.show-output.run.stdout} | 14 +++++-- 7 files changed, 54 insertions(+), 57 deletions(-) delete mode 100644 tests/run-make/libtest-json/Makefile delete mode 100644 tests/run-make/libtest-json/f.rs delete mode 100755 tests/run-make/libtest-json/validate_json.py rename tests/{run-make/libtest-json/output-default.json => ui/test-attrs/format-json.normal.run.stdout} (70%) create mode 100644 tests/ui/test-attrs/format-json.rs rename tests/{run-make/libtest-json/output-stdout-success.json => ui/test-attrs/format-json.show-output.run.stdout} (64%) diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index f32413540498d..39089daa18983 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -87,7 +87,6 @@ run-make/issue-88756-default-output/Makefile run-make/issue-97463-abi-param-passing/Makefile run-make/jobserver-error/Makefile run-make/libs-through-symlinks/Makefile -run-make/libtest-json/Makefile run-make/libtest-junit/Makefile run-make/libtest-padding/Makefile run-make/libtest-thread-limit/Makefile diff --git a/tests/run-make/libtest-json/Makefile b/tests/run-make/libtest-json/Makefile deleted file mode 100644 index c8bc7b5dd4a4c..0000000000000 --- a/tests/run-make/libtest-json/Makefile +++ /dev/null @@ -1,20 +0,0 @@ -# ignore-cross-compile -# needs-unwind -include ../tools.mk - -# Test expected libtest's JSON output - -OUTPUT_FILE_DEFAULT := $(TMPDIR)/libtest-json-output-default.json -OUTPUT_FILE_STDOUT_SUCCESS := $(TMPDIR)/libtest-json-output-stdout-success.json - -all: f.rs validate_json.py output-default.json output-stdout-success.json - $(RUSTC) --test f.rs - 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_DEFAULT) | "$(PYTHON)" validate_json.py - cat $(OUTPUT_FILE_STDOUT_SUCCESS) | "$(PYTHON)" validate_json.py - - # Normalize the actual output and compare to expected output file - cat $(OUTPUT_FILE_DEFAULT) | sed 's/"exec_time": [0-9.]*/"exec_time": $$TIME/' | diff output-default.json - - cat $(OUTPUT_FILE_STDOUT_SUCCESS) | sed 's/"exec_time": [0-9.]*/"exec_time": $$TIME/' | diff output-stdout-success.json - diff --git a/tests/run-make/libtest-json/f.rs b/tests/run-make/libtest-json/f.rs deleted file mode 100644 index edfe25086ae9a..0000000000000 --- a/tests/run-make/libtest-json/f.rs +++ /dev/null @@ -1,22 +0,0 @@ -#[test] -fn a() { - println!("print from successful test"); - // Should pass -} - -#[test] -fn b() { - assert!(false); -} - -#[test] -#[should_panic] -fn c() { - assert!(false); -} - -#[test] -#[ignore = "msg"] -fn d() { - assert!(false); -} diff --git a/tests/run-make/libtest-json/validate_json.py b/tests/run-make/libtest-json/validate_json.py deleted file mode 100755 index 657f732f2bffd..0000000000000 --- a/tests/run-make/libtest-json/validate_json.py +++ /dev/null @@ -1,8 +0,0 @@ -#!/usr/bin/env python - -import sys -import json - -# Try to decode line in order to ensure it is a valid JSON document -for line in sys.stdin: - json.loads(line) diff --git a/tests/run-make/libtest-json/output-default.json b/tests/ui/test-attrs/format-json.normal.run.stdout similarity index 70% rename from tests/run-make/libtest-json/output-default.json rename to tests/ui/test-attrs/format-json.normal.run.stdout index 01710f59e5d74..45aef751a734c 100644 --- a/tests/run-make/libtest-json/output-default.json +++ b/tests/ui/test-attrs/format-json.normal.run.stdout @@ -2,9 +2,12 @@ { "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 'b' panicked at f.rs:9:5:\nassertion failed: false\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" } +{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'b' panicked at $DIR/format-json.rs:LL:5: +assertion failed: false +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +" } { "type": "test", "event": "started", "name": "c" } { "type": "test", "name": "c", "event": "ok" } { "type": "test", "event": "started", "name": "d" } { "type": "test", "name": "d", "event": "ignored", "message": "msg" } -{ "type": "suite", "event": "failed", "passed": 2, "failed": 1, "ignored": 1, "measured": 0, "filtered_out": 0, "exec_time": $TIME } +{ "type": "suite", "event": "failed", "passed": 2, "failed": 1, "ignored": 1, "measured": 0, "filtered_out": 0, "exec_time": "$EXEC_TIME" } diff --git a/tests/ui/test-attrs/format-json.rs b/tests/ui/test-attrs/format-json.rs new file mode 100644 index 0000000000000..f7c4567c4a239 --- /dev/null +++ b/tests/ui/test-attrs/format-json.rs @@ -0,0 +1,39 @@ +//@ edition: 2021 +//@ run-fail +//@ check-run-results +//@ check-run-stdout-is-json-lines +//@ needs-unwind (for #[should_panic]) +// ignore-tidy-linelength + +//@ revisions: normal show-output +//@ compile-flags: --test +//@ run-flags: --test-threads=1 -Zunstable-options --format=json +//@ [show-output] run-flags: --show-output +//@ normalize-stdout-test: "(?format-json.rs:)[0-9]+(?:[0-9]+)" -> "${prefix}LL${suffix}" +//@ normalize-stdout-test: "(?\"exec_time\": *)[0-9.]+" -> "${prefix}\"$$EXEC_TIME\"" + +// Check that passing `--format=json` to the test harness produces output that +// matches the snapshot, and is valid JSON-lines. + +#[test] +fn a() { + println!("print from successful test"); + // Should pass +} + +#[test] +fn b() { + assert!(false); +} + +#[test] +#[should_panic] +fn c() { + assert!(false); +} + +#[test] +#[ignore = "msg"] +fn d() { + assert!(false); +} diff --git a/tests/run-make/libtest-json/output-stdout-success.json b/tests/ui/test-attrs/format-json.show-output.run.stdout similarity index 64% rename from tests/run-make/libtest-json/output-stdout-success.json rename to tests/ui/test-attrs/format-json.show-output.run.stdout index 878eb6c7c260d..c7f62418b0666 100644 --- a/tests/run-make/libtest-json/output-stdout-success.json +++ b/tests/ui/test-attrs/format-json.show-output.run.stdout @@ -1,10 +1,16 @@ { "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", "name": "a", "event": "ok", "stdout": "print from successful test +" } { "type": "test", "event": "started", "name": "b" } -{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'b' panicked at f.rs:9:5:\nassertion failed: false\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" } +{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'b' panicked at $DIR/format-json.rs:LL:5: +assertion failed: false +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +" } { "type": "test", "event": "started", "name": "c" } -{ "type": "test", "name": "c", "event": "ok", "stdout": "thread 'c' panicked at f.rs:15:5:\nassertion failed: false\n" } +{ "type": "test", "name": "c", "event": "ok", "stdout": "thread 'c' panicked at $DIR/format-json.rs:LL:5: +assertion failed: false +" } { "type": "test", "event": "started", "name": "d" } { "type": "test", "name": "d", "event": "ignored", "message": "msg" } -{ "type": "suite", "event": "failed", "passed": 2, "failed": 1, "ignored": 1, "measured": 0, "filtered_out": 0, "exec_time": $TIME } +{ "type": "suite", "event": "failed", "passed": 2, "failed": 1, "ignored": 1, "measured": 0, "filtered_out": 0, "exec_time": "$EXEC_TIME" } From bd1080e3d41a149bba54b7a532f88944f93091a5 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 21 Jun 2024 12:33:24 +1000 Subject: [PATCH 5/5] Also check that `tests-listing-format-json.rs` produces JSON Lines --- tests/ui/test-attrs/tests-listing-format-json.rs | 3 +-- tests/ui/test-attrs/tests-listing-format-json.run.stdout | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/ui/test-attrs/tests-listing-format-json.rs b/tests/ui/test-attrs/tests-listing-format-json.rs index b735a82c16625..1efddfab66780 100644 --- a/tests/ui/test-attrs/tests-listing-format-json.rs +++ b/tests/ui/test-attrs/tests-listing-format-json.rs @@ -3,9 +3,8 @@ //@ run-flags: --list --format json -Zunstable-options //@ run-pass //@ check-run-results +//@ check-run-stdout-is-json-lines //@ only-nightly -//@ normalize-stdout-test: "fake-test-src-base/test-attrs/" -> "$$DIR/" -//@ normalize-stdout-test: "fake-test-src-base\\test-attrs\\" -> "$$DIR/" // Checks the listing of tests with --format json. diff --git a/tests/ui/test-attrs/tests-listing-format-json.run.stdout b/tests/ui/test-attrs/tests-listing-format-json.run.stdout index 33cc939b59f5d..b4131e97c34bc 100644 --- a/tests/ui/test-attrs/tests-listing-format-json.run.stdout +++ b/tests/ui/test-attrs/tests-listing-format-json.run.stdout @@ -1,5 +1,5 @@ { "type": "suite", "event": "discovery" } -{ "type": "test", "event": "discovered", "name": "a_test", "ignore": false, "ignore_message": "", "source_path": "$DIR/tests-listing-format-json.rs", "start_line": 21, "start_col": 4, "end_line": 21, "end_col": 10 } -{ "type": "test", "event": "discovered", "name": "m_test", "ignore": false, "ignore_message": "", "source_path": "$DIR/tests-listing-format-json.rs", "start_line": 14, "start_col": 4, "end_line": 14, "end_col": 10 } -{ "type": "test", "event": "discovered", "name": "z_test", "ignore": true, "ignore_message": "not yet implemented", "source_path": "$DIR/tests-listing-format-json.rs", "start_line": 18, "start_col": 4, "end_line": 18, "end_col": 10 } +{ "type": "test", "event": "discovered", "name": "a_test", "ignore": false, "ignore_message": "", "source_path": "$DIR/tests-listing-format-json.rs", "start_line": 20, "start_col": 4, "end_line": 20, "end_col": 10 } +{ "type": "test", "event": "discovered", "name": "m_test", "ignore": false, "ignore_message": "", "source_path": "$DIR/tests-listing-format-json.rs", "start_line": 13, "start_col": 4, "end_line": 13, "end_col": 10 } +{ "type": "test", "event": "discovered", "name": "z_test", "ignore": true, "ignore_message": "not yet implemented", "source_path": "$DIR/tests-listing-format-json.rs", "start_line": 17, "start_col": 4, "end_line": 17, "end_col": 10 } { "type": "suite", "event": "completed", "tests": 3, "benchmarks": 0, "total": 3, "ignored": 1 }