assert-rs / predicates-rs

Boolean-valued predicate functions in Rust
docs.rs/predicates
Apache License 2.0
173 stars 29 forks source link

Better documentation for find_case #71

Open asomers opened 5 years ago

asomers commented 5 years ago

It sounds like Predicate::find_case is important, but for the life of me I can't figure out how to use it. Could you please expand its documentation? What I'm looking for is a way to do something like this, so I can print a useful message whenever a predicate fails.

assert!(pred.eval(&input), pred.fail_message(&input))
epage commented 5 years ago

Yeah, this is an area that needs some work.

assert_fs and assert_cmd have some examples, for example: https://github.com/assert-rs/assert_cmd/blob/master/src/assert.rs#L255

Something else I've been meaning to do is write a hamcrest-like crate for so you can just do assert_that!(value, pred); and you get the information printed for you.

epage commented 5 years ago

btw the .tree() is coming from the extension trait brought in at https://github.com/assert-rs/assert_cmd/blob/master/src/assert.rs#L12.

ian-h-chamberlain commented 4 years ago

@epage I have created a simple assert_that macro which does basically what you describe for use in my own project:

pub use predicates;
pub use predicates_tree;

#[macro_export]
macro_rules! assert_that {
    ($item:expr, $pred:expr $(,)?) => {{
        use $crate::predicates::prelude::*;
        use $crate::predicates_tree::CaseTreeExt;

        if let Some(case) = $pred.find_case(false, $item) {
            panic!("{}", case.tree());
        };
    }};
}

Is this good enough for a first pass at inclusion to the library? I'd be happy to adapt it and open a PR if so.

I'm not sure if this would be part of predicates-tree, or maybe an optional feature of this crate? It seems general-purpose enough to be part of this crate but obviously requires CaseTreeExt to work.

epage commented 4 years ago

Is this good enough for a first pass at inclusion to the library? I'd be happy to adapt it and open a PR if so.

Sounds great, thanks!

I'm not sure if this would be part of predicates-tree, or maybe an optional feature of this crate? It seems general-purpose enough to be part of this crate but obviously requires CaseTreeExt to work.

Hmm, thats an interesting point.

It should either go in predicates or we should have a separate fluent-assertion crate.

Thoughts?

ian-h-chamberlain commented 4 years ago

Hmm, maybe a separate crate does make sense. Without it, users would need to either opt-in or opt-out of using this macro, since it creates a new dependency. Having a feature flag for a single convenience macro seems a little heavy-handed to me, but so does a new crate for that matter.

I think it is generally a somewhat common practice to have macro-only crates as conveniences for working with library crates, although in my experience those tend to be complex procedural macros which generate a lot of boilerplate code, rather than something simple like this.

For what it's worth, it seems the name assert_that is not yet taken on crates.io, so maybe that makes sense? It would have dependencies on both predicates and predicates-tree and re-export them for the sake of the macro, I suppose. I'm not sure it would be very ergonomic to use without also importing predicates::prelude::* since otherwise you have no predicate to assert on! Unless we used syntax like the following, assuming it is possible with macro namespacing rules:

extern crate assert_that;
use assert_that::assert_that;

#[test]
fn does_it_work() {
    // expands to use re-exported predicate, i.e. 
    // `assert_that::predicates::prelude::predicate::str::similar`
    assert_that!("Hello world", str::similar("Goodbye, world"));
}

I could see some additional extensions potentially being added to this, although I think most code changes would be made in the predicates-tree and predicates crates rather than in the implementation of this macro. For reference, I would look at GTest Matchers and their ASSERT_THAT macro for a possible UX for this one.

Perhaps future extensions could include helper macros for writing custom predicates ("matchers") like their MATCHER_P macro? Beyond that, I'm not sure what more could be done within that assertion crate in general, but I think there's a possibility for future enhancements.

Assuming that a separate crate makes sense – would you prefer to create a separate repo, or just a subcrate within this repo's Cargo workspace?

epage commented 4 years ago

Assuming that a separate crate makes sense – would you prefer to create a separate repo, or just a subcrate within this repo's Cargo workspace?

Like with the other crates (assert_fs, assert_cmd), I'm thinking separate crate but can also live in this org.

ian-h-chamberlain commented 4 years ago

@epage I have created and published a 0.1 version of assert_that: https://github.com/ian-h-chamberlain/assert_that

I also created https://github.com/ian-h-chamberlain/assert_that/issues/2 where we can discuss whether or not it makes sense to move it to this organization. I'm inclined to wait and see if the crate gets much/any usage or new use cases arise before transferring ownership, but if you'd rather transfer it sooner I think that would be fine.

epage commented 4 years ago

Thanks! Excited to see this develop!