nrxus / faux

Struct mocking library for Rust
https://nrxus.github.io/faux/
MIT License
411 stars 14 forks source link

Doesn't work correctly with async_trait #28

Closed sazzer closed 4 years ago

sazzer commented 4 years ago

I'm implementing a Clean Architecture, where I've got a number of Traits that represent my Use Cases, and then a single Struct that implements these. That works great, and means that I've got good structure to my code.

However, testing it has been awkward in Rust. Faux seems to be ideal because I can mock out the structs and not need to have generics everywhere, but it doesn't seem to work with async_trait. By the errors, I'm needing to return a Pin<Box<Future<Output = XXX>> from my mock instead of just an XXX, but actually doing that is not always trivial.

nrxus commented 4 years ago

By async_trait do you mean: https://github.com/dtolnay/async-trait

Could you please provide an example of the code you are attempting to write?

Does the error occurs during the expansion of #[faux::methods] or when attempting to mock the method with when!(...).then(...)?

sazzer commented 4 years ago

I do mean that crate, yes.

I'll try and put together a simple example of the problem, but basically:

In my test, I have the line:

    faux::when!(player_service.register_player).safe_then(|_| Err(PlayerRegistrationError::UnknownError));

The struct player_service is defined as:

/// Use case for registering players
#[async_trait]
pub trait PlayerRegistration {
  /// Attempt to register a player account, or return the existing one if it's already registered
  async fn register_player(&self, details: Registration)
    -> Result<Player, PlayerRegistrationError>;
}

/// The Player Service to interact with players
#[cfg_attr(test, faux::create)]
pub struct PlayerService {
  pub(super) repository: PlayerRepository,
}

#[cfg_attr(test, faux::methods)]
impl PlayerService {
  /// Create a new Player Service
  pub fn new(repository: PlayerRepository) -> Self {
    Self { repository }
  }
}

#[async_trait]
#[cfg_attr(test, faux::methods)]
impl PlayerRegistration for PlayerService {
  /// Attempt to register a player account, or return the existing one if it's already registered
  async fn register_player(
    &self,
    details: Registration,
  ) -> Result<Player, PlayerRegistrationError> {
    log::info!("Registering player: {:?}", details);
    todo!()
  }
}

The idea being that PlayerRegistration is a "Use Case" as defined by the Clean Architecture, and so is a trait. This needs to be async because it's ultimately working in terms of bb8, which is itself all async.

However, building the above gives me:

error[E0308]: mismatched types
   --> src/authentication/google/provider.rs:316:63
    |
316 |     faux::when!(player_service.register_player).safe_then(|_| Err(PlayerRegistrationError::UnknownError));
    |                                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                                                               |
    |                                                               expected struct `std::pin::Pin`, found enum `std::result::Result`
    |                                                               help: you need to pin and box this expression: `Box::pin(Err(PlayerRegistrationError::UnknownError))`
    |
    = note: expected struct `std::pin::Pin<std::boxed::Box<dyn static_assertions::core::future::future::Future<Output = std::result::Result<entity::entity::Entity<players::models::player_id::PlayerId, players::models::player::PlayerData>, players::usecases::registration::PlayerRegistrationError>> + std::marker::Send>>`
                 found enum `std::result::Result<_, players::usecases::registration::PlayerRegistrationError>`
nrxus commented 4 years ago

I think the trick to get this to work is to flip proc macro order. If you put #[cfg_attr(test, faux::methods)] above #[async_trait] it seems to work.

I wrote a little short snippet: https://github.com/nrxus/faux-playground/blob/master/src/lib.rs

I admit it is not very intuitive and maybe I should document it somewhere.... Although I am also unsure if I should "commit" to faux working with other macro libraries when it doesn't yet work with all of the rust features :laughing:

Not wanting to commit to faux working with async-trait is why I put it on a separate repo rather than a test within faux, since I don't want to create the appearance that I am testing against it with every change if that makes sense.

nrxus commented 4 years ago

I am honestly surprised that it "just" works. I will write down below some implementation reasons why it works just to note them down, but they are implementation detail-y so no need for anyone to read (:

faux supports inherent async fns through a fairly ~hacky~ simple change: call .await when proxying to the real method, otherwise behave as normal. faux deliberately makes it so that when! behaves as if it was NOT async, thus not needing any futures or pins or what not.

If faux gets to do it's expansion before async-trait's macro then it still thinks that the function is just a normal async fn and expands as usual. The "real" method is copied with all of its original attributes (i.e., proc macros). async-trait then does it's magic on the real method, but faux does not really care because at the end of the day it can call .await on it so it's fine. async-trait then ALSO does its magic on the morphed method that faux expanded to, changing the signature to no longer be async, but since faux already run then it all goes smooth.

I am honestly SHOCKED but pleased that this works, it validated my original assumption that I should keep all of the attributes and whatnot on both the original and the morphed method :grinning:

sazzer commented 4 years ago

Interesting. That's my Java developer showing through - it never even occurred to me that the order of those would have any impact at all! (I still don't fully understand why it does, but it's good enough for now :) )

nrxus commented 4 years ago

I wrote a bit about it in the README that will hopefully help explain it a bit better. Let me know if something still doesn't quite make sense, it definitely needs a couple of edits for clarity but i was unsure exactly how.

I am closing this issue, however, since it does not seem to be an issue that faux could address other than better documentation which the new README section is trying to do.