verus-lang / verus

Verified Rust for low-level systems code
MIT License
1.06k stars 58 forks source link

rust_verify_test: support expected message in FAILS comment #1200

Open mmcloughlin opened 1 week ago

mmcloughlin commented 1 week ago

This PR extends the testing mechanism that checks for FAILS comments to additionally allow an expected error message.

A comment of the form // FAILS just checks for an error. The form // FAILS: <expect> also confirms that <expect> appears in the diagnostic text.

This PR demonstrates the feature by also updating some of the integer_ring tests.

mmcloughlin commented 1 week ago

I'm not sure if this is a good idea, open to suggestions.

A simple alternative is something like a helper assert_one_fails_with_vir_error_msg. This inline comment approach could be nice for the multi-error case though.

If we are going to support this kind of extended // FAILS syntax, we might also want to think about enforcing a format for FAILS comments. At the moment there are some comments with notes attached to them, for example:

https://github.com/verus-lang/verus/blob/edb8f536ddf31bf1d160367519e68f09aab80a4f/source/rust_verify_test/tests/loops.rs#L616

https://github.com/verus-lang/verus/blob/edb8f536ddf31bf1d160367519e68f09aab80a4f/source/rust_verify_test/tests/integer_ring.rs#L217

Perhaps we support both an expectation and a note?

// FAIL
// FAIL: <expect_msg>
// FAIL (<note>)
// FAIL: <expect_msg> (<note>)
utaal commented 6 days ago

Hi @mmcloughlin. Thank you for the PR.

I have a fairly strong preference for the alternative approach:

A simple alternative is something like a helper assert_one_fails_with_vir_error_msg.

The FAILS comment, in my mind, should just be a marker of the affected line, and I'd prefer we kept that as simple as possible. Multiple errors with FAILS are uncommon, because we generally will only reliably get one SMT failure (the earliest failing assertion), and so we don't generally write tests with multiple FAILS.

Support for info or warning would be useful, through the addition of other markers (there it would be more useful to have multiple -- and perhaps identify them by some "id" to match them with the expected message. In fact, we could also do this for "FAILS" in the uncommon case where there are multiple FAILS we want to handle, e.g.


use vstd::string::*;
fn get_char_fails() {
    let x = new_strlit("hello world");
    let val = x.get_char(0); // FAILS(0)
    assert(val === 'h'); // FAILS(1)
}

uses:

assert_fails_with_vir_msgs(err, [
    Level::Error, "invalid comparison operator `===`",
    Level::Error, "expected function, found macro `assert`",
])