-
Notifications
You must be signed in to change notification settings - Fork 11
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
Use the new async logging macros, fix various log messages #176
Conversation
Just a heads-up that mozilla/cubeb-rs#70 has been merged with the version updated to 0.10.1, so this PR should build once the version numbers here have been updated. Please let me know if you run into any issues, thanks! |
a67cd23
to
8733f1d
Compare
src/backend/tests/interfaces.rs
Outdated
@@ -566,6 +566,7 @@ fn test_ops_stream_set_volume() { | |||
} | |||
|
|||
#[test] | |||
#[ignore] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the error without #[ignore]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cubeb_stream_get_current_device
fails.
thread 'backend::tests::interfaces::test_ops_stream_current_device' panicked at 'assertion failed: `(left == right)`
left: `-1`,
right: `0`', src/backend/tests/interfaces.rs:578:9
stack backtrace:
0: rust_begin_unwind
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panicking.rs:584:5
1: core::panicking::panic_fmt
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/panicking.rs:142:14
2: core::panicking::assert_failed_inner
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/panicking.rs:224:17
3: core::panicking::assert_failed
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/panicking.rs:181:5
4: cubeb_coreaudio::backend::tests::interfaces::test_ops_stream_current_device::{{closure}}
at ./src/backend/tests/interfaces.rs:578:9
5: cubeb_coreaudio::backend::tests::utils::test_ops_stream_operation::{{closure}}
test backend::tests::interfaces::test_ops_stream_current_device ... FAILED
at ./src/backend/tests/utils.rs:1110:9
6: cubeb_coreaudio::backend::tests::utils::test_ops_context_operation
at ./src/backend/tests/utils.rs:1056:5
7: cubeb_coreaudio::backend::tests::utils::test_ops_stream_operation
at ./src/backend/tests/utils.rs:1077:5
8: cubeb_coreaudio::backend::tests::interfaces::test_default_output_stream_operation
at ./src/backend/tests/interfaces.rs:467:5
9: cubeb_coreaudio::backend::tests::interfaces::test_ops_stream_current_device
at ./src/backend/tests/interfaces.rs:570:5
10: cubeb_coreaudio::backend::tests::interfaces::test_ops_stream_current_device::{{closure}}
at ./src/backend/tests/interfaces.rs:569:1
11: core::ops::function::FnOnce::call_once
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/ops/function.rs:248:5
12: core::ops::function::FnOnce::call_once
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#177 should fix this
We got a compiler error with Rust Nightly. See rust-lang/rust#99563 |
rust-lang/rust#99893 shoudl fix this |
a868ff5
to
924f792
Compare
@ChunMinChang, I've updated this based on your fix and fixed a nightly-only clippy issue, please take another look! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
This won't build for now, we need to bump
cubeb-backend
's version here after merging mozilla/cubeb-rs#70, but should be an easy rebase. It works well locally when overriding.I took the opportunity to fix the phrasing of some logs while I was verifying what type they should be.