Conversation
7d70f5d to
12fb2a1
Compare
63d213b to
e24d803
Compare
12fb2a1 to
adbc477
Compare
e24d803 to
ceb1dce
Compare
ceb1dce to
afd3a8f
Compare
Wumpf
left a comment
There was a problem hiding this comment.
I wish we could instead have a "null context" on re_renderer, but that would be a lot more work I reckon. Not happy how we have to skip over so much code right now.
That said, approving it regardless since it's a huge improvement of what type of unit tests we can write 👍, TestContext and utility methods that we might add later to it will be invaluable
| let Some(render_ctx) = ctx.render_ctx else { | ||
| return Err(SpaceViewSystemExecutionError::NoRenderContextError); | ||
| }; |
There was a problem hiding this comment.
I don't think there's a better way that's not super tricky to implement right now, but this kind of early out here makes this approach a lot less viable for benchmarks. It would be great if we could still some logic of the visualizers, at least including the queries
There was a problem hiding this comment.
Agreed. In this case though, literally the next line needs a render_ctx, and it can't be moved further down... 🤷
| mod store_context; | ||
| pub mod store_hub; | ||
| mod tensor; | ||
| pub mod test_context; |
There was a problem hiding this comment.
It feels like it should, but then cargo clippy --all-features --all-targets fails. I'll leave a todo.
What
render_contextfield ofViewerContextanOptionto make it easier to crate aViewerContextin a test environment.ViewerContextfor use in unit test.SelectionPanel::show_panel()Chained to #6431
There are many improvements that could be added:
ViewportBlueprintre_log::warn/errassert: Add testability features tore_log#6450Checklist
mainbuild: rerun.io/viewernightlybuild: rerun.io/viewerTo run all checks from
main, comment on the PR with@rerun-bot full-check.