Skip to content
This repository has been archived by the owner on Dec 29, 2021. It is now read-only.

[WIP] Add support for arbitrary predicates #57

Closed
wants to merge 2 commits into from
Closed

[WIP] Add support for arbitrary predicates #57

wants to merge 2 commits into from

Conversation

dsprenkels
Copy link
Contributor

@dsprenkels dsprenkels commented Oct 18, 2017

Fixes #55.

I currently use, as OutputAssertion struct which has a closure containing the actual information of the assertion. In #55 @epage suggest using an enum to differentiate between different types of enum. Although it will probably be a little bit more verbose than what I have not, I think I agree to using this approach (instead of what I have now). I.e. This PR still needs some work.

Planned enum type:

enum OutputAssertion {
    MatchesExact(String, bool), // (expect, expected_result)
    MatchesFuzzy(String, bool), // (expect, expected_result),
    SatisfiesTest(test: Rc<Fn(&str) -> Result<()>>),
    SatisfiesPred(pred: Rc<Fn(&str) -> bool>),
}

Could also merge SatisfiesPred into SatisfiesTest, by making a new closure every time, but I think this kinda misses the point, so I probably won't do this. :P

@epage
Copy link
Collaborator

epage commented Oct 18, 2017

So we don't forget, there is also the question of whether we should use a Box<Fn> (function trait, stateful) or a fn (function pointer, stateless).

Box<Fn> is more general but do people need the extra flexibility?

Some options

  • keep the code as-is because there is a known use case
  • keep the code as-is since it is already written
  • switch to fn knowing we can later switch to Box<Fn> by using From to keep the API the same

@dsprenkels
Copy link
Contributor Author

dsprenkels commented Oct 20, 2017

I think I want to implement not only Box<Fn>, but Rc<Fn>, so that OutputAssertion will still be Clone. The use case that I am thinking of is people wanting to pass closures OutputAssertion:

#[macro_use] extern crate assert_cli;

fn checklen(expect_len: usize) {
    assert_cmd!(echo "-n" "forty-two").stdout().satisfies(|x| x.len() == expect_len).unwrap();
}

(Or could this also be achieved using fn?)

@epage
Copy link
Collaborator

epage commented Oct 20, 2017

Thanks for listing out a use case!

(Or could this also be achieved using fn?)

You'd have to hard code an fn for each length:

#[macro_use] extern crate assert_cli;

fn test_length() {
    assert_cmd!(echo "-n" "forty-two").stdout().satisfies(|x| x.len() == 9).unwrap();
}

(I'm assuming this closure will resolve to be just a fn)

I was mostly considering direct calls to assert_cli where hard coded functions are more reasonable. My reason being that reusing test boiler plate is probably a smell because it is probably incidental code sharing (shared implementation details that will diverge as requirements change) rather than abstraction sharing (requirements are the same).

I did not consider the idea of people building larger abstractions on top of assert_cli in which case they might need to take a specialized value from the user and create a closure out of it, requiring an Fn trait.

@epage
Copy link
Collaborator

epage commented Oct 28, 2017

FYI #74 will cause a lot of merge conflicts. I went ahead and reworked your ideas in a branch of #74 but haven't pushed it yet until #74 is in.

@dsprenkels
Copy link
Contributor Author

Thanks for the heads-up. I haven't had the mood to write the code myself. In my project I ended up using just duct. I am looking forward to seeing your solution.

@epage
Copy link
Collaborator

epage commented Feb 4, 2018

Just merged #87 which adds a limited form of predicates. Closing this for now.

@epage epage closed this Feb 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants