Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8883,7 +8883,9 @@ dependencies = [
"re_sdk",
"re_server",
"re_uri",
"re_view_text_document",
"re_viewer",
"re_viewport_blueprint",
"tempfile",
"tokio",
]
Expand Down
18 changes: 18 additions & 0 deletions crates/viewer/re_viewer/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ use crate::{

const WATERMARK: bool = false; // Nice for recording media material

#[cfg(feature = "testing")]
pub type TestHookFn = Box<dyn FnOnce(&ViewerContext<'_>)>;

#[derive(serde::Deserialize, serde::Serialize)]
#[serde(default)]
pub struct AppState {
Expand Down Expand Up @@ -65,6 +68,12 @@ pub struct AppState {
#[serde(skip)]
pub(crate) share_modal: crate::ui::ShareModal,

/// Test-only: single-shot callback to run at the end of the frame. Used in integration tests
/// to interact with the `ViewerContext`.
#[cfg(feature = "testing")]
#[serde(skip)]
pub(crate) test_hook: Option<TestHookFn>,

/// A stack of display modes that represents tab-like navigation of the user.
#[serde(skip)]
pub(crate) navigation: Navigation,
Expand Down Expand Up @@ -114,6 +123,9 @@ impl Default for AppState {
view_states: Default::default(),
selection_state: Default::default(),
focused_item: Default::default(),

#[cfg(feature = "testing")]
test_hook: None,
}
}
}
Expand Down Expand Up @@ -678,6 +690,12 @@ impl AppState {
self.open_url_modal.ui(ui);
self.share_modal
.ui(&ctx, ui, startup_options.web_viewer_base_url().as_ref());

// Only in integration tests: call the test hook if any.
#[cfg(feature = "testing")]
if let Some(test_hook) = self.test_hook.take() {
test_hook(&ctx);
}
}
}

Expand Down
22 changes: 22 additions & 0 deletions crates/viewer/re_viewer/src/viewer_test_utils/app_testing_ext.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use re_viewer_context::StoreHub;

use crate::App;

#[cfg(feature = "testing")]
Copy link
Member

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!

pub trait AppTestingExt {
fn testonly_get_store_hub(&mut self) -> &mut StoreHub;
fn testonly_set_test_hook(&mut self, func: crate::app_state::TestHookFn);
}

#[cfg(feature = "testing")]
impl AppTestingExt for App {
fn testonly_get_store_hub(&mut self) -> &mut StoreHub {
self.store_hub
.as_mut()
.expect("store_hub should be initialized")
}

fn testonly_set_test_hook(&mut self, func: crate::app_state::TestHookFn) {
self.state.test_hook = Some(func);
}
}
4 changes: 4 additions & 0 deletions crates/viewer/re_viewer/src/viewer_test_utils/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
mod app_testing_ext;

#[cfg(feature = "testing")]
pub use app_testing_ext::AppTestingExt;
use egui_kittest::Harness;
use re_build_info::build_info;

Expand Down
8 changes: 5 additions & 3 deletions tests/rust/re_integration_test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,20 @@ version.workspace = true
publish = false

[dependencies]
egui_kittest.workspace = true
egui.workspace = true
re_redap_client.workspace = true
re_protos.workspace = true
re_sdk.workspace = true
re_server.workspace = true
re_uri.workspace = true
re_viewer = { workspace = true, features = ["testing"] }
re_viewport_blueprint.workspace = true
tempfile.workspace = true
tokio = { workspace = true, features = ["rt-multi-thread", "macros"] }

[dev-dependencies]
re_viewer = { workspace = true, features = ["testing"] }
egui_kittest.workspace = true
egui.workspace = true
re_view_text_document.workspace = true

[lints]
workspace = true
232 changes: 232 additions & 0 deletions tests/rust/re_integration_test/src/kittest_harness_ext.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
use std::sync::Arc;

use egui_kittest::{SnapshotOptions, kittest::Queryable as _};
use re_sdk::{
Component as _, ComponentDescriptor, EntityPath, EntityPathPart, RecordingInfo, StoreId,
StoreKind,
external::{
re_log_types::{SetStoreInfo, StoreInfo},
re_tuid::Tuid,
},
log::Chunk,
};
use re_viewer::{
SystemCommand, SystemCommandSender as _,
external::{
re_chunk::{ChunkBuilder, LatestAtQuery},
re_entity_db::EntityDb,
re_types,
re_viewer_context::{self, ViewerContext, blueprint_timeline},
},
viewer_test_utils::AppTestingExt as _,
};
use re_viewport_blueprint::ViewportBlueprint;

// Kittest harness utilities specific to the Rerun app.
pub trait HarnessExt {
// Initializes the chuck store with a new, empty recording and blueprint.
fn init_recording(&mut self);

// Runs a function with the `ViewerContext` generated by the actual Rerun application.
fn run_with_viewer_context(&mut self, func: impl FnOnce(&ViewerContext<'_>) + 'static);

// Removes all views and containers from the current blueprint.
fn clear_current_blueprint(&mut self);

// Sets up a new viewport blueprint and saves the new one in the chunk store.
fn setup_viewport_blueprint(
&mut self,
setup_blueprint: impl FnOnce(&ViewerContext<'_>, &mut ViewportBlueprint) + 'static,
);

// Logs an entity to the active recording.
fn log_entity(
&mut self,
entity_path: impl Into<EntityPath>,
build_chunk: impl FnOnce(ChunkBuilder) -> ChunkBuilder,
);

// Clicks a node in the UI by its label.
fn click_label(&mut self, label: &str);
fn right_click_label(&mut self, label: &str);

// Takes a snapshot of the current app state with good-enough snapshot options.
fn snapshot_app(&mut self, snapshot_name: &str);

// Prints the current viewer state. Don't merge code that calls this.
#[allow(unused)]
fn debug_viewer_state(&mut self);
Comment on lines +56 to +58
Copy link
Member

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");
}
Comment on lines +60 to +70
Copy link
Member

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)

}

impl HarnessExt for egui_kittest::Harness<'_, re_viewer::App> {
fn clear_current_blueprint(&mut self) {
self.setup_viewport_blueprint(|_viewer_context, blueprint| {
for item in blueprint.contents_iter() {
blueprint.remove_contents(item);
}
});
self.run_ok();
}

fn setup_viewport_blueprint(
&mut self,
setup_blueprint: impl FnOnce(&ViewerContext<'_>, &mut ViewportBlueprint) + 'static,
) {
self.run_with_viewer_context(|viewer_context| {
let blueprint_query = LatestAtQuery::latest(blueprint_timeline());
let mut viewport_blueprint =
ViewportBlueprint::from_db(viewer_context.blueprint_db(), &blueprint_query);
setup_blueprint(viewer_context, &mut viewport_blueprint);
viewport_blueprint.save_to_blueprint_store(viewer_context);
});
self.run_ok();
}

fn run_with_viewer_context(&mut self, func: impl FnOnce(&ViewerContext<'_>) + 'static) {
self.state_mut().testonly_set_test_hook(Box::new(func));
self.run_ok();
}

fn log_entity(
&mut self,
entity_path: impl Into<EntityPath>,
build_chunk: impl FnOnce(ChunkBuilder) -> ChunkBuilder,
) {
let app = self.state_mut();
let builder = build_chunk(Chunk::builder(entity_path));
let store_hub = app.testonly_get_store_hub();
let active_recording = store_hub
.active_recording_mut()
.expect("active_recording should be initialized");
active_recording
.add_chunk(&Arc::new(
builder.build().expect("chunk should be successfully built"),
))
.expect("chunk should be successfully added");
self.run_ok();
}

fn init_recording(&mut self) {
let app = self.state_mut();
let store_hub = app.testonly_get_store_hub();

let store_info = StoreInfo::testing();
let application_id = store_info.application_id().clone();
let recording_store_id = store_info.store_id.clone();
let mut recording_store = EntityDb::new(recording_store_id.clone());

recording_store.set_store_info(SetStoreInfo {
row_id: Tuid::new(),
info: store_info,
});
{
// Set RecordingInfo:
recording_store
.set_recording_property(
EntityPath::properties(),
RecordingInfo::descriptor_name(),
&re_types::components::Name::from("Test recording"),
)
.expect("Failed to set recording name");
recording_store
.set_recording_property(
EntityPath::properties(),
RecordingInfo::descriptor_start_time(),
&re_types::components::Timestamp::now(),
)
.expect("Failed to set recording start time");
}
{
// Set some custom recording properties:
recording_store
.set_recording_property(
EntityPath::properties() / EntityPathPart::from("episode"),
ComponentDescriptor {
archetype: None,
component: "location".into(),
component_type: Some(re_types::components::Text::name()),
},
&re_types::components::Text::from("Swallow Falls"),
)
.expect("Failed to set recording property");
recording_store
.set_recording_property(
EntityPath::properties() / EntityPathPart::from("episode"),
ComponentDescriptor {
archetype: None,
component: "weather".into(),
component_type: Some(re_types::components::Text::name()),
},
&re_types::components::Text::from("Cloudy with meatballs"),
)
.expect("Failed to set recording property");
}

let blueprint_id = StoreId::random(StoreKind::Blueprint, application_id);
let blueprint_store = EntityDb::new(blueprint_id.clone());

store_hub.insert_entity_db(recording_store);
store_hub.insert_entity_db(blueprint_store);
store_hub.set_active_recording_id(recording_store_id.clone());
store_hub
.set_cloned_blueprint_active_for_app(&blueprint_id)
.expect("Failed to set blueprint as active");

app.command_sender.send_system(SystemCommand::SetSelection(
re_viewer_context::Item::StoreId(recording_store_id.clone()).into(),
));
self.run_ok();
}

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();
}
Comment on lines +193 to +201
Copy link
Member

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


fn debug_viewer_state(&mut self) {
println!(
"Active recording: {:#?}",
self.state_mut().testonly_get_store_hub().active_recording()
);
println!(
"Active blueprint: {:#?}",
self.state_mut().testonly_get_store_hub().active_blueprint()
);
self.setup_viewport_blueprint(|_viewer_context, blueprint| {
println!("Blueprint view count: {}", blueprint.views.len());
for id in blueprint.view_ids() {
println!("View id: {id}");
}
println!(
"Display mode: {:?}",
_viewer_context.global_context.display_mode
);
});
}

fn snapshot_app(&mut self, snapshot_name: &str) {
self.run_ok();
// TODO(aedm): there is a nondeterministic font rendering issue.
self.snapshot_options(
snapshot_name,
&SnapshotOptions::new().failed_pixel_count_threshold(0),
);
Comment on lines +226 to +230
Copy link
Member

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?

}
}
2 changes: 2 additions & 0 deletions tests/rust/re_integration_test/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
//! Integration tests for rerun and the in memory server.

mod kittest_harness_ext;
mod test_data;

pub use kittest_harness_ext::HarnessExt;
use re_redap_client::{ClientConnectionError, ConnectionClient, ConnectionRegistry};
use re_server::ServerHandle;
use re_uri::external::url::Host;
Expand Down
35 changes: 35 additions & 0 deletions tests/rust/re_integration_test/tests/basic_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use re_integration_test::HarnessExt as _;
use re_sdk::TimePoint;
use re_sdk::log::RowId;
use re_view_text_document::TextDocumentView;
use re_viewer::external::re_types;
use re_viewer::external::re_viewer_context::ViewClass as _;
use re_viewer::viewer_test_utils;
use re_viewport_blueprint::ViewBlueprint;

#[tokio::test(flavor = "multi_thread")]
pub async fn test_single_text_document() {
Comment on lines +10 to +11
Copy link
Member

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?

let mut harness = viewer_test_utils::viewer_harness();
harness.init_recording();
harness.toggle_selection_panel();
Copy link
Member

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? 🤷

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");
Comment on lines +10 to +34
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so cool

}
Loading
Loading