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

capture argument values for further assertions #366

Open
albx79 opened this issue Mar 1, 2022 · 9 comments
Open

capture argument values for further assertions #366

albx79 opened this issue Mar 1, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@albx79
Copy link

albx79 commented Mar 1, 2022

When mocking a method that takes complex structures as parameters, it is common to prime the mock to capture that argument, and subsequently assert on it. See e.g. https://site.mockito.org/javadoc/current/org/mockito/ArgumentCaptor.html.

I was able to achieve the same with little code:

    pub struct Captor<T: Clone + 'static>(Arc<Mutex<Option<T>>>);

    pub fn captor<T: Clone + 'static>() -> Captor<T> {
        Captor(Arc::new(Mutex::new(None)))
    }

    impl<T: Clone + 'static> Captor<T> {
        pub fn capture(&self) -> impl Fn(&T) -> bool {
            let store = self.0.clone();
            move |value| {
                *store.lock().unwrap() = Some(value.clone());
                true
            }
        }

        pub fn value(self) -> Option<T> {
            self.0.lock().unwrap().take()
        }
    }

(maybe it would be more flexible to use Vec<T> instead of Option<T>).

However, I believe that this is basic functionality, expected to be provided by a mocking library. Captor should just be another Predicate.

Thanks.

@asomers
Copy link
Owner

asomers commented Mar 1, 2022

Can you give an example of how you would use this feature?

@asomers asomers added the enhancement New feature or request label Mar 1, 2022
@albx79
Copy link
Author

albx79 commented Mar 1, 2022

let my_captor = captor::<ComplexStruct>();

// prime the mock
my_mock.expect_method().with(function(my_captor.capture())).returning(...);

// ... do your test

// assertions
my_mock.checkpoint();
let actual_value = my_captor.value().unwrap();
assert_eq!(actual_value.some_field, "some value");
// etc., test as many or as few fields as you need

Actually, if capture() could return a predicate, you could simplify line 4 to

my_mock.expect_method().with(my_captor.capture()).returning(...);

@asomers
Copy link
Owner

asomers commented Mar 25, 2022

Ok, I see how this would be useful. And it's one of the rare feature requests that doesn't require changing the proc macros. Would you like to try submitting a PR to implement it?

@albx79
Copy link
Author

albx79 commented Mar 27, 2022

sure, I'll give it a shot.

@jmevel
Copy link

jmevel commented Aug 11, 2023

Hi guys, I wanted to create a similar feature request and then I realized someone already did.

Really too bad the PR was closed because the source branch was deleted...
Also, I don't quite understand how to use the Captor example above (sorry I'm new to Rust).

I was thinking if that could be possible to implement withf_once (FnOnce) and/or withf_mut (FnMut) functions?
We could then do something like this (this is my actual use case)

// Arrange
let mut sent_links: Vec<String> = Vec::new();
let mut email_client_mock = MockEmailClient::new();

email_client_mock
    .expect_send_email()
    .withf_mut(
        move |recipient: &str, subject: &str, text_content: &str| {
            assert_eq!(recipient, "foo@domain.com");
            assert_eq!(subject, "Welcome !");
            let confirmation_link = extract_single_link(text_content);
            sent_links.push(confirmation_link.to_owned());
            true
        },
    )
    .once()
    .returning(|_, _, _| Ok(()));

// Whatever
// Act
[...]

// Assert
let first_link = Url::parse(sent_links.first().unwrap()).unwrap();
let response = reqwest::get(first_link).await.unwrap();
assert_eq!(response.status().as_u16(), 200);

Sorry if my code might seem a bit stupid (or wrong), once again, I'm a beginner in Rust. I just hope my use case is clear enough.

In that example I can't call reqwest::get from within the closure because it would require the closure to be async and this is for the moment an unstable feature that requires to be on nightly.
Also this is not something I'd like to because it would make my withf function very big and doing things it is not supposed to do in my opinion (like sending an HTTP request)

Would that be possible to implement such functions?

I agree with @albx79, being able to capture arguments for further processing is a MUST HAVE feature for a mocking library. I've been playing with mockall for a few days, it's an awesome crate but now I'm kind of stuck in what I want to achieve.

Thanks for your answer

@asomers
Copy link
Owner

asomers commented Aug 12, 2023

I agree that reqwest::get is probably not something that you should call from within your returning closure. And I also understand how the Captor would be useful for your use case. Would you like to try to resurrect the abandoned PR?

But without adding new features, I also have a few suggestions for how to improve your current tests:

  • Don't assert within the withf function. Instead, just check those values and return false if they don't match. That way, if the called arguments don't match your expections, you'll get a "unexpected method call" error from Mockall, not an assertion error with a weird stack trace.
  • If you want to use a mutable callback, you can already do that in returning. It takes an FnMut.
  • If for some reason you really really want to capture the arguments during withf instead of returning, you can wrap your sent_links in a Mutex. That way you won't need an FnMut to change it.
  • Consider whether you really do need to fetch the sent_links, or if it would be more appropriate to simply validate the link text. This isn't Mockall-specific; it's just a common pattern for unit testing in general.

@jmevel
Copy link

jmevel commented Aug 16, 2023

Hi Alan,

Thanks a lot for your kind and detailed reply, this is very much appreciated 🙏
I took note of all your suggestions and will check them when I can find some free time for that.

  • I didn't even think of using returning actually, I overlooked the documentation and wrongly assumed this was only to capture the return value of a function while I needed the parameters. Maybe this feature can be put in the spotlight in the documentation with an example so beginners like me can quickly find what they're looking for?

  • About sending an HTTP request to the confirmation link, this is something I'm already used to do in .NET in my integration tests (using the Gherkin syntax)
    In integration tests we try to be as close as possible to the real scenarios. A user who receives a confirmation link by email will click on it and his/her account will be set as confirmed in our database. The end goal of my integration test, after sending an HTTP request to simulate a click on the link, will be to check in the database that this account is indeed in a confirmed status.
    Simply checking the link's URL without going further is in my opinion not enough.
    I'm following the Zero to production in Rust book and this is what the author is actually doing. I'm trying to follow everything from this book even though I diverged a bit because I implemented an SMTP email client instead of using an online REST API service like he does. That's why I ended up using mockall in my tests unlike the author in his book.

  • For the moment I try to stay focused on this book and stick on it when I can find some free time (not easy with a baby) but the idea of reviving the Captor PR is interesting. However since you already gave two other ways of achieving this with returning and also Mutex, is there really a value-added to implement this feature in mockall?
    Maybe all we need at the end is just a bit of improvement in the documentation if what you suggested is that easy. What do you think?

@jmevel
Copy link

jmevel commented Aug 16, 2023

@asomers
I just tried quickly using returning but I can't manage to capture arguments in a mutable vector like in my example above because the FnMut is Send + 'static so I get an error saying the closure may outlive my current function where the sent_links variable is defined.
If I use the move keyword to move sent_links inside the closure then I can't use it later on (unless I'm missing something?).

What's the reason returning is expecting a Send + 'static? Can't we provide a different lifetime instead of static ?

@asomers
Copy link
Owner

asomers commented Aug 16, 2023

You probably need an Arc<Mutex<Vec<String>>>, then. That will let you move it into the closure and also save a copy for later.

What's the reason returning is expecting a Send + 'static? Can't we provide a different lifetime instead of static ?

No, we really can't, because internally, the closure needs to be stored in a structure that is 'static.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants