Skip to content

Commit

Permalink
Fix panics and pretty_assert not outputting stdout
Browse files Browse the repository at this point in the history
This commit fixes assay so that it actually uses the stdout from the spawned
subprocess. Up to this point panics would cause tests to fail, however, it did
not panic with the stdout of that subprocess. It would instead just print out
the panic message from the callsite in the macro. To rectify this we now install
a custom panic hook that will use the default if a special header is not passed
into a panic message. In the replaced hook though we print out the output of the
panic from the spawned subprocess now. With this we actually now show the proper
output to the caller, almost as if we never spawned a subprocess.

We now also have a way to test these by using one test to spawn broken tests
which use the assay macro, but are ignored otherwise. This way we can test bad
tests or failure cases and check that the output is what we expect!
  • Loading branch information
mgattozzi committed Dec 8, 2024
1 parent 300f53f commit bb6e940
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 33 deletions.
11 changes: 10 additions & 1 deletion assay-proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,16 @@ pub fn assay(attr: TokenStream, item: TokenStream) -> TokenStream {
.expect("executed a subprocess");
let stdout = String::from_utf8(out.stdout).unwrap();
if stdout.contains(&format!("{name} - should panic ... ok")) || stdout.contains(&format!("{name} ... FAILED")) {
panic!();
let stdout_line = format!("---- {name} stdout ----");
let split = stdout
.lines()
.skip_while(|line| line != &stdout_line)
.skip(1)
.take_while(|s| !s.starts_with("----") && !s.starts_with("failures:"))
.collect::<Vec<&str>>()
.join("\n");
assay::panic_replace();
panic!("ASSAY_PANIC_INTERNAL_MESSAGE\n{split}")
}
} else{
child().unwrap();
Expand Down
30 changes: 30 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,40 @@ use std::{
env,
error::Error,
fs::{copy, create_dir_all},
panic,
path::{Component, Path, PathBuf},
sync::OnceLock,
};
use tempfile::{Builder, TempDir};

#[doc(hidden)]
pub static PANIC_HOOK_REPLACE: OnceLock<()> = OnceLock::new();
#[doc(hidden)]
pub fn panic_replace() {
const HEADER: &str = "ASSAY_PANIC_INTERNAL_MESSAGE\n";
PANIC_HOOK_REPLACE.get_or_init(|| {
let default = panic::take_hook();
panic::set_hook(Box::new(move |panic_info| {
let msg = panic_info
.payload()
.downcast_ref::<&str>()
.map(|s| s.to_string())
.or_else(|| {
panic_info
.payload()
.downcast_ref::<String>()
.map(|s| s.to_owned())
})
.unwrap_or_default();
if msg.starts_with(HEADER) {
println!("{}", msg[HEADER.len()..].trim());

Check failure on line 53 in src/lib.rs

View workflow job for this annotation

GitHub Actions / Lint

stripping a prefix manually
} else {
default(panic_info);
}
}))
});
}

#[doc(hidden)]
pub struct PrivateFS {
ran_from: PathBuf,
Expand Down
27 changes: 0 additions & 27 deletions test.py

This file was deleted.

71 changes: 71 additions & 0 deletions tests/pretty_assert.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
use assay::assay;
use std::process::Command;

#[assay(ignore)]
fn assert_eq() {
assert_eq!(1, 5);
}

#[assay(ignore)]
fn assert_ne() {
assert_ne!(["foo", "bar"], ["foo", "bar"]);
}

#[assay(ignore)]
fn assert_eq_sorted() {
assert_eq_sorted!([1, 3, 2], [1, 2, 4]);
}

#[test]
fn pretty_assertions() {
let output = Command::new("cargo")
.args(&["test", "--workspace", "--", "--ignored", "assert"])
.output()
.unwrap();
let assert_tests = String::from_utf8(output.stdout).unwrap();

if assert_tests.contains(
"---- assert_eq_sorted stdout ----
thread 'assert_eq_sorted' panicked at tests/pretty_assert.rs:16:3:
assertion failed: `(left == right)`
Diff < left / right > :
[
1,
< 3,
2,
> 4,
]",
) && assert_tests.contains(
"---- assert_eq stdout ----
thread 'assert_eq' panicked at tests/pretty_assert.rs:6:3:
assertion failed: `(left == right)`
Diff < left / right > :
<1
>5",
) && assert_tests.contains(
"
---- assert_ne stdout ----
thread 'assert_ne' panicked at tests/pretty_assert.rs:11:3:
assertion failed: `(left != right)`
Both sides:
[
\"foo\",
\"bar\",
]",
) && assert_tests.contains(
"failures:
assert_eq
assert_eq_sorted
assert_ne
test result: FAILED. 0 passed; 3 failed; 0 ignored; 0 measured; 1 filtered out",
) {
panic!(
"Unexpected output for assertions.\n\nOutput:\n{}",
assert_tests
);
}
}
37 changes: 32 additions & 5 deletions tests/should_fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

//! Inverted tests that we *want* to panic or pass. Given assay causes a test
//! to spawn itself we need to make sure these tests can actually properly fail.
//! We run these as part of a script to actually call them. Mainly because these
//! test that the assay macro works and if a test fails or does not it works as
//! expected. These are all ignored by default and must be explicitly called for
//! if we want them to run. This is because we expect these to fail and so we need
//! to run commands to check for a failure output for CI purposes
//! We run these with a test that calls cargo test --ignored. This lets us run
//! them in CI with cargo test --workspace to make sure what would fail is
//! tested without failing cargo test. These tests are all ignored by default
//! and must be explicitly called for if we want them to run.
use assay::assay;
use std::process::Command;

#[assay(ignore)]
fn should_panic_and_cause_a_failure_case() {
Expand All @@ -23,3 +23,30 @@ fn should_panic_and_cause_a_failure_case() {

#[assay(ignore, should_panic)]
fn should_not_panic_and_cause_a_failure_case() {}

#[test]
fn panics_in_macros() {
let output = Command::new("cargo")
.args(&[
"test",
"--workspace",
"--",
"--ignored",
"panic_and_cause_a_failure_case",
])
.output()
.unwrap();
let tests = String::from_utf8(output.stdout).unwrap();

if !tests.contains(
"---- should_not_panic_and_cause_a_failure_case stdout ----
note: test did not panic as expected",
) && !tests.contains(
"---- should_panic_and_cause_a_failure_case stdout ----
thread 'should_panic_and_cause_a_failure_case' panicked at tests/should_fail.rs:21:3:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace",
) {
panic!("Unexpected output for panics.\n\nOutput:\n{}", tests);
}
}

0 comments on commit bb6e940

Please sign in to comment.