Skip to content

Remove test_logger feature#9125

Merged
mergify[bot] merged 6 commits into
sigp:unstablefrom
chong-he:test-logger
Apr 30, 2026
Merged

Remove test_logger feature#9125
mergify[bot] merged 6 commits into
sigp:unstablefrom
chong-he:test-logger

Conversation

@chong-he

Copy link
Copy Markdown
Member

Issue Addressed

Proposed Changes

Remove all instances of test_logger in the code

Additional Info

Running with, e.g.:
cargo test --release -p beacon_chain - no beacon node logs are printed
cargo test --release -p beacon_chain -- --nocapture - beacon node logs are printed

@chong-he chong-he added test improvement Improve tests low-hanging-fruit Easy to resolve, get it before someone else does! labels Apr 13, 2026
@chong-he chong-he requested a review from jxs as a code owner April 13, 2026 07:52
@chong-he chong-he added the ready-for-review The code is ready for review label Apr 13, 2026
@macladson macladson self-requested a review April 14, 2026 07:25
}
```

### test_logger

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could add some docs here about how to use the new logger. Does the EnvFilter let us control the log level for different components, etc?

It might be nice to even filter out dependency logs by default? (I know the HTTP crates are quite noisy)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some revision to the doc, and we can use the RUST_LOG environment variable to control the log level. The default is debug but we can change it with RUST_LOG=info, tested and it's working

@michaelsproul michaelsproul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 30, 2026
@mergify mergify Bot added the queued label Apr 30, 2026
@mergify

mergify Bot commented Apr 30, 2026

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 30 minutes 57 seconds in the queue, including 28 minutes 16 seconds running CI.

Required conditions to merge

mergify Bot added a commit that referenced this pull request Apr 30, 2026
@mergify mergify Bot merged commit 8d77b1c into sigp:unstable Apr 30, 2026
39 checks passed
@mergify mergify Bot removed the queued label Apr 30, 2026
@chong-he chong-he deleted the test-logger branch April 30, 2026 05:42
@chong-he chong-he mentioned this pull request Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-merge This PR is ready to merge. test improvement Improve tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants