Allow capturing logging endpoint messages #397
Conversation
| #[derive(Clone)] | ||
| pub struct LogEndpoint { | ||
| name: Vec<u8>, | ||
| writer: Arc<Mutex<dyn Write + Send>>, |
There was a problem hiding this comment.
Since this escapes out now, would it be worth adding a synonym for Arc<Mutex<dyn Write + Send>> for readability elsewhere?
There was a problem hiding this comment.
Good question. While trying to decide how to answer it, I've stared at the places this is visible, and I think my answer is that it shouldn't actually be visible in most of those places. In particular, I don't think either the LogEndpoint or Session types should be exported. The only place library users should encounter this type is in the ExecuteCtx::with_capture_logs method, and I don't think a type alias is necessary for that.
It's a little annoying naming this type multiple places within the library, but I think it's a little better to keep the details visible so that, for example, it's clear that using clone is cheap.
So I guess I'm not inclined to introduce a type alias for this.
Some integration tests need to capture log output in order to verify that logging endpoints work correctly from guests, but it's also potentially useful for people using Viceroy as a library. The previous way that the integration tests did this was not reliable when multiple tests ran in parallel, because there was only one global hook for capturing logs. Exposing this as a configuration option on execution contexts instead allows each test to set independent capture buffers. I guess this should be considered a breaking API change since `viceroy_lib::logging::LOG_WRITER` was exported publicly from the crate.
Some integration tests need to capture log output in order to verify that logging endpoints work correctly from guests, but it's also potentially useful for people using Viceroy as a library. The previous way that the integration tests did this was not reliable when multiple tests ran in parallel, because there was only one global hook for capturing logs. Exposing this as a configuration option on execution contexts instead allows each test to set independent capture buffers. I guess this should be considered a breaking API change since `viceroy_lib::logging::LOG_WRITER` was exported publicly from the crate.
Some integration tests need to capture log output in order to verify that logging endpoints work correctly from guests, but it's also potentially useful for people using Viceroy as a library.
The previous way that the integration tests did this was not reliable when multiple tests ran in parallel, because there was only one global hook for capturing logs. Exposing this as a configuration option on execution contexts instead allows each test to set independent capture buffers.
I guess this should be considered a breaking API change since
viceroy_lib::logging::LOG_WRITERwas exported publicly from the crate.