Findings
-
File logging is effectively disabled
src/logging.rs creates the tracing_appender::non_blocking guard as _guard, but drops it when setup_tracing() returns. The guard must stay alive for logs to be written/flushed. I reproduced this with BENCH_LOG_FILE=/tmp/impalab-review.log RUST_LOG=info ...; the command succeeded, but the log file was 0 bytes.
Fix: keep the guard for process lifetime, e.g. return it from setup and hold it in main, or store it in a OnceLock.
-
Missing explicit --config files are silently ignored
src/config.rs reads the config through FileReader, whose real implementation returns Ok(None) on NotFound at src/cli.rs. That is reasonable for optional manifest-style reads, but not for a user-supplied --config missing.json. I reproduced impa run --config missing.json exiting 0 with no stdout/stderr.
Fix: for explicit config paths, use a strict read or convert None into a config-read error.
Verification
cargo test passes: 33 tests total.
cargo clippy --all-targets -- -D warnings passes.
Findings
File logging is effectively disabled
src/logging.rs creates the
tracing_appender::non_blockingguard as_guard, but drops it whensetup_tracing()returns. The guard must stay alive for logs to be written/flushed. I reproduced this withBENCH_LOG_FILE=/tmp/impalab-review.log RUST_LOG=info ...; the command succeeded, but the log file was0bytes.Fix: keep the guard for process lifetime, e.g. return it from setup and hold it in
main, or store it in aOnceLock.Missing explicit
--configfiles are silently ignoredsrc/config.rs reads the config through
FileReader, whose real implementation returnsOk(None)onNotFoundat src/cli.rs. That is reasonable for optional manifest-style reads, but not for a user-supplied--config missing.json. I reproducedimpa run --config missing.jsonexiting0with no stdout/stderr.Fix: for explicit config paths, use a strict read or convert
Noneinto a config-read error.Verification
cargo testpasses: 33 tests total.cargo clippy --all-targets -- -D warningspasses.