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

Provide a way to test Bevy App #2896

Open
arialpew opened this issue Oct 1, 2021 · 11 comments
Open

Provide a way to test Bevy App #2896

arialpew opened this issue Oct 1, 2021 · 11 comments
Labels
A-Core Common functionality for all bevy apps C-Examples An addition or correction to our examples C-Testing A change that impacts how we test Bevy or how users test their apps D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@arialpew
Copy link

arialpew commented Oct 1, 2021

What problem does this solve or what need does it fill?

Bevy App should be testable in headless mode (no renderer), for Continuous Integration and fast feedback when user logic / system interaction is not what user intendeed.

What solution would you like?

Idealy, a complete example for testing a Bevy App should be provided out of the box.

Currently, there's a minimal example provided here : https://github.com/bevyengine/bevy/blob/main/crates/bevy_ecs/examples/resources.rs

This doesn't work well because you have no way to test things that happen "later in time". You advance Bevy synchronously with iteration in a for loop. If you sleep the main thread between each iteration, you'll advance in time but the side effect is test will be slower. So this example work well for startup system or system that run each tick, but this doesn't work for system that happen "later" in time.

We need a way to test a Bevy Application and travel time to a future point where Bevy State is updated, so assertion can work, for all case (FixedTimeStep System or not).

  1. Provide a way to advance or scale Bevy time, skip frame, for test (custom Scheduler ? Or maybe the current default Scheduler is enough ?).
  2. Test should be runnable concurrently with cargo test, some plugins are not compatible (for example, LogPlugin is not usable in context of a test if 2 Bevy App use 2 instance of LogPlugin concurrently).
  3. Documentation.

What alternative(s) have you considered?

Don't bother with testing, stay like this.

Honestly, for small Bevy App you don't need test, but for complex system interaction it's a nice feature.

I'm confident about Bevy behavior and logic, but I don't trust my own code especially when it's inverted control (https://en.wikipedia.org/wiki/Inversion_of_control, the framework is responsible to run my application).

A nice side effect of this feature is if you make Bevy easily testable (out of the box with cargo test), contributors will have a better understanding of Bevy code base/change.

Additional context

Related : #1057 and #1720 and #2687

@arialpew arialpew added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Oct 1, 2021
@bjorn3
Copy link
Contributor

bjorn3 commented Oct 1, 2021

This is already partially supported and used on CI:

time CI_TESTING_CONFIG=$example VK_ICD_FILENAMES=$(pwd)/vk_swiftshader_icd.json DRI_PRIME=0 xvfb-run cargo run --example $example_name --no-default-features --features "bevy_dynamic_plugin,bevy_gilrs,bevy_gltf,bevy_wgpu,bevy_winit,render,png,hdr,x11,bevy_ci_testing"

The bevy_ci_testing feature is used to enable it and the CI_TESTING_CONFIG env var is used to pass the path to a config file like

(
exit_after: Some(300)
)
. It does still need swiftshader for software rendering though. It can currently only be used to test that the game doesn't crash within the specified amount of time.

@arialpew
Copy link
Author

arialpew commented Oct 1, 2021

You can run a crash test, but it's not robust enough to cover logical/game behavior errors from user point of view. In a context of ECS where the framework take control of the application, I think this should be something like this :

  1. Unit test system in isolation (single system test).
  2. Integration test with a slice of the whole application (multiple systems running concurrently).
  3. Crash/bench test for a long period of time with a whole application (memory usage, CPU usage, tickrate, FPS in case of render, ...).

@alice-i-cecile alice-i-cecile added A-Core Common functionality for all bevy apps C-Testing A change that impacts how we test Bevy or how users test their apps and removed S-Needs-Triage This issue needs to be labelled labels Oct 1, 2021
@alice-i-cecile alice-i-cecile added C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy and removed C-Feature A new feature, making something new possible labels Jan 24, 2022
@alice-i-cecile
Copy link
Member

This is now approximately solved (see #1057) for more status. It just needs a proper example in Bevy, probably in the tests folder.

If you're looking to tackle this, feel free to steal from my own work without attribution.

@djeedai
Copy link
Contributor

djeedai commented Dec 26, 2023

For clarity, this issue is not solved (#1057 actually redirects here) and all related issues point to this one as the tracking one.

This is still very much an open issue; even without any mocking involved, it's impossible to run an app with DefaultPlugins inside a #[test] function, and I couldn't find any explanation why as the error message is either cryptic (panic on unwrap()) or non-actionable (something about main thread, that has no explanation about why we ended in this state nor how to avoid).

thread 'plugin::tests::add_plugin' panicked at [...]\winit-0.28.7\src\platform_impl\windows\event_loop.rs:194:13:
Initializing the event loop outside of the main thread is a significant cross-platform compatibility hazard. If you absolutely need to create an EventLoop on a different thread, you can use the EventLoopBuilderExtWindows::any_thread function.

@alice-i-cecile
Copy link
Member

That error will be related to NonSend resources: tests are generally spun up in paralllel. Note that running apps with default plugins in tests will also be quite challenging in most CI setups, even if that is fixed, as they are not set up for rendering.

@djeedai
Copy link
Contributor

djeedai commented Dec 26, 2023

Thanks for the explanation @alice-i-cecile. It would be good to have an exhaustive list of the blockers listed on this GitHub issue (or others). I think there's that NonSend winit thing you mention, the headless rendering, maybe thread starvation, ... anything else?

Unfortunately there's no easy way to test any impl Plugin that I know of except through App, which is the pain point here for all crate maintainers.

@alice-i-cecile
Copy link
Member

Audio is also likely to be challenging to test in CI. Our screenshotting and input mocking tools are also limited.

@djeedai
Copy link
Contributor

djeedai commented Dec 27, 2023

But are those really blockers, or just missing tools for specific test types? What I was listing were things that I think block App::run() today. Having input mock is only useful for testing input, but you can do a lot already without it.

@emarcotte
Copy link

emarcotte commented Dec 27, 2023

I have been pretty successfully testing systems that just tweak data (rather than graphical side effects) by making headless windows, disabling winit, and manually fast-forwarding time in between calls to update (or calling run system once). Each of the last few releases had made this more seemless.

Basic gist is something like:

...
    let mut app = App::default();

    let default_plugins = DefaultPlugins
        .build()
        .set(RenderPlugin {
            render_creation: WgpuSettings {
                backends: None,
                ..default()
            }
            .into(),
        })
        .disable::<LogPlugin>()
        .disable::<WinitPlugin>();

    app.add_plugins(default_plugins)

....

/// Fast forward the bevy clock. This is probably very buggy. Have fun!
pub(crate) fn fast_forward(app: &mut App, dur: Duration) {
    app.world
        .get_resource_mut::<Time>()
        .expect("NO TIME!")
        .advance_by(dur);
}

... actual test

        // Run the system 1 time: shouldn't spawn
        app.world.run_system_once(super::spawn);
        let mut res = app.world.query::<&Baddie>();
        assert_eq!(
            0,
            res.iter(&app.world).count(),
            "Shouldn't have spawned anything yet"
        );

        // Fast forward a coupld seconds, it should.
        fast_forward(&mut app, Duration::from_secs(2));
        app.world.run_system_once(super::spawn);
        assert_eq!(
            1,
            res.iter(&app.world).count(),
            "Shouldn't have spawned anything yet"
        );

        // Pump events to send the notifications to handlers.
        app.update();
        assert!(app
            .world
            .resource::<EventTracker>()
            .notifications
            .iter()
            .any(|n| n.message == "Spawned a baddie"));

@djeedai
Copy link
Contributor

djeedai commented Mar 1, 2024

Note: This above is fine if you don't need anything from rendering. Unfortunately this skips the entire init of the RenderApp, even though most of it doesn't need a valid window/swap chain or even a renderer (GPU adapter). This means all steps like extract etc. can't be tested. Only the main World is available.

@djeedai
Copy link
Contributor

djeedai commented Mar 1, 2024

These two changes could be made to prevent hanabi from panicking

Wait, why do you need the second one? That seems unrelated 🤔

github-merge-queue bot pushed a commit that referenced this issue Nov 24, 2024
# Objective

This older PR from `Wcubed` seemed well worth saving, adopted from
#7314. See also tracking issue #2896 for ongoing discussion of Bevy
testability. Thanks `Wcubed`!

## Solution

- Updated for 0.15
- Added the `expected`/`actual` pattern
- Switched to function plugin
- Tweaked a bit of description

## Testing

Green.

---------

Co-authored-by: Wybe Westra <dev@wwestra.nl>
Co-authored-by: Wybe Westra <wybe.westra@protonmail.com>
ecoskey pushed a commit to ecoskey/bevy that referenced this issue Dec 2, 2024
# Objective

This older PR from `Wcubed` seemed well worth saving, adopted from
bevyengine#7314. See also tracking issue bevyengine#2896 for ongoing discussion of Bevy
testability. Thanks `Wcubed`!

## Solution

- Updated for 0.15
- Added the `expected`/`actual` pattern
- Switched to function plugin
- Tweaked a bit of description

## Testing

Green.

---------

Co-authored-by: Wybe Westra <dev@wwestra.nl>
Co-authored-by: Wybe Westra <wybe.westra@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Core Common functionality for all bevy apps C-Examples An addition or correction to our examples C-Testing A change that impacts how we test Bevy or how users test their apps D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

No branches or pull requests

5 participants