Skip to content

Commit

Permalink
Do log during testing, but "captured" by default
Browse files Browse the repository at this point in the history
- `just test` is silent
- `just test-verbose` is noisy and sequential
- CI is noisy and sequential and `trace` level
  • Loading branch information
DavisVaughan committed Dec 12, 2024
1 parent e2a7858 commit 05c3dc3
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 15 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/test-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,8 @@ jobs:
run: cargo build

- name: Run unit tests
run: cargo test
env:
AIR_LOG: trace
# `--nocapture` to see our own `tracing` logs
# `--test-threads 1` to ensure `tracing` logs aren't interleaved
run: cargo test -- --nocapture --test-threads 1
6 changes: 5 additions & 1 deletion .github/workflows/test-mac.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,8 @@ jobs:
run: cargo build

- name: Run unit tests
run: cargo test
env:
AIR_LOG: trace
# `--nocapture` to see our own `tracing` logs
# `--test-threads 1` to ensure `tracing` logs aren't interleaved
run: cargo test -- --nocapture --test-threads 1
6 changes: 5 additions & 1 deletion .github/workflows/test-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,8 @@ jobs:
run: cargo build

- name: Run unit tests
run: cargo test
env:
AIR_LOG: trace
# `--nocapture` to see our own `tracing` logs
# `--test-threads 1` to ensure `tracing` logs aren't interleaved
run: cargo test -- --nocapture --test-threads 1
27 changes: 19 additions & 8 deletions crates/lsp/src/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use tower_lsp::lsp_types::MessageType;
use tower_lsp::Client;
use tracing::level_filters::LevelFilter;
use tracing_subscriber::fmt::time::LocalTime;
use tracing_subscriber::fmt::TestWriter;
use tracing_subscriber::{
fmt::{writer::BoxMakeWriter, MakeWriter},
layer::SubscriberExt,
Expand Down Expand Up @@ -138,8 +139,13 @@ pub(crate) fn init_logging(
let writer = if client_info.is_some_and(|client_info| {
client_info.name.starts_with("Zed") || client_info.name.starts_with("Visual Studio Code")
}) {
// These IDEs are known to support `window/logMessage` well
BoxMakeWriter::new(LogWriterMaker::new(log_tx))
} else if is_test_client(client_info) {
// Ensures a standard `cargo test` captures output unless `-- --nocapture` is used
BoxMakeWriter::new(TestWriter::default())
} else {
// Fallback for other editors / IDEs
BoxMakeWriter::new(std::io::stderr)
};

Expand All @@ -165,21 +171,26 @@ pub(crate) fn init_logging(

let subscriber = tracing_subscriber::Registry::default().with(layer);

if !is_test_client(client_info) {
if is_test_client(client_info) {
// During parallel testing, `set_global_default()` gets called multiple times
// per process. That causes it to error, but we ignore this.
tracing::subscriber::set_global_default(subscriber).ok();
} else {
tracing::subscriber::set_global_default(subscriber)
.expect("Should be able to set the global subscriber.");
.expect("Should be able to set the global subscriber exactly once.");
}

tracing::info!("Logging initialized with level: {log_level}");
}

/// We never log during tests as tests run in parallel within a single process,
/// but you can only have 1 global subscriber per process.
/// We use a special `TestWriter` during tests to be compatible with `cargo test`'s
/// typical output capturing behavior.
///
/// If you are debugging a single test, you can override this to emit messages to stderr.
/// Important notes:
/// - `cargo test` swallows all logs unless you use `-- --nocapture`.
/// - Tests run in parallel, so logs can be interleaved unless you run `--test-threads 1`.
///
/// Note that if you override this and run multiple tests in parallel, then the call
/// to `set_global_default()` will error causing a panic.
/// We use `cargo test -- --nocapture --test-threads 1` on CI because of all of this.
fn is_test_client(client_info: Option<&ClientInfo>) -> bool {
client_info.map_or(false, |client_info| client_info.name == "AirTestClient")
}
Expand Down Expand Up @@ -253,7 +264,7 @@ impl<S> tracing_subscriber::layer::Filter<S> for LogLevelFilter {
let filter = if meta.target().starts_with("air") || meta.target().starts_with("lsp") {
self.filter.tracing_level()
} else {
tracing::Level::INFO
tracing::Level::WARN
};

meta.level() <= &filter
Expand Down
13 changes: 9 additions & 4 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@ gen-formatter:
gen-grammar:
cargo run -p xtask_codegen -- grammar r

# Run the parser and formatter tests
# Run the tests
test:
cargo test -p air_r_parser
cargo test -p air_r_formatter
cargo test

# Run the tests in verbose mode
# `--nocapture` to see our own `tracing` logs
# `--test-threads 1` to ensure `tracing` logs aren't interleaved
test-verbose:
cargo test -- --nocapture --test-threads 1

# Run the quick formatter test
quick:
test-quick:
cargo test --package air_r_formatter --test quick_test -- quick_test --exact --show-output --ignored

# Creates a new crate
Expand Down

0 comments on commit 05c3dc3

Please sign in to comment.