Utility functions for integration testing

[alice-i-cecile]

Objective

• Integration testing Bevy apps is heavy on boilerplate
• Several strategies are used repeatedly
• Solves Add convenience methods for integration testing the ECS #3814.
• Solves Provide a way to test Bevy App #2896.

Solution

• Demonstrate how to integration test apps effectively
• Implement some related helpers on World

Limitations

Input mocking has not been included. Fundamentally; we need an abstraction over the input methods to handle this nicely.
See https://github.com/Leafwing-Studios/leafwing-input-manager/blob/main/src/input_mocking.rs for an external implementation of this.

Working with queries in integration tests still fundamentally sucks, because of the need to store query state somewhere. As a result, I've included some uncomfortably special-cased helpers.

TODO

• FIx CI
• Add set_component
• Add run_system
• Hide methods behind a trait that's not in the prelude

    fn set_component<C, F>(&mut self, value: &C)
    where
        C: Component + PartialEq + Debug + Clone,
        F: WorldQuery,
        <F as WorldQuery>::Fetch: FilterFetch,
    {
        let mut query_state = self.world.query_filtered::<&mut C, F>();
        for mut component in query_state.iter_mut(&mut self.world) {
            if *component != *value {
                *component = value.clone();
            }
        }
    }

[shanesveller]

Very interested in this topic. In the past I've used a minimal App, a setup startup system and an examine system in CoreStage::Last, along with an enum resource to define common initial scenarios. Then I add the systems under test to the other stages as appropriate, set it to RunOnce and let 'er rip. It seems relatively effective, but that definitely winds up being quite a lot to put in one test method, both cognitively and probably for runtime cost as well.

[alice-i-cecile]

@shanesveller This PR is now ready for review; I'd love to hear what you think of the end result.

[james7132]

Overall, I think this is very useful, but this seems like something that should be behind a #[cfg(test)] conditional, or separated out entirely an optional dev-dependency. It's definitely not optimal to be compiling testing code into production binaries, even if it gets optimized out.

[alice-i-cecile]

Overall, I think this is very useful, but this seems like something that should be behind a #[cfg(test)] conditional, or separated out entirely an optional dev-dependency. It's definitely not optimal to be compiling testing code into production binaries, even if it gets optimized out.

That's an interesting idea. I think that I agree with you on the assertions, but the other helpers have important non-testing uses (even if they don't enable new functionality). I'll shuffle this around.

[mockersf]

I would prefer those methods to not be available during standard development, but they should be documented on docs.rs. I think the best way to have both would be to have everything in a bevy_test crate and use it only behind a #[cfg(test)]. But that seems not very nice to do... So my next preference would be to have disclaimers in the docs that those methods should be used for test purpose only.

query_len

aka the one I dislike the less... Ok I guess it's nice to hide the query building in an helper.

send_event and events_len

in both those cases, I find the Events<E> struct to be easy enough already to work with. I would prefer an helper method on World to get the events struct, and its friend on App, and then showing it's easy to call send or len on it. As it's limited in scope to tests, exposing a mut getter on the events resource should be fine.

assert_component_eq

I find the name very confusing, and the functionality very limited.

• naming: it doesn't have any plural in it
• functionality: limited to eq on the whole component, what if I wanted to pass a closure that would have a custom check

I think what I would like is a method returning an iterator on the result of the query on that component with the filter that are passed as type param, so something like fn get_components<C, F>(&mut self) -> impl Iterator<Item = C>. Then I can use all the tools on iter to do my checks.

assert_system

This one is hard to understand from the name. I don't have a good suggestion though. It also can't be used with any system as it requires one that output a Result, which is not somethings that's nice to use in Bevy by default. I don't really see the point of testing a system that would be built only for tests. Unless we start to push for systems returning a Result much more widely in Bevy, I don't see this being really useful...

After reading the example integration test

I have even a strong feeling that what would be really useful is hiding the complexity of getting an iterator from a query which is something you often do:

• to set a component https://github.com/bevyengine/bevy/pull/3839/files#diff-e04ddf25e6b50e8f06a88740d3a07bb96bfef546bf36afc347f18027fc8c66bfR122-R123
• to get the value to assert on it https://github.com/bevyengine/bevy/pull/3839/files#diff-e04ddf25e6b50e8f06a88740d3a07bb96bfef546bf36afc347f18027fc8c66bfR130-R131 (see how you don't use assert_component_eq as you want to check only one of its attribute? and that it's <?)

So merging query_len and assert_component_eq into an easy helper to get the iterator from a query.

About the example itself used in the test, I don't think it's a good idea to show something that is using Velocity or Gravity without checking time at any point. If someone was to copy this code as a basis, it would work in unexpected way.

[mockersf]

an examine system in CoreStage::Last

That could be an interesting pattern to show for tests: put all your invariants check in a system, and run that systems after an update

[alice-i-cecile]

I think I agree with basically all of your proposals. I'll make those changes and then re-request review :)

[alice-i-cecile]

Alright, so I think this is the best we can do for now. Fundamentally, I want to be able to query the world in a single line, but, because query is assumed to have a cached state stored somewhere, this is impossible.

The only decent work-around is to include special-cased helper methods, to reduce the intense boilerplate of checking "is this value what I expect".

In the long run, I very much want a better solution, but that will involve much more serious design work and engine refactoring.

[alice-i-cecile]

Closing this out in favor of something more scoped.