Feature Request: Abstractable Types for unit testing about gdext HOT 10 CLOSED

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.

from gdext.

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?

from gdext.

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. 😄

from gdext.

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.

from gdext.

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.

from gdext.

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 😅

from gdext.

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

from gdext.

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 😀

from gdext.

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.

from gdext.

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.

from gdext.