Skip to content
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

Consider implementing Testable for Option<bool> #105

Closed
carllerche opened this issue Nov 11, 2015 · 6 comments
Closed

Consider implementing Testable for Option<bool> #105

carllerche opened this issue Nov 11, 2015 · 6 comments

Comments

@carllerche
Copy link

It seems like Option<bool> would be a simple way to discard tests vs. having to return a TestResult directly.

@BurntSushi
Copy link
Owner

I personally kind of like writing TestResult::discard because it's more explicit what's going on. I don't feel terribly strongly though.

@porglezomp
Copy link

I think TestResult is much clearer, and it's not very inconvenient in my opinion.

@andrewhickman
Copy link

This would be nice for use with the checked_ functions e.g.

#[quickcheck]
fn test_div_rem(a: i64, b: i64) -> Option<bool> {
    let div = a.checked_div(b)?;
    let rem = a.checked_rem(b)?;
    Some(div * b + rem == a)
}

@tobz1000
Copy link

tobz1000 commented Apr 30, 2019

Has this been reconsidered? I'd be happy to submit a PR.

The more ideal solution would probably be to implement the Try trait for TestResult, allowing for early return of TestResult::discard(), but it doesn't look like the trait will stabilise any time soon.

@BurntSushi
Copy link
Owner

I remain unconvinced that this is worth doing. I'd rather see tests being explicitly discarded.

@neithernut
Copy link
Contributor

Types like the one drafted in #281 (whether they are part of this library or implemented on the user's side) create use-cases where returning an Option would be preferable to the alternatives imo. Consider something like the following:

fn revrev(original: u128) -> Option<Equivalence<u128>> {
    if original == u128::MAX {
        // for some reason, u128::MAX must be ignored for the test
        None
    } else {
        // pseudo idendity
        Some(Equivalence::of(xs, revrev))
    }
}

Without an Testable implementation for Option, we'd need to write something like:

fn revrev(original: u128) -> TestResult {
    if original == u128::MAX {
        // for some reason, u128::MAX must be ignored for the test
        TestResult::discard()
    } else {
        // pseudo idendity
        Equivalence::of(xs, revrev).into_result(Gen::new(0))
    }
}

... with the very unsightly Gen::new(0) which shouldn't even used in the conversion, if it is a conversion. That is, unless there is some other way to convert Equivalence into a TestResult. Such a function could be mandated for such convenience types, but it would require manually writing such a conversion for all those types (or introduce a trait explicitly for such conversions and maybe impl Testable via that trait for the conversion case).

Of course, discarding a test/value explicitly would be far more expressive. And I myself am not sure whether using an Option is really intuitive or whether it would confuse some users. Maybe a type expressing this intend explicitly would be worth considering?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants