Identity checking

[neithernut]

Some classes of properties checked with this crate may merit some convenience functionality. One of those classes being properties which essentially check whether some transformation is equivalent to identity.

One domain in which I found this approach useful are parsers: you'd format some arbitrary data-structure and read it back through the parser, but I suspect you end up constructing pseudo-identities in quite a few domains. In fact, the very example in the readme tests for such a property:

#[quickcheck]
fn double_reversal_is_identity(xs: Vec<isize>) -> bool {
    xs == reverse(&reverse(&xs))
}

Depending on the complexity of the functions involved in the construction, tracking the input reported by quickcheck through the code may be rather tedious, even after the inputs are shrunk. Knowing how exactly the result of the pseudo-identity differs from its input can help narrowing down the error. So users may end up printing both as part of the test:

#[quickcheck]
fn double_reversal_is_identity(xs: Vec<isize>) -> bool {
    let res = reverse(&reverse(&xs));
    if xs == res {
        true
    } else {
        eprintln!("Original: {:?}", xs);
        eprintln!("Identity: {:?}", xs);
        false
    }
}

However, this also results in them being printed during the shrinking, which is not ideal. I assume this wouldn't be the case if using TestResult::error instead of eprintln, and we can always construct the test ourselves. But still, we end up writing common boilerplate for that class of tests. It would be nice if we could write the following, instead:

#[quickcheck(identity)]
fn double_reversal_is_identity(xs: Vec<isize>) -> Vec<isize> {
    reverse(&reverse(&xs))
}

Quickcheck would generate a test which does the comparison between the function's input and output. If they differ, it would also print both input and output after shrinking.

---

A generalization of this class would be cases in which we want to compare two values. We'd write:

#[quickcheck(equivalence)]
fn double_reversal_is_identity(xs: Vec<isize>) -> (Vec<isize>, Vec<isize>) {
    (reverse(&reverse(&xs)), xs)
}

And the macro would generate a test which compares the two values in the tuple and prints them (along with the input) after shrinking. In this case, the output (i.e. the tuple element) would not need to be of the same type as the inputs.

---

This would obviously be a convenience feature. I volunteer implementing it, but I'd like to hear opinions on whether this is actually something anyone else would find useful first.

[BurntSushi]

I'm not so sure about it. It seems weird to me to improve the debugging experience for one specific kind of case (which I admit, is common) but not do it for the rest.

There are some other subjective criteria at play for me at least:

1. I personally don't want to get in the business of making the proc macro more complex and the added maintenance burden that entails. Mostly because as someone who doesn't work with that part of the ecosystem too much, diving into it to fix things when they go wrong takes a while. But, this is hand wavy and I don't feel strongly, especially if this addition is simple.
2. I am in general not particularly swayed by concerns about boiler plate unless it's particularly bad. I suppose I haven't felt your pain. But using TestResult::error doesn't seem too bad? Can you show an example of using it? Maybe a simple macro_rules macro on your end would cure that problem very easily, for example.

So overall, I'm skeptical of this change. I am not a "hard no" on it though. But that's the way I'm leaning.

[neithernut]

I personally don't want to get in the business of making the proc macro more complex and the added maintenance burden that entails. Mostly because as someone who doesn't work with that part of the ecosystem too much, diving into it to fix things when they go wrong takes a while. But, this is hand wavy and I don't feel strongly, especially if this addition is simple.

Reluctance to add complexity, especially to proc macros, is totally relatable. In practice, we would differentiate based on an argument (identity, equivalence or none), maybe right at the top level of the macro. But it would inflate that macro, especially if more and more cases are added over time.

I am in general not particularly swayed by concerns about boiler plate unless it's particularly bad. I suppose I haven't felt your pain. But using TestResult::error doesn't seem too bad?

It's definitely an option. It feels a bit more natural printing two lines of debug messages via two print-like statements but that's just a preference. What actually bothers me, if using eprintln!, is that I end up printing these values for each stage of shrinking rather than only the final one. Skimming over this implementation of Testable, using TestResult::error does already give me what I want in that regard (haven't observed it at runtime, yet EDIT: it does).

Can you show an example of using it?

I do have a few examples in the code I'm currently working on, but it's not open-sourced, yet.

Maybe a simple macro_rules macro on your end would cure that problem very easily, for example.

Yes, it would indeed be easy to implement as an independent macro on top of the existing one. But in the end, it's just a minor inconvenience. These test types I described or additional macros wouldn't exactly be a must have. I can live without them.

I will probably play around and maybe post such a macro example here at some point.

[neithernut]

After reading the documentation some more, I concluded that using the Testable trait as an extension point is probably the better solution. I already created a PR (#281) demonstrating such a solution.