Skip to content

Fix panic on RUST_LOG=<filter> during tests #1154

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

Closed
wants to merge 10 commits into from

Conversation

Cardosaum
Copy link
Contributor

I'm opening this PR to address a bug report encountered when running a test with RUST_LOG=info. The bug causes a panic due to a regex parse error related to Unicode-aware case insensitivity matching - exactly like the issue described in tracing-subscriber. I have tried to fix it by specifying test-log feature to both log and trace in the Cargo.toml file.

I can't validate the fix on my machine yet, since this seems to arise only when -F cuda is passed, and I don't have the required hardware to test it.

Copy link

vercel bot commented Nov 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 29, 2023 4:06pm

@flaub
Copy link
Member

flaub commented Nov 16, 2023

It doesn't seem like this is related to -F cuda, but I guess I could be wrong.

@Cardosaum Cardosaum changed the title Update dev-dependencies test-log in Cargo.toml Fix panic on RUST_LOG=<filter> during tests Nov 16, 2023
@Cardosaum Cardosaum self-assigned this Nov 16, 2023
@Cardosaum Cardosaum added the bug Something isn't working label Nov 16, 2023
@flaub flaub added this to the 0.20.0 milestone Nov 16, 2023
@Cardosaum Cardosaum requested review from flaub and SchmErik and removed request for flaub November 21, 2023 14:16
@@ -14,7 +14,8 @@ tracing = { version = "0.1", features = ["log"] }
tracing-subscriber = { version = "0.3", features = ["env-filter"] }

[dev-dependencies]
test-log = { version = "0.2", default-features = false, features = ["trace"] }
env_logger = "0.10"
test-log = { version = "0.2", features = ["trace", "log"] }
Copy link
Member

Choose a reason for hiding this comment

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

The default already contains log

Suggested change
test-log = { version = "0.2", features = ["trace", "log"] }
test-log = { version = "0.2", features = ["trace"] }

@@ -8,8 +8,9 @@ homepage = { workspace = true }
repository = { workspace = true }

[dev-dependencies]
env_logger = "0.10"
Copy link
Contributor

Choose a reason for hiding this comment

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

why is both env_logger and tracing subscriber a part of this? I thought that we only needed tracing and tracing-subscriber

@flaub
Copy link
Member

flaub commented Nov 30, 2023

I found the underlying issue that some of us have run into, it's likely because we were using an older version of tracing-subscriber and we don't have all our lock files checked in.

tokio-rs/tracing#2565

@flaub flaub closed this Dec 1, 2023
@flaub flaub deleted the cardosaum/hotfix/test-log-features branch December 1, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants