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

Generics in mocked methods #18

Open
nrxus opened this issue Jan 14, 2020 · 19 comments
Open

Generics in mocked methods #18

nrxus opened this issue Jan 14, 2020 · 19 comments

Comments

@nrxus
Copy link
Owner

nrxus commented Jan 14, 2020

Neither of these work.

pub fn new_generic_input<T>(&self, a: T) { /* some code here */ } 
pub fn new_generic_output<T>(&self) -> T { /* some code here */ } 
pub fn impl_arg(&self, a: impl SomeTrait) { /* some code here */ } 
pub fn impl_out(&self) -> impl SomeTrait { /* some code here */ } 

The issue is on the definition of the _when_{} method.
The first two seem solvable together. Add all the generics from the mocked method to the when method.
The third one seems like it would require "de-sugaring" the impl Foo into <T: Foo> for the when method only.
The last one.... 🤷‍♂️ We could change it into a generic argument to the when method as a first-step MVP (read: hack).

@SirVer
Copy link

SirVer commented Feb 15, 2020

I am actually running into this with code similar to this:

    pub fn list_directory(&self, ... callback: impl fn(files::Metadata)>

Is there a known workaround how I can mock this code still?

@nrxus
Copy link
Owner Author

nrxus commented Feb 15, 2020

The only workaround I can think of is not mocking this method, but mocking things that this method calls.

#[faux::methods]
impl MyStruct {
    // this can be mocked
    pub fn metadata(&self) -> files::Metadata {
        /* get it somehow */
    }
}

impl MyStruct {
    // this cannot be mocked so it is in a separate impl block.
    pub fn list_directory(&self, callback: impl fn(files::Metadata) {
        // the call to metadata may have been mocked. 
        let metadata = self.metadata();
        callback.call(metadata);
    }
}

The main thing to note is that in the non-mocked methods, you may not access any fields in the struct. It is not very nice but at least it lets you get around the issue somewhat, and you can still mock the other methods on your struct.

Thanks for bringing up that you run into this! It helps me prioritize what to do next.

@SirVer
Copy link

SirVer commented Feb 16, 2020

Ah, unfortunately that did not help me, since I really needed the callback based API in my use case. I went with pulling my API out into a trait and manually mock the behavior of this trade now. Good enough for my uses for me, but I eagerly await mock becoming more powerful!

Thanks for your work, this all seems super promising.

@nrxus
Copy link
Owner Author

nrxus commented Feb 17, 2020

Would changing your signature to use generics rather than impl arguments be good enough? The former is significantly easier from a faux's perspective but I am aware that it is not as ergonomic to use.

Changing:

pub fn list_directory(&self, ... callback: impl Fn(files::Metadata))

to:

pub fn list_directory<F: Fn(files::Metadata)>(&self, ... callback: F)

@SirVer
Copy link

SirVer commented Feb 18, 2020

I would not mind doing that for easier mocking in future cases - the loss of ergonomics in this case is minimal and the gain of direct mocking substantial, so totally worth it.

@nrxus
Copy link
Owner Author

nrxus commented Mar 31, 2020

Apologies for the long turn around time, life has been a little crazy and this actually ended up being a bit of a mind melt. As it turns out it is actually easier for faux to mock methods that use impl rather than generic bounds. However, the caveat to this is that this only works for impl arguments that are object safe.

My current solution (a26cf09) wraps all impl Trait arguments in a Box<dyn Trait> when calling for when!(..).*then(). For callback methods, such as your own, Box<Fn*> implements Fn* so it should be fairly seamless, but it does make other use cases less ergonomic.

Unfortunately this is the only way I can think of at the moment to allow for this in a "safe" manner. I think I might just release this as-is to see if it solves your use case with the caveat that if I think of a better solution I might end up changing the API of it.

@nrxus
Copy link
Owner Author

nrxus commented Mar 31, 2020

Implementation Issues (writing for my own sake, no need to read)

Unsafety

For methods such as:

pub fn my_method<T>(&self, a: T) {}

My first though for a when method was:

fn _when_my_method<T>(&self) -> When<(T), ()> {
    /* make a When */
}

The When struct now has *then methods that look like:

fn then(self, mock: impl FnMut(T) -> O + 'static + Send) {
    /* store the mock */
}

At this point the user can now pass a mock that assumes any user-specified T:

mocked_struct._when_my_method<i32>().safe_then(|i: i32| {})

This seems all fine and dandy until... the user calls for the method:

mocked_struct.my_method("a string is still a T this method accepts");

And now we are big trouble. faux is going to transmute stuff as it does assuming all the checks have been done, and it's going to try to pass the user-given &str to a closure that accepts i32, and because it's all transmuted it will create UB.

Unnameable types

Given a structure similar to above, how would someone declare a mock for:

pub fn my_callback<F: Fn(i32)>(&self, a: F) {}

When method:

fn _when_my_method<F: Fn(i32)>(&self) -> When<(T), ()> {
    /* make a When */
}

When the user calls for when:

mocked_struct._when_my_method</* what goes here? */>()
    .safe_then(|i: /* what type is this? */| {})

I don't believe there is a way to name concrete types to closures as generic bounds so.... this would not work.

Alternatives

Exclude and ignore generic types

Somehow (?) detect what arguments are generics, and exclude them from the When method. This is fairly limiting but it could be "simple"

Box all generic types

Somehow (?) detect what arguments are generics, box them all up. This means make the When methods receive a boxed argument, and when the method is called box the argument prior to passing it to the mock. This means that only bounds that can be converted into a Box<dyn Trait> are allowed but it is better than nothing. There is an outstanding issue, however, how to deal with generics without a trait bound, should a dumb trait be added that anything implements? Perhaps use the Any trait and only support 'static types in generics? This is a pretty big open question =/

These two depend on finding what the trait arguments are which is not easy task for more complicated cases but it is perhaps doable, I would have to try it out. The impl Trait case is simpler because I do not have to "find" them, they are fairly explicit in the arguments.

@nrxus nrxus closed this as completed Mar 31, 2020
@nrxus nrxus reopened this Mar 31, 2020
@nrxus
Copy link
Owner Author

nrxus commented Mar 31, 2020

I clicked the wrong button, this is definitely not closed.

@SirVer
Copy link

SirVer commented Apr 1, 2020

Thanks for you work! I definitely make use of this!

@tv42
Copy link

tv42 commented Jun 10, 2021

I think I'm hitting this with

#[cfg_attr(test, faux::methods)]
impl S {
    pub async fn foo<M>(&self, message: &M) -> Result<(), Error>
    where
        M: Message + Serialize + Sync,
    {
        // ...
    }
error[E0412]: cannot find type `M` in this scope
   --> foo.rs:349:52
    |
349 |     pub async fn foo<M>(&self, message: &M) -> Result<(), Error>
    |                                                    ^ not found in this scope

@nrxus
Copy link
Owner Author

nrxus commented Jun 11, 2021

If possible, this should work by switching to using the impl _ syntax:

pub async fn foo<M>(&self, message: &impl Message + Serialize + Sync) -> Result<(), Error> {
        // ...
}

That being said, I should look into this since it is not always possible to use the impl _ syntax so thanks for bringing this up! (:

@tv42
Copy link

tv42 commented Jun 11, 2021

Interesting side discovery: faux forces all function impl traits to also implement Debug. There's a {:?} somewhere inside faux::when!.

Trying to please faux with impl leads to some furious build errors:

use std::fmt::Debug;
trait A: Debug {}

#[cfg_attr(test, faux::create)]
struct S {}

#[cfg_attr(test, faux::methods)]
impl S {
    pub fn foo(&self, _a: &impl A) {
        println!("foo")
    }
}

fn main() {
    #[derive(Debug)]
    struct T(u8);
    impl A for T {}
    let t = T(42);
    let s = S {};
    s.foo(&t);
}

#[cfg(test)]
#[test]
fn repro() {
    let mut mock = S::faux();
    faux::when!(mock.foo).then(|_| println!("mock foo"));
}
$ cargo run
[...]
foo
$ cargo test
[...]
error[E0720]: cannot resolve opaque type
 --> src/main.rs:9:28
  |
7 | #[cfg_attr(test, faux::methods)]
  |                  ------------- returning here with type `When<&S, &impl A, (), faux::when::Any>`
8 | impl S {
9 |     pub fn foo(&self, _a: &impl A) {
  |                            ^^^^^^ recursive opaque type

error[E0063]: missing field `0` in initializer of `S`
  --> src/main.rs:19:13
   |
19 |     let s = S {};
   |             ^ missing `0`

@nrxus
Copy link
Owner Author

nrxus commented Jun 12, 2021

Ah! 🤦🏾‍♂️

That's totally my bad. I just realized my impl Trait solution does not work when doing &impl Trait. In theory this should be an easy fix so I will look into it soon.

That being said, the trait does not need to implement debug at all. See the tests. Additionally, this part of the error:

error[E0063]: missing field `0` in initializer of `S`
  --> src/main.rs:19:13
   |
19 |     let s = S {};
   |             ^ missing `0`

has to do with the your main function trying to create an S without using a function that is wrapped by #[faux::methods] but rather trying to create it directly. Mockable structs can only be created by using functions/methods that are wrapped inside the #[faux::methods] attribute.

@tv42
Copy link

tv42 commented Jun 12, 2021

Ah yes, adding an S::new that's inside #[faux::methods] and using that gets rid of that error, just leaving the recursive opaque type one.

@nrxus
Copy link
Owner Author

nrxus commented Jun 16, 2021

I have a fix up but I was having issues with releasing yesterday, my plan is to get a release for this today or tomorrow. You can see the updated tests and let me know if you think it isn't covering your use case: https://github.com/nrxus/faux/blob/master/tests/generic_methods.rs

@nrxus
Copy link
Owner Author

nrxus commented Jun 17, 2021

@tv42 I just published a new release that should have this fix. Try it out and let me know! (:

@jmoguilevsky
Copy link

jmoguilevsky commented Dec 22, 2022

Hi!
Did you find any workaround or fix for output generics?

I have a function that's signature is:

pub async fn get_authenticated_request<T: DeserializeOwned>(
        &self,
        path: &str,
        token: &str,
    ) -> Result<T, CommonError>

And found no workaround to make it build

@jmoguilevsky
Copy link

@nrxus any ideas?

@nrxus
Copy link
Owner Author

nrxus commented Dec 26, 2022

Hey @jmoguilevsky, things get hectic during the holidays so I wasn't looking at faux at all 😅 .

Hm this use case isn't supported by faux yet unfortunately although looking at my explanation from a year ago I think I could create a hack around to at least get it to compile, although maybe not make them mockable. That'll take me a little bit to implement though and my time has been scarce lately unfortunately.

For a workaround for now, if possible, I would try to have that method in an impl block that is not wrapped by #[cfg_attr(test, faux::methods)]. The tricky thing though is that if you do that you cannot use any of the fields in the struct directly but you would have to go through methods that are inside an impl block with the #[cfg_attr(test, faux::methods)] attribute. Something like:

#[cfg_attr(test, faux::create)]
struct Foo {
    /* snip */
}

// THIS ONE IS WRAPPED BY `faux::methods`
#[cfg_attr(test, faux::methods)]
impl Foo {
    async fn inner_authenticated_request(
        &self,
        path: &str,
        token: &str,
    ) -> Result<String, CommonError> {
        /* snip your implementation */
    }
}

// NOT WRAPPED BY `faux::methods`
impl Foo {
    pub async fn get_authenticated_request<T: DeserializeOwned>(
        &self,
        path: &str,
        token: &str,
    ) -> Result<T, CommonError> {
        let raw = self.inner_authenticated_request().await?;
        /* snip convert raw string to deserialized T */
    }
}

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

No branches or pull requests

4 participants