Feature Request: Abstractable Types for unit testing

[Gisleburt]

One of the big reasons to choose Rust for game dev is its incredibly good built in testing tools.

Currently, the way that we wire custom classes, its not possible to write unit tests as the struct depends on the godot engine being present.

Take this overly simplified class:

#[derive(GodotClass, Debug)]
#[class(base = Node)]
pub struct AppState<'b> {
    #[base]
    base: Base<Node>,
    // some other fields
}

#[godot_api]
impl AppState {
    #[signal]
    fn paused();

    #[func]
    pub fn pause(&mut self) {
        // some other logic
        self.base.emit_signal("paused".into(), &[]);
    }
}

You might want to check that the logic in pause works and that emit_signal was called with paused as a parameter.

One way we could do this is by passing AppState a version of Base that stores what signals have been called:

#[cfg(test)]
mod tests {
    use super::*;

    // todo
    #[test]
    fn test_app_can_be_paused() {
        let base = InMemoryBase::new();
        {
            let mut app_state = AppState::new(&mut base);
            app_state.pause()
        }
        assert!(base.get_signals().contains("paused")); 
    }
}

I've tried a few of the suggestions for abstraction to allow my core logic to be tested but largely means writing a second struct with the same function names and then just having one call the other, meaning not only is it a lot of extra work to do this, but still leaves large parts of the code untestable, not to mention things like signals which can't be called from the other structs. For things like AppState it really doesn't make sense to have separate logic as all its doing is managing the changing of state and allowing other things to query the state so there's no way for me to test it.

I would love to help on this, but don't really know where to start in the codebase. It seems like we'd have to make a Trait around godot::engine::Object, then somehow allow Custom Class's to take other things that impl that trait without breaking the custom macro's ability to wire the struct.

I'm sure there are other ways to get the same results too.

Thoughts?

[Bromeon]

Thanks for starting an interesting design discussion! 🙂

#[test]
fn test_app_can_be_paused() {
    let base = InMemoryBase::new();
    {
        let mut app_state = AppState::new(&mut base);
        app_state.pause()
    }
    assert!(base.get_signals().contains("paused"));
}

If I understand you correctly, you're asking for a mock for Base<Node>, which supports at least the methods emit_signal() and get_signals().

To understand your use case better, two questions:

1. How would you implement this, without manually replicating the Godot API, just without the Godot engine?
2. Even if we had a mock that replicates Godot functionality -- would it really add that much value, given that there may always be small discrepancies in the behavior, as well as the possibility that Godot changes implementation? Would an integration test not fit better if you want to test Godot integration?

[Gisleburt]

Thanks Bromeon,

My thinking here is that I want to test my code, not the libraries code, so being able to almost entirely abstract out the library from my code would be the ideal circumstance.

I actually think I might be onto something that would let people write their own mocks simply by adding in generics and where clauses to godot-macros. From the code it looks like it was the plan to do this at some point. My changes to godot-macros builds at least, I'm just having a bit of trouble with the godot part of the project due to some issue with clang. 🤔

I'll open a PR for further discussion, one sec. 😄

[Gisleburt]

The PR has a little more info. #37

For clarity, I just picked emit_signal for the example, I think you should be able to stub/mock whatever functionality you like.

[Bromeon]

My thinking here is that I want to test my code, not the libraries code, so being able to almost entirely abstract out the library from my code would be the ideal circumstance.

Simply making the #[base] field generic won't work, because the whole library expects that objects deriving GodotClass conform to certain semantics, including tight integration into the engine. In other words, #[derive(GodotClass)] is a contract. For example:

• The proc-macro generates an implementation for GodotExt with constructor and virtual functions (ready(), to_string() etc).
• The object must be able to be stored inside Gd<T> smart pointers, which expects engine support for things like instance ID, variant conversions, Godot to_string(), etc.
• There are Deref/DerefMut impls and an Inherits class hierarchy, allowing operations like cast or upcast.

All these operations are not meaningfully possible with a "fake base" (aka mock), and most of them need to be available at compile time for type safety. Also, many types (even fundamental ones like GodotString) require engine support to just be instantiated.

---

That said, I totally understand your use case of abstracting away Godot interactions. I just think the abstraction boundary should be moved a bit 🙂

Concretely, due to the contract mentioned above, the most straightforward solution might be to not derive GodotClass in the first place if you are in a unit test. Something like this, untested:

#[cfg_attr(not(test), derive(GodotClass, Debug))]
#[cfg_attr(not(test), class(base = Node))]
pub struct AppState {
    // Running in engine
    #[cfg(not(test))
    #[base]
    base: Base<Node>,

    // Running in unit-test
    #[cfg(test)]
    base: NodeMock, // your custom impl
}

Maybe some of these things could be made a bit more ergonomic over time, but it might already serve as a start.

[Gisleburt]

Oh nice! I tried something similar to that but couldn't get it working, this does... nearly 😂

It works for my tests but breaks when used with #[godot_api]. This is what I have:

#[cfg_attr(not(test), godot_api)]
impl AppState {
    //...
    #[cfg_attr(not(test), func)]
    pub fn is_not_playing(&self) -> bool {
        self.state == State::NotPlaying
    }
    //...
}

This works in test, but in build I get:

error: cannot find attribute `func` in this scope
  --> src/state/app_state.rs:52:27
   |
52 |     #[cfg_attr(not(test), func)]
   |                           ^^^^

If I remove the cfg_attr from func it works in build, but in test I get:

error: cannot find attribute `func` in this scope
  --> src/state/app_state.rs:52:7
   |
52 |     #[func]
   |       ^^^^

If I remove the cfg_attr from godot_api that opens up a can of worms because we're not deriving GodotClass in test

error[E0277]: the trait bound `app_state::AppState: godot::prelude::GodotClass` is not satisfied
  --> src/state/app_state.rs:32:1
   |
32 | #[godot_api]
   | ^^^^^^^^^^^^ the trait `godot::prelude::GodotClass` is not implemented for `app_state::AppState`

This is super close though, and I can live with my code being a hilarious proportion of annotations 😅

[Gisleburt]

Solved... with even more annotations 😂

The problem is, Rust doesn't necessarily evaluate cfg or cfg_attr in a sensible way, so its turnning on func before gotot_api. This can be fixed with cfg_eval

In my crate root, I turn on the nightly feature for cfg_eval

#![feature(cfg_eval)]

Then I add cfg_eval to the struct impl:

#[cfg_eval]
#[cfg_attr(not(test), godot_api)]
impl AppState {
    // ...
    #[cfg_attr(not(test), func)]
    pub fn is_not_playing(&self) -> bool {
        self.state == State::NotPlaying
    }
}

Now it evaluates all cfg annotations first, then deals with everything else.

[Gisleburt]

Thank you so much @Bromeon, happy to close this unless you particularly want to look at alternatives

[Bromeon]

I see -- the problem is that #[func] is not itself a proc-macro attribute, but just an attribute. That is, it does nothing on its own but is consumed by the outer #[godot_api] (which is a proc-macro attribute).

One solution I can think of right now: you would write your own proc-macro like #[godot_api], that simply goes through the entire impl block, strips all attributes from functions, and returns the resulting impl. Then, you could conditionally use either godot-rust's or your version:

#[cfg_attr(test, my_test_api)]    // <- your proc macro attribute
#[cfg_attr(not(test), godot_api)] // <- godot-rust's
impl AppState {
    #[func] // <- always like this, no cfg
    pub fn is_not_playing(&self) -> bool {
        self.state == State::NotPlaying
    }
}

You can look into the godot_api.rs file to see how this can be done with venial, it's relatively simple. You just need a separate crate to run proc macros.

You could even try that your proc-macro outputs a #[godot_api] in non-test configurations, then you would only need one attribute and no #[cfg] at all in client code. I'm not 100% sure whether this works though.

#[my_test_api]    // <- in non-test, evaluates to:    #[godot_api] impl AppState { /* verbatim */ }
impl AppState {
    #[func]
    pub fn is_not_playing(&self) -> bool {
        self.state == State::NotPlaying
    }
}

---

Just saw you found in the meantime an approach with #[cfg_eval]. I didn't know this one, nice! Still nightly but looks very useful. You might still consider my suggestion to cut down on annotation hell 😀

[Bromeon]

Closing -- the problem here has been addressed through conditional compilation, see last few posts 🙂

More utilities for testing/mocking might emerge in the future, but this issue is a bit too specific for a general approach to that.

[Gisleburt]

I'm going to try adding two new attributes #[not_test] and #[not_doctest] and see if I can disable everything else if they're present by checking cfg!(...) for that flag.

A much nicer UX, I think, would be to have a #[godot_cfg(...)] attribute so that you can do #[godot_cfg(not(test, doctest))], however, I'm not sure if you can evaluate the contents of the macro (i.e. if cfg!(token_stream.do_something_here()) at the same time as reading it.