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

Come up with a logging solution for tests. #357

Closed
Yawning opened this issue Jun 5, 2018 · 3 comments
Closed

Come up with a logging solution for tests. #357

Yawning opened this issue Jun 5, 2018 · 3 comments
Labels
c:testing Category: testing p:2 Priority: desired feature

Comments

@Yawning
Copy link
Contributor

Yawning commented Jun 5, 2018

We have standardized on log as our logging library. The pretty_env_logger (or env_logger for that matter) logs to stderr, but the output file descriptor can be changed at initialization time to stdout. cargo test is supposed to capture all writes to stdout to avoid spam (cargo test -- --nocapture disables this behavior), but this is currently broken (rust-lang/rust#42474), in the context of multi-threaded code.

It would be nice to have logs that can be enabled/disabled for the tests, if only for debugging.

@Yawning Yawning added p:2 Priority: desired feature c:testing Category: testing labels Jun 5, 2018
@Yawning
Copy link
Contributor Author

Yawning commented Jun 5, 2018

This seems to work, and I'm using it in the ethereum random beacon test case.

fn try_init_logging() {
    // `pretty_env_logger` logs to stderr by default.  While it could be
    // re-targetted to log to stdout, Rust/Cargo bugs prevent stdout from
    // threads being captured, so there's no point.
    //
    // See: https://github.com/rust-lang/rust/issues/42474

    for arg in std::env::args() {
        if arg == "--nocapture" {
            pretty_env_logger::formatted_builder()
                .unwrap()
                .filter(None, LevelFilter::Trace)
                .init();
            return;
        }
    }
}

@pro-wh
Copy link
Contributor

pro-wh commented Jun 5, 2018

why should we configure it to to write to stdout?

@Yawning
Copy link
Contributor Author

Yawning commented Jun 5, 2018

Because tests run by cargo test will squelch writes to stdout (well, it will try to, it's kind of broken) unless --nocapture is explicitly specified, in order to keep output readable. I think this is reasonable behavior because "pages and pages of web3 logs for the random beacon test" for example, aren't useful to most people unless they're debugging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:testing Category: testing p:2 Priority: desired feature
Projects
None yet
Development

No branches or pull requests

2 participants