Merge pull request #2 from rerun-io/impl-debug

Fix filter not being completely recursive

examples/basic_integration.rs

@@ -13,7 +13,7 @@ pub struct Harness<'a> {
     /// the ui component that should be tested (for egui that's just a closure).
     app: Box<dyn FnMut(&egui::Context) + 'a>,
     /// The kittest State
-    state: kittest::State,
+    pub state: kittest::State,
 }
 
 impl<'a> Harness<'a> {

examples/querying.rs

@@ -36,14 +36,19 @@ fn main() {
     let _check_me = harness.get(by().role(Role::CheckBox).name_contains("Check"));
 
     // You can also query children of a node
-    let _group = harness.get_by_name("My Group");
+    let group = harness.get_by_role_and_name(Role::Label, "My Group");
     // get_by_name won't panic here since we only find the button in the group
-    // TODO(lucas): Egui doesn't add node as children of their container right now
-    // group.get_by_name("Duplicate");
+    group.get_by_name("Duplicate");
 
-    // TODO(lucas): This should match once egui adds children to their containers
-    // let btn_in_parent = harness.get_all_by_name("Duplicate").next_back().unwrap();
-    // assert_eq!(btn_in_parent.parent_id().unwrap(), group.id());
+    let btn_in_parent = harness
+        .get_all_by_name("Duplicate")
+        .next_back()
+        .expect("No buttons found");
+    assert_eq!(
+        btn_in_parent.parent_id().expect("No parent id"),
+        group.id(),
+        "Button is not in the group"
+    );
 
     // query_by and get_by functions will panic if more than one node is found
     // harness.get_by_role(Role::Button); // This will panic!
@@ -66,7 +71,12 @@ fn make_tree() -> Harness<'static> {
             let group_label = ui.label("My Group");
             _ = ui
                 .group(|ui| {
-                    _ = ui.button("Duplicate");
+                    // TODO(lucasmerlin): Egui should probably group widgets by their parent automatically
+                    ui.ctx()
+                        .clone()
+                        .with_accessibility_parent(group_label.id, || {
+                            _ = ui.button("Duplicate");
+                        });
                 })
                 .response
                 .labelled_by(group_label.id);

src/filter.rs

@@ -19,6 +19,7 @@ pub struct By<'a> {
     had_predicate: bool,
     role: Option<Role>,
     value: Option<&'a str>,
+    pub(crate) recursive: bool,
 }
 
 impl<'a> Debug for By<'a> {
@@ -31,6 +32,7 @@ impl<'a> Debug for By<'a> {
             had_predicate,
             role,
             value,
+            recursive,
         } = self;
         let mut s = f.debug_struct("By");
         if let Some(name) = name {
@@ -52,6 +54,9 @@ impl<'a> Debug for By<'a> {
         if let Some(value) = value {
             s.field("value", &value);
         }
+        if !*recursive {
+            s.field("recursive", recursive);
+        }
         s.finish()
     }
 }
@@ -73,6 +78,7 @@ impl<'a> By<'a> {
             had_predicate: false,
             role: None,
             value: None,
+            recursive: true,
         }
     }
 
@@ -115,6 +121,13 @@ impl<'a> By<'a> {
         self
     }
 
+    /// Should we search recursively?
+    /// Default is true.
+    pub fn recursive(mut self, recursive: bool) -> Self {
+        self.recursive = recursive;
+        self
+    }
+
     /// Should the labels of labelled nodes be filtered?
     pub(crate) fn should_filter_labels(&self) -> bool {
         !self.include_labels && self.name.is_some()
@@ -133,12 +146,14 @@ impl<'a> By<'a> {
             had_predicate: self.had_predicate,
             role: self.role,
             value: self.value,
+            recursive: self.recursive,
         }
     }
 
     /// Returns true if the given node matches this filter.
     /// Note: For correct filtering if [`Self::include_labels`] is false, the tree should be
     /// filtered like in [`crate::Queryable::query_all`].
+    /// Note: Remember to check for recursive filtering
     pub(crate) fn matches(&self, node: &Node<'_>) -> bool {
         if let Some(name) = self.name {
             if let Some(node_name) = node.name() {

src/lib.rs

@@ -3,10 +3,10 @@ mod event;
 mod filter;
 mod node;
 mod query;
-mod tree;
+mod state;
 
 pub use event::*;
 pub use filter::*;
 pub use node::*;
 pub use query::*;
-pub use tree::*;
+pub use state::*;

src/node.rs

@@ -1,12 +1,13 @@
 use crate::event::{Event, SimulatedEvent};
 use crate::query::Queryable;
-use crate::tree::EventQueue;
-use crate::{ElementState, Key, MouseButton};
+use crate::state::EventQueue;
+use crate::{by, ElementState, Key, MouseButton};
 use accesskit::{ActionRequest, Vec2};
 use std::fmt::{Debug, Formatter};
 use std::ops::Deref;
 
 /// A node in the accessibility tree. This should correspond to a widget or container in the GUI
+#[derive(Copy, Clone)]
 pub struct Node<'tree> {
     node: accesskit_consumer::Node<'tree>,
     pub(crate) queue: &'tree EventQueue,
@@ -32,6 +33,13 @@ impl<'a> Debug for Node<'a> {
         if let Some(toggled) = self.node.toggled() {
             s.field("toggled", &toggled);
         }
+
+        let children = self
+            .query_all(by().recursive(false))
+            .collect::<Vec<Node<'_>>>();
+
+        s.field("children", &children);
+
         s.finish()
     }
 }

src/query.rs

@@ -1,26 +1,41 @@
 use crate::filter::By;
 use crate::Node;
-use accesskit_consumer::FilterResult;
 use std::collections::BTreeSet;
-use std::iter::FusedIterator;
+use std::iter::{once, FusedIterator};
+
+fn children_recursive(node: Node<'_>) -> Box<dyn Iterator<Item = Node<'_>> + '_> {
+    let queue = node.queue();
+    Box::new(node.children().flat_map(move |node| {
+        let node = Node::new(node, queue);
+        once(node).chain(children_recursive(node))
+    }))
+}
+
+fn children(node: Node<'_>) -> impl Iterator<Item = Node<'_>> + '_ {
+    let queue = node.queue();
+    node.children().map(move |node| Node::new(node, queue))
+}
+
+fn children_maybe_recursive(
+    node: Node<'_>,
+    recursive: bool,
+) -> Box<dyn Iterator<Item = Node<'_>> + '_> {
+    if recursive {
+        children_recursive(node)
+    } else {
+        Box::new(children(node))
+    }
+}
 
 #[allow(clippy::needless_pass_by_value)]
+#[track_caller]
 fn query_all<'tree>(
     node: Node<'tree>,
     by: By<'tree>,
 ) -> impl DoubleEndedIterator<Item = Node<'tree>> + FusedIterator<Item = Node<'tree>> + 'tree {
     let should_filter_labels = by.should_filter_labels();
 
-    let queue = node.queue();
-    let results = node
-        .filtered_children(move |node| {
-            if by.matches(&Node::new(*node, queue)) {
-                FilterResult::Include
-            } else {
-                FilterResult::ExcludeNode
-            }
-        })
-        .map(|node| Node::new(node, queue));
+    let results = children_maybe_recursive(node, by.recursive).filter(move |node| by.matches(node));
 
     let nodes = results.collect::<Vec<_>>();
 
@@ -48,6 +63,7 @@ fn query_all<'tree>(
 }
 
 #[allow(clippy::needless_pass_by_value)]
+#[track_caller]
 fn get_all<'tree>(
     node: Node<'tree>,
     by: By<'tree>,
@@ -56,12 +72,13 @@ fn get_all<'tree>(
     let mut iter = query_all(node, by).peekable();
     assert!(
         iter.peek().is_some(),
-        "No nodes found matching the query: {debug_query:?}"
+        "No nodes found matching the query:\n{debug_query:#?}\n\nOn node:\n{node:#?}"
     );
     iter
 }
 
 #[allow(clippy::needless_pass_by_value)]
+#[track_caller]
 fn query<'tree>(node: Node<'tree>, by: By<'tree>) -> Option<Node<'tree>> {
     let debug_query = by.debug_clone_without_predicate();
     let mut iter = query_all(node, by);
@@ -70,21 +87,22 @@ fn query<'tree>(node: Node<'tree>, by: By<'tree>) -> Option<Node<'tree>> {
     if let Some(second) = iter.next() {
         let first = result?;
         panic!(
-            "Found two or more nodes matching the query: \n{debug_query:?}\n\nFirst node: {first:?}\nSecond node: {second:?}\
+            "Found two or more nodes matching the query: \n{debug_query:#?}\n\nFirst node:\n{first:#?}\n\nSecond node: {second:#?}\
                 \n\nIf you were expecting multiple nodes, use query_all instead of query."
         );
     }
     result
 }
 
 #[allow(clippy::needless_pass_by_value)]
+#[track_caller]
 fn get<'tree>(node: Node<'tree>, by: By<'tree>) -> Node<'tree> {
     let debug_query = by.debug_clone_without_predicate();
     let option = query(node, by);
     if let Some(node) = option {
         node
     } else {
-        panic!("No nodes found matching the query: {debug_query:?}");
+        panic!("No nodes found matching the query:\n{debug_query:#?}\n\nOn node:\n{node:#?}");
     }
 }
 

src/tree.rs → src/state.rs

@@ -3,13 +3,22 @@ use crate::query::Queryable;
 use crate::Node;
 use accesskit::TreeUpdate;
 use parking_lot::Mutex;
+use std::fmt::{Debug, Formatter};
 
 /// The kittest state
 pub struct State {
     tree: accesskit_consumer::Tree,
     queued_events: Mutex<Vec<Event>>,
 }
 
+impl Debug for State {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("State")
+            .field("tree", &self.node())
+            .finish_non_exhaustive()
+    }
+}
+
 pub(crate) type EventQueue = Mutex<Vec<Event>>;
 
 impl State {