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

Using DefaultPlugins breaks for multiple tests due to LogPlugin #1969

Closed
MJohnson459 opened this issue Apr 20, 2021 · 7 comments
Closed

Using DefaultPlugins breaks for multiple tests due to LogPlugin #1969

MJohnson459 opened this issue Apr 20, 2021 · 7 comments
Labels
C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@MJohnson459
Copy link
Contributor

MJohnson459 commented Apr 20, 2021

Bevy version

0.5

Operating system & version

Ubuntu 18.04

What you did

I am trying to write tests for systems and encounter this error when there are multiple tests

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

    fn setup() -> App {
        let mut builder = App::build();
        builder.add_plugins(DefaultPlugins);
        builder.app
    }

    #[test]
    fn test_one() {
        let _app = setup();
        assert!(true);
    }

    #[test]
    fn test_two() {
        let _app = setup();
        assert!(true);
    }
}

What you expected to happen

Log instances shouldn't conflict

What actually happened

Could not set global default tracing subscriber. If you've already set up a tracing subscriber, please disable LogPlugin from Bevy's DefaultPlugins

Additional information

A possible fix might be to remove the explicit panics if creating a global subscriber fails

.expect("Could not set global default tracing subscriber. If you've already set up a tracing subscriber, please disable LogPlugin from Bevy's DefaultPlugins");

@MJohnson459 MJohnson459 added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Apr 20, 2021
@alice-i-cecile
Copy link
Member

Fully testing the AppBuilder like this is hard; see #1720. You may be interested in #1714, which provides an alternative strategy.

@alice-i-cecile alice-i-cecile added core C-Usability A targeted quality-of-life change that makes Bevy easier to use C-Bug An unexpected or incorrect behavior and removed C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Apr 20, 2021
@MJohnson459
Copy link
Contributor Author

Just to add some context for my specific problem, I'm using bevy headless with default-features = false so most of the threading or rendering problems (e.g. #1720 ) don't affect me. I'm also trying to test functions related to the bevy_rapier2d plugin which requires app.add_plugins(RapierPhysicsPlugin) - this makes it difficult to use world directly as in #1714 because each resource/system would need to be copied from the plugin manually.
(discussed on discord https://discord.com/channels/691052431525675048/742569353878437978/833666064063135766)

In other words, my use case is pretty niche. Normally I would just ignore this issue because of that but it does seem like this could be a relatively low-hanging fix to remove one of the "global" systems and make this case supported with default-features = false.


Note that I do have a work around by copying from the DefaultPlugin minus the LogPlugin so this is not in any way critical:

        builder
            .add_plugin(CorePlugin::default())
            .add_plugin(TransformPlugin::default())
            // ... etc
            ;

Finally, prior to switching to bevy I was also using tracing and rather than setting the global subscriber directly I was using something like this:

let _ = tracing_subscriber::fmt::fmt()
    .with_max_level(Level::DEBUG)
    .try_init();

This assumes that the failure is always because the global subscriber already exists which the documentation just calls "likely"

Returns an Error if the initialization was unsuccessful, likely because a global subscriber was already installed by another call to try_init.

@mockersf
Copy link
Member

mockersf commented Apr 20, 2021

in LogPlugin, there is an opinionated way to start a tracing collector. if you want to provide your own, you can disable the plugin from DefaultPlugins:

   builder.add_plugins_with(DefaultPlugins, |group| {
        group.disable::<bevy::log::LogPlugin>()
    });

@mockersf
Copy link
Member

would the doc added in #1973 be good enough for you to avoid this issue?

@MJohnson459
Copy link
Contributor Author

I'm not sure. On one hand the extra documentation is great but on the other removing the LogPlugin still forces users to add their own log handler for tests (or forgo output). I think ideally I would prefer the LogPlugin to survive being used in tests but I can also see the case for just disabling it.

I am happy for this to be closed if the other option isn't something that would be desired by others.

@Bauxitedev
Copy link

Bauxitedev commented Jun 11, 2021

I've got a related issue I think. Whenever I call info! or any of the logging macros provided by bevy::prelude in my tests, the output doesn't show up when I run cargo test at all (even with the --no-capture flag). It works when I use log's macros directly, but I don't want to add that to all my files, because then I run the risk of accidentally calling bevy's logger macros instead of log's logger macros if I forget it or re-arrange my imports, which means the output silently doesn't get printed during tests. Any ideas?

@alice-i-cecile
Copy link
Member

Fixed by #6411.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

No branches or pull requests

4 participants