Fix tests error log by enabling LogPlugin only once#713
Conversation
|
CI is broken |
Yeah. But it doesn't look like it's due to this change. |
|
Unfortunately I dont have permissions to merge anything if CI isn't passing. So either this PR or a new PR to fix it is fine. |
I have opened a CI fix PR in #714. |
|
Merged, go ahead and rebase :) |
7ef610f to
11133be
Compare
|
I'll deal with the test failure tomorrow. |
|
Could just be a flaky test |
|
Hmm... Testing directly on my environment passes, but running on CI environment with Also, I noticed that benches have the same issue on |
Job succeed on another local |
When running tests, there are many: ``` ERROR bevy_log: Could not set global logger and tracing subscriber as they are already set. Consider disabling LogPlugin. ``` Because tests usually run in the same context, but each test tries to set logger and tracing subscriber, which should be only set once in a context. This commit takes the approach of only setting up [LogPlugin] once in the whole test run, rather than removing [LogPlugin] entirely for all test apps. Although there are no logs in Valence's existing tests currently, keeping the logger available is useful for debugging single tests in the future. Also note by calling `testing::add_plugins`, `example_test_server_tick_increment()` and `idle_update` now disables [NetworkPlugin], which should be okay in these tests. Ref <valence-rs#712>
9bbba58 to
770d563
Compare
Objective
Fixes #712.
When running tests, there are many:
Because tests usually run in the same context, but
each test tries to set logger and tracing subscriber,
which should be only set once in a context.
Solution
This commit takes the approach of only setting up
[LogPlugin] once in the whole test run, rather than
removing [LogPlugin] entirely for all test apps.
Although there are no logs in Valence's existing tests
currently, keeping the logger available is useful for
debugging single tests in the future.
Also note by calling
testing::add_plugins,example_test_server_tick_increment()andidle_updatenow disables [NetworkPlugin],which should be okay in these tests.