Fix: RPC test failures #7734
Open
Fix: RPC test failures #7734
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chong-he
added
test improvement
jimmygchen
reviewed
Jul 11, 2025
| tracing_subscriber::fmt() | ||
| .with_env_filter(EnvFilter::try_new(level).unwrap()) | ||
| .init(); | ||
| }); |
There was a problem hiding this comment.
Thanks for the PR!
I noticed a little limitation with this approach though - once tracing is initialised globally (via init) with a specific log level (e.g., "info"), any subsequent tests that try to use a different level (e.g., "debug") won't see their preferred level applied, and it would not be obvious why debug logs aren't showing.
I think we could try to create test-local subscribers using the tracing::subscriber::set_default method or something similar, which returns a guard that unregisters the subscriber when dropped.
Some RPC tests were failing intermittently due to a race condition when initializing the global tracing subscriber. This could cause a panic if multiple tests attempted to initialize the subscriber simultaneously. This change prevents the race condition, making the tests more stable.
Use `tracing::subscriber::set_default` for setting the subscriber on a thread/test. Using `std::sync::Once` would cause any subsequent tests that try to use a different level won't see their preferred level applied, and it would not be obvious why debug logs aren't showing.
danielrachi1
force-pushed
the
rpc_tests_fix
branch
from
c936b1b to
d86c218
Compare
jimmygchen
reviewed
Jul 22, 2025
| .with_env_filter(EnvFilter::try_new(level).unwrap()) | ||
| .try_init() | ||
| .unwrap(); | ||
| tracing::subscriber::set_default( |
There was a problem hiding this comment.
I think the subscriber is immediately dropped because the return value is not used here. Have you tested this locally?
jimmygchen
added
waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Addressed
Fixes #7735
Proposed Changes
Use
tracing::subscriber::set_defaultto ensure that each test/thread has its own subscirber.