Proof of Concept: custom scope for check!

[felixwrt]

I use assert2::check! a lot in my tests, but was missing some flexibility. Especially when using check! in a loop, there's currently no way to prevent panicking after the first iteration in which the check fails. I read #23 and had a slightly different idea that I successfully tried out:

This PR contains the proof of concept of that idea. I've introduced a CheckContext type that tracks whether a check failed and panics when it goes out of scope. This context can be used in the new macro check_ctx!. I've also written two tests that show how these two items can be used. This is the output generated by these tests:

running 3 tests
test __assert2_impl::print::diff::test_div_ceil ... ok
test test::test_check_in_loop ... FAILED
test test::test_check_nested ... FAILED

failures:

---- test::test_check_in_loop stdout ----
Assertion failed at src/lib.rs:385:4:
  assert!( i % 2 == 0 )
with expansion:
  1 == 0
with message:
  Check failed for i=1

Assertion failed at src/lib.rs:385:4:
  assert!( i % 2 == 0 )
with expansion:
  1 == 0
with message:
  Check failed for i=3

Assertion failed at src/lib.rs:385:4:
  assert!( i % 2 == 0 )
with expansion:
  1 == 0
with message:
  Check failed for i=5

thread 'test::test_check_in_loop' panicked at src/lib.rs:372:13:
check failed
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test::test_check_nested stdout ----
Assertion failed at src/lib.rs:401:3:
  assert!( i > 200 )
with expansion:
  123 > 200

Assertion failed at src/lib.rs:405:3:
  assert!( i % 2 == 0 )
with expansion:
  1 == 0

thread 'test::test_check_nested' panicked at src/lib.rs:372:13:
check failed


failures:
    test::test_check_in_loop
    test::test_check_nested

test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--lib`

I quite like the approach and I think that it's even possible to add support for passing a context to the existing check! macro.

What do you think? Could you see something like this in assert2?

[de-vri-es]

Hey!

Thanks for the PR. I had been thinking about a mechanism like this as a solution too. The main reason I didn't implement it is because I was still holding out for something like test::mark_failed(). There is some progress towards test::mark_failed(), but it's also nowhere near ready.

In a way, this PR would make it more difficult to switch because we're giving more control over when the panic happens. So changing the macro to not panic at all would be more of a breaking change than it is now (the macro documentation explicitly states that the function might stop panicking at all in the future).

So I'm a bit torn: on the one hand I want to have a solution, and I think the one in this PR is currently the only viable one. On the other hand, I don't really want check!() to panic at all in the future.

[felixwrt]

Sry that it's been taking me so long to respond...

Couldn't check_ctx have the same note ascheck saying that it might not panic anymore in the future? If failing a test without panicking becomes possible in the future, check_ctx could be changed to not panic and could be deprecated. Existing code would continue to work and could gradually be fixed to resolve the usage of check_ctx.

Am I missing something?

[IzawGithub]

Hello, friendly bump to ask, I've been testing @felixwrt branch for a bit, and the drop solution seems to work well.
Would it be possible to merge this?

This solution is especially great to create multiple tests cases.
For example, I'm using check_ctx like so:

const INPUTS: [&str; ...] = ["aaa", "bbb", ...];
const EXPECTED: [&str; ...] = ["1", "2", ...];

let ctx = CheckContext::new();
INPUTS.iter()
    .interleave(EXPECTED.iter())
    .for_each(|(input, expected)| {
        let actual = fn_to_test(input);
        check_ctx!(ctx, expected == actual);
    })

This is a super nice API, instead of having to do macro shenanigans to generate tests case, or using something like rstest #[case] which does the same thing under the hood.

I understand not wanting to add something that could be deprecated in the future, but I don't really see a way that this could be done in stable until custom tests framework is merged, which seems pretty far away, and this is a really useful feature to have.

[de-vri-es]

Ok, I'm convinced that we should have something that can work short term.

I think I want to call it just a Scope and also use it for things like capturing additional information to be shown in case of test failures.

I'm not 100% sure about the exact API for this though. It will probably have to be macro based too (atleast for capturing variables) because captured variables also need to be un-registered when the scope closes where they were captured.

I'm thinking something along these lines:

let scope = assert2::Scope::new();
assert2::capture!(scope, $expression);

And this would expand to something like:

let scope = assert2::Scope::new();
let scope = scope.capture(
    stringify!($expression),
    $expression,
);

And probably this would internally be backed by an Arc<Mutex<ScopeInner>> so that it can be shared across threads.

The difficult part: what syntax will the assert!() and check!() macros use for taking a scope argument? I would love to use a thread-local variable and just have one implicit scope per thread, but this would break down when multiplexing futures on the same thread, and I don't think that's acceptable.

[de-vri-es]

Anyway, there's also the question of syntax. We could go with a new macro name, or some special syntax.

I don't really like weird syntax, but I also don't feel great about adding more macros either.

Some options:

assert_scoped!(scope, $expression, $msg...);
assert!(@my_scope, $expression, $msg...);
assert!(scope = my_scope, $expression, $msg...); // Maybe this is ambiguous? Also somewhat consfusing considering `$msg...` can have named placeholders too.
assert!(@scope = my_scope, $expression, $msg...);
assert!(#[scope = my_scope] $expression, $msg...); // Ewww?
assert!(
    #[scope = my_scope]
    $expression,
    $msg...
); // Still ewww?

I think looking at all of these a new macro seems the cleanest 😅

[uimataso]

Hi! I really love this crate. I use it on every project now. And I’d also love to see this feature get merged, so here are my two cents on this :D

I think the best option could be a new macro, because:

• not in assert!:
  I think the main property of assert! is panicking immediately, so the scope feature doesn’t really fit that property.
  Also, scope doesn’t combine well with the assert!(let _ = _) feature. Since the binding will require the macro panic immediately on unhappy condition.

• not in check! (if we want to migrate to a potential new test framework):
  Since scope uses a panic guard, I don’t think it would work well with test::mark_failed() when the new test framework comes out.
  And if the new test framework has its own scope feature, we could add scope support to check! at that time and just deprecate the old one, so we wouldn’t need to maintain two different systems in one macro.
  (im not a library author myself, tho, so i don’t really know much about this...)

• formatter:
  Although I also think assert!(@my_scope, $expression, $msg) looks clean, adding special syntax makes the formatter stop working (though it still works in some cases).

So IMO, a new macro check_scoped!(scope, $expression, $msg) might be the best option here.

But again, I’m not a library author myself and don’t know much about macros, so take my words with a grain of salt :)

[de-vri-es]

Regarding whether or not assert! needs a scope version: I think it would be useful. Not for delaying panics, because as you say it will always directly panic.

But I want to re-use the same scope object for adding captures/diagnostics. For example, I would like to support something like:

use assert2::{assert_scoped, capture};

let mut scope = assert2::scope();
for i in 0..10 {
  capture!(scope, i);
  assert_scoped!(scope, difficult_math(i) == 2 * i);
}

And this would show the value of i if the assertion fails.

Right now you have to do this:

use assert2::assert;

for i in 0..10 {
  assert!(difficult_math(i) == 2 * i, "i = {i}");
}

But that gets cumbersome for more complex assertions and it allows for typos.

I would also like to let them automatically get removed from the "scope" object when they go out of scope:

use assert2::{assert_scoped, capture};

let mut scope = assert2::scope();
for i in 0..10 {
  capture!(scope, i);
  assert!(scope, difficult_math(i) == 2 * i);
}
assert!(false); // Doesn't show `i` because it's out of scope already.

But that's kinda contrary to how the scope is supposed to work for delaying checks... But it's probably necessary if we're going to keep around a &dyn Debug for printing it.