-
Notifications
You must be signed in to change notification settings - Fork 538
Kittest utility toolbelt #11435
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
base: main
Are you sure you want to change the base?
Kittest utility toolbelt #11435
Conversation
Web viewer built successfully.
Note: This comment is updated whenever you push a commit. |
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.
Really awesome ⭐
|
||
use crate::App; | ||
|
||
#[cfg(feature = "testing")] |
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.
Alternatively, put #![cfg(feature = "testing")]
at the top of the file!
// Prints the current viewer state. Don't merge code that calls this. | ||
#[allow(unused)] | ||
fn debug_viewer_state(&mut self); |
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.
Why not merge code that calls it? Is it very slow?
If it's called from a test, stdout/stderr only reaches the terminal output if the tests fails.
fn toggle_blueprint_panel(&mut self) { | ||
self.click_label("Blueprint panel toggle"); | ||
} | ||
|
||
fn toggle_time_panel(&mut self) { | ||
self.click_label("Time panel toggle"); | ||
} | ||
|
||
fn toggle_selection_panel(&mut self) { | ||
self.click_label("Selection panel toggle"); | ||
} |
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.
It would be a lot nicer to have idempotent commands here, i.e. fn show_blueprint_panel(&mut self, open: bool)
fn click_label(&mut self, label: &str) { | ||
self.get_by_label(label).click(); | ||
self.run_ok(); | ||
} | ||
|
||
fn right_click_label(&mut self, label: &str) { | ||
self.get_by_label(label).click_secondary(); | ||
self.run_ok(); | ||
} |
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.
I appreciate having these helpers to not having to call .run_ok()
after each action… but I wish we had this in kittest instrad. kittest
allows user to buffer up many inputs (e.g. push down some modifiers before clicking), which is powerful, but the price is all these manual calls to .run()
. And realistically, you almost shouldn't test clicking two things in the same frame, since realistically that shouldn't happen in real usage.
In other words: I believe kittest's API is a bit too low-level for most tests.
I don't think there is anything actionable here, except food for thought, and ping @lucasmerlin
// TODO(aedm): there is a nondeterministic font rendering issue. | ||
self.snapshot_options( | ||
snapshot_name, | ||
&SnapshotOptions::new().failed_pixel_count_threshold(0), | ||
); |
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.
Is there an open issue about this we can link to?
Did you investigate doing the manual texture filtering in egui-wgpu?
pub async fn test_single_text_document() { | ||
let mut harness = viewer_test_utils::viewer_harness(); | ||
harness.init_recording(); | ||
harness.toggle_selection_panel(); |
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.
See this is the problem with non-idempotency. Is this opening or closing the selection panel? 🤷
#[tokio::test(flavor = "multi_thread")] | ||
pub async fn test_single_text_document() { | ||
let mut harness = viewer_test_utils::viewer_harness(); | ||
harness.init_recording(); | ||
harness.toggle_selection_panel(); | ||
harness.snapshot("single_text_document_1"); | ||
|
||
// Log some data | ||
harness.log_entity("txt/hello", |builder| { | ||
builder.with_archetype( | ||
RowId::new(), | ||
TimePoint::STATIC, | ||
&re_types::archetypes::TextDocument::new("Hello World!"), | ||
) | ||
}); | ||
|
||
// Set up the viewport blueprint | ||
harness.clear_current_blueprint(); | ||
harness.setup_viewport_blueprint(|_viewer_context, blueprint| { | ||
blueprint.add_view_at_root(ViewBlueprint::new_with_root_wildcard( | ||
TextDocumentView::identifier(), | ||
)); | ||
}); | ||
|
||
harness.snapshot("single_text_document_2"); |
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.
This is so cool
#[tokio::test(flavor = "multi_thread")] | ||
pub async fn test_single_text_document() { |
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.
Why do we need tokio here at all?
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.
Looks like the toggle button is still being hovered?
I think we should try to avoid that.
There should be some way to remove the mouse cursor after a click. Maybe kittest should even do so automatically?
Ping @lucasmerlin
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.
It feels like these are maybe unnecessarily large. What sets their default size?
Related
What