Avoid unwrap log initialization

[allevo]

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

I would like to test heavily my application. Obviously my test suite is composed by multiple test and every test create a new through App::new() and register the plugin I need to the new app instance.
Because logs are important I really would like keep them active during the tests runs.

What solution would you like?

Remove the unwrap from here

bevy/crates/bevy_log/src/lib.rs

Line 127 in 5e2cfb2

LogTracer::init().unwrap();

substituting it with a different error handling.

What alternative(s) have you considered?

Remove LogPlugin from my tests

Additional context

// test 1
let mut app = App::new();
app.add_plugin(LogPlugin::default());
// test 2
let mut app = App::new();
app.add_plugin(LogPlugin::default()); // <----- this 

Error log:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: SetLoggerError(())', .......\.cargo\registry\src\github.com-1ecc6299db9ec823\bevy_log-0.7.0\src\lib.rs:127:27

[mockersf]

when you are using multiple app (or in test), you should setup login yourself, and not add the LogPlugin several time. That way you would have unified login and no panic 🙂

[allevo]

Hi, thanks for your comment.

I wouldn't like to configure by myself the log part because:

• this is not how other plugins work. In fact I'm able to adding them in different tests and all works fine
• tests can not behave like the debug/release build doesp
• copy and paste the code from bevy code to my one just for removing an unwrap is not so ok
• could be a messy managing a own version for future changes

If my solution (adds a flag force) is not the best one, we can leverage on #[config(test)] to avoid the unwrap.

[bjorn3]

we can leverage on #[config(test)] to avoid the unwrap.

#[cfg(test)] is only enabled for the crate actually being tested, and not for it's dependencies like bevy_log.

[allevo]

unwrap in a library still seems to me an ugly stuff...
Which approach we can take?
Just for knowing if you are interested in this enhancement or no....

[bjorn3]

Honestly to me it feels like LogPlugin shouldn't be a plugin in the first place as it mutates global state and doesn't affect the App at all. Instead I feel like it should be a bevy::log::setup() call at the start of main. This would have a negative effect on ergonomics in case only a single App is used though.

[allevo]

Fair enough.
If you want, I can submit a PR for that

[Ryder17z]

Fair enough. If you want, I can submit a PR for that

please do. having to change it locally is annoying.

[anchpop]

I tried changing LogTracer::init().unwrap(); to if let Ok(_) = LogTracer::init() { here: anchpop@ec8713b#diff-ce0ef3820ac31e1eaf71e97972a68d832239ca8affc9441f6dd419f434d15781R125

It does work at preventing it from panicking, but the problem is that it seems to be circumventing whatever magic cargo test does to suppress output to stdout when --nocapture isn't passed to the test binary. So I get noise like this in my tests:

2022-09-10T23:10:16.374437Z  INFO my_game::file: test log
2022-09-10T23:10:16.375310Z  INFO my_game::file: test log    // ← noise from logs
test tests::change_tests::test::my_test ... ok               // ← test output
2022-09-10T23:10:16.375938Z  INFO my_game::file: test log    // ← noise from logs
2022-09-10T23:10:16.376509Z  INFO my_game::file: test log
2022-09-10T23:10:16.376962Z  INFO my_game::file: test log

@bjorn3 do you know of a solution to this? I would be happy to make a PR if so, since would go well with #5931.

[bjorn3]

Maybe the same thread pool is reused by multiple tests and once the test spawning the thread pool tests initially libtest doesn't know to which test to attribute the output anymore?

[james7132]

This was changed to a non-panicking warning in #6757.