Skip to content

Rewrite the no-input-file.stderr test in Rust and support diff #124257

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

Merged
merged 1 commit into from
Apr 25, 2024
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
7 changes: 7 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3343,6 +3343,7 @@ version = "0.0.0"
dependencies = [
"object 0.34.0",
"regex",
"similar",
Copy link
Member

@jieyouxu jieyouxu Apr 24, 2024

Choose a reason for hiding this comment

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

Could you check if you can just use the diff from the environment? As in, I think we already assume diff exists:

# diff with common flags for multi-platform diffs against text output
DIFF := diff -u --strip-trailing-cr

If that's not available, then I'm open to adding similar.

EDIT: Now that I think about it, the easier to run the test the better. For example, I think grep isn't by default available on Windows? Would be nice to not have to rely on those external dependencies if possible/reasonable especially if they can have platform-specific differences and what not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be nice to not have to rely on those external dependencies if possible/reasonable especially if they can have platform-specific differences and what not.

I think so too.

"wasmparser",
]

Expand Down Expand Up @@ -5138,6 +5139,12 @@ version = "1.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64"

[[package]]
name = "similar"
version = "2.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fa42c91313f1d05da9b26f267f931cf178d4aba455b4c4622dd7355eb80c6640"

[[package]]
name = "siphasher"
version = "0.3.11"
Expand Down
1 change: 1 addition & 0 deletions src/tools/run-make-support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ edition = "2021"

[dependencies]
object = "0.34.0"
similar = "2.5.0"
wasmparser = "0.118.2"
regex = "1.8" # 1.8 to avoid memchr 2.6.0, as 2.5.0 is pinned in the workspace
81 changes: 81 additions & 0 deletions src/tools/run-make-support/src/diff/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
use similar::TextDiff;
use std::path::Path;

#[cfg(test)]
mod tests;

pub fn diff() -> Diff {
Diff::new()
}

#[derive(Debug)]
pub struct Diff {
expected: Option<String>,
expected_name: Option<String>,
actual: Option<String>,
actual_name: Option<String>,
}

impl Diff {
/// Construct a bare `diff` invocation.
pub fn new() -> Self {
Self { expected: None, expected_name: None, actual: None, actual_name: None }
}

/// Specify the expected output for the diff from a file.
pub fn expected_file<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
let path = path.as_ref();
let content = std::fs::read_to_string(path).expect("failed to read file");
let name = path.to_string_lossy().to_string();

self.expected = Some(content);
self.expected_name = Some(name);
self
}

/// Specify the expected output for the diff from a given text string.
pub fn expected_text<T: AsRef<[u8]>>(&mut self, name: &str, text: T) -> &mut Self {
self.expected = Some(String::from_utf8_lossy(text.as_ref()).to_string());
self.expected_name = Some(name.to_string());
self
}

/// Specify the actual output for the diff from a file.
pub fn actual_file<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
let path = path.as_ref();
let content = std::fs::read_to_string(path).expect("failed to read file");
let name = path.to_string_lossy().to_string();

self.actual = Some(content);
self.actual_name = Some(name);
self
}

/// Specify the actual output for the diff from a given text string.
pub fn actual_text<T: AsRef<[u8]>>(&mut self, name: &str, text: T) -> &mut Self {
self.actual = Some(String::from_utf8_lossy(text.as_ref()).to_string());
self.actual_name = Some(name.to_string());
self
}

/// Executes the diff process, prints any differences to the standard error.
#[track_caller]
pub fn run(&self) {
let expected = self.expected.as_ref().expect("expected text not set");
let actual = self.actual.as_ref().expect("actual text not set");
let expected_name = self.expected_name.as_ref().unwrap();
let actual_name = self.actual_name.as_ref().unwrap();

let output = TextDiff::from_lines(expected, actual)
.unified_diff()
.header(expected_name, actual_name)
.to_string();

if !output.is_empty() {
panic!(
"test failed: `{}` is different from `{}`\n\n{}",
expected_name, actual_name, output
)
}
}
}
39 changes: 39 additions & 0 deletions src/tools/run-make-support/src/diff/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#[cfg(test)]
mod tests {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the tests here will not be executed in CI for now (I don't know how to configure it).

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 that is fine for now, but if you want to add support for running the unit tests in run-make-support, you'll need to do something like CompiletestTest:

impl Step for CompiletestTest {

Modulo the unstable test feature part because run-make-support is stable-only and does not use any unstable (cargo or rustc) features.

Copy link
Member

Choose a reason for hiding this comment

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

I filed issue #124267 to track this.

use crate::*;

#[test]
fn test_diff() {
let expected = "foo\nbar\nbaz\n";
let actual = "foo\nbar\nbaz\n";
diff().expected_text("EXPECTED_TEXT", expected).actual_text("ACTUAL_TEXT", actual).run();
}

#[test]
fn test_should_panic() {
let expected = "foo\nbar\nbaz\n";
let actual = "foo\nbaz\nbar\n";

let output = std::panic::catch_unwind(|| {
diff()
.expected_text("EXPECTED_TEXT", expected)
.actual_text("ACTUAL_TEXT", actual)
.run();
})
.unwrap_err();

let expected_output = "\
test failed: `EXPECTED_TEXT` is different from `ACTUAL_TEXT`

--- EXPECTED_TEXT
+++ ACTUAL_TEXT
@@ -1,3 +1,3 @@
foo
+baz
bar
-baz
";

assert_eq!(output.downcast_ref::<String>().unwrap(), expected_output);
}
}
2 changes: 2 additions & 0 deletions src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

pub mod cc;
pub mod clang;
pub mod diff;
pub mod llvm_readobj;
pub mod run;
pub mod rustc;
Expand All @@ -20,6 +21,7 @@ pub use wasmparser;

pub use cc::{cc, extra_c_flags, extra_cxx_flags, Cc};
pub use clang::{clang, Clang};
pub use diff::{diff, Diff};
pub use llvm_readobj::{llvm_readobj, LlvmReadobj};
pub use run::{run, run_fail};
pub use rustc::{aux_build, rustc, Rustc};
Expand Down
7 changes: 7 additions & 0 deletions src/tools/run-make-support/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ impl Rustc {
self
}

/// Specify the print request.
pub fn print(&mut self, request: &str) -> &mut Self {
self.cmd.arg("--print");
self.cmd.arg(request);
self
}

/// Add an extra argument to the linker invocation, via `-Clink-arg`.
pub fn link_arg(&mut self, link_arg: &str) -> &mut Self {
self.cmd.arg(format!("-Clink-arg={link_arg}"));
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/allowed_run_make_makefiles.txt
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ run-make/no-builtins-attribute/Makefile
run-make/no-builtins-lto/Makefile
run-make/no-cdylib-as-rdylib/Makefile
run-make/no-duplicate-libs/Makefile
run-make/no-input-file/Makefile
run-make/no-intermediate-extras/Makefile
run-make/obey-crate-type-flag/Makefile
run-make/optimization-remarks-dir-pgo/Makefile
Expand Down
4 changes: 0 additions & 4 deletions tests/run-make/no-input-file/Makefile

This file was deleted.

9 changes: 9 additions & 0 deletions tests/run-make/no-input-file/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extern crate run_make_support;

use run_make_support::{diff, rustc};

fn main() {
let output = rustc().print("crate-name").run_fail_assert_exit_code(1);

diff().expected_file("no-input-file.stderr").actual_text("output", output.stderr).run();
}
Loading