Skip to content

Commit

Permalink
API examples overhaul & roundtrip tests (#3204)
Browse files Browse the repository at this point in the history
**Commit by commit**

This PR overhauls API examples to make them roundtrippable and checks
those roundtrips on CI.

These roundtrips serve two purposes: A) the obvious one: they make sure
that using our different SDKs in similar ways actually yield similar
data, but more importantly B) they act as regression tests since it is
highly unlikely that two or more tests written in different languages
fail in the exact same way at the exact same time.

As I've painfully found out during the `MsgSender` migration, _a lot_ of
our tests had bitrotten and simply did not work in one way or another
anymore (in fact I ended up fixing yet another batch for this PR).
This PR should provide the foundations so that this doesn't happen
again.

A nice side-effect of this is the introduction of the
`_RERUN_TEST_FORCE_SAVE` environment variable, which forces all
`RecordingStream`s instantiated across all 3 languages to write to an
rrd file on disk rather than do whatever they were asked to do.
This makes testing things _much_ easier.
  • Loading branch information
teh-cmc authored Sep 6, 2023
1 parent cb197a2 commit 8b428ce
Show file tree
Hide file tree
Showing 73 changed files with 818 additions and 409 deletions.
5 changes: 3 additions & 2 deletions .github/workflows/reusable_build_and_test_wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,9 @@ jobs:
# --release so we can inherit from some of the artifacts that maturin has just built before
# --target x86_64-unknown-linux-gnu because otherwise cargo loses the target cache... even though this is the target anyhow...
# --no-py-build because rerun-sdk is already built and installed
# NOTE: run with --release so we can reuse some of the "Build Wheel"'s job artifacts, hopefully
run: RUST_LOG=debug scripts/ci/run_e2e_roundtrip_tests.py --release --target x86_64-unknown-linux-gnu --no-py-build
run: |
RUST_LOG=debug tests/roundtrips.py --release --target x86_64-unknown-linux-gnu --no-py-build
RUST_LOG=debug docs/code-examples/roundtrips.py --release --target x86_64-unknown-linux-gnu --no-py-build
- name: Cache RRD dataset
if: needs.set-config.outputs.RUN_TESTS == 'true'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/reusable_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -337,4 +337,4 @@ jobs:

- name: Build code examples
shell: bash
run: ./tests/cpp/build_all_doc_examples.sh --werror
run: ./docs/code-examples/build_all.sh --werror
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ screenshot*.png
web_demo

.nox/
out.rrd
*.rrd
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,4 @@ FetchContent_MakeAvailable(LoguruGitRepo) # defines target 'loguru::loguru'
add_subdirectory(rerun_cpp) # The Rerun C++ SDK library
add_subdirectory(examples/cpp/minimal)
add_subdirectory(tests/cpp)
add_subdirectory(docs/code-examples)
4 changes: 2 additions & 2 deletions crates/re_log_types/src/data_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1145,8 +1145,8 @@ impl DataTable {
c1.datatype() == c2.datatype(),
"Found discrepancy in row #{ri}: cells' datatypes don't match!\n{}",
similar_asserts::SimpleDiff::from_str(
&format!("{:?}", c1.datatype()),
&format!("{:?}", c2.datatype()),
&format!("{:?}:{:?}", c1.component_name(), c1.datatype()),
&format!("{:?}:{:?}", c2.component_name(), c2.datatype()),
"cell1",
"cell2"
)
Expand Down
84 changes: 75 additions & 9 deletions crates/re_sdk/src/recording_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,25 @@ use crate::sink::{LogSink, MemorySinkStorage};

// ---

/// Private environment variable meant for tests.
///
/// When set, all recording streams will write to disk at the path indicated by the env-var rather
/// than doing what they were asked to do (`connect()`, `buffered()`, even `save()`..!).
const ENV_FORCE_SAVE: &str = "_RERUN_TEST_FORCE_SAVE";

/// If private environment variable `_RERUN_TEST_FORCE_SAVE` is set, this will return a `FileSink`
/// pointing at the path that the env-var is set to.
///
/// Useful to save the result of any Rerun snippet to an rrd file, whether it supports it or not.
fn forced_sink() -> RecordingStreamResult<Option<Box<dyn LogSink>>> {
Ok(if let Ok(path) = std::env::var(ENV_FORCE_SAVE) {
re_log::warn!(?path, "forcing FileSink");
Some(Box::new(crate::sink::FileSink::new(path)?) as Box<dyn LogSink>)
} else {
None
})
}

/// Errors that can occur when creating/manipulating a [`RecordingStream`].
#[derive(thiserror::Error, Debug)]
pub enum RecordingStreamError {
Expand Down Expand Up @@ -184,7 +203,11 @@ impl RecordingStreamBuilder {
RecordingStream::new(
store_info,
batcher_config,
Box::new(crate::log_sink::BufferedSink::new()),
if let Some(sink) = forced_sink()? {
sink
} else {
Box::new(crate::log_sink::BufferedSink::new())
},
)
} else {
re_log::debug!("Rerun disabled - call to buffered() ignored");
Expand Down Expand Up @@ -216,7 +239,18 @@ impl RecordingStreamBuilder {

let (enabled, store_info, batcher_config) = self.into_args();
if enabled {
RecordingStream::new(store_info, batcher_config, Box::new(sink)).map(|rec| {
RecordingStream::new(
store_info,
batcher_config,
// NOTE: We still have to return a `MemorySinkStorage` that will never get written
// to.. beware!
if let Some(sink) = forced_sink()? {
sink
} else {
Box::new(sink)
},
)
.map(|rec| {
storage.rec = Some(rec.clone());
(rec, storage)
})
Expand Down Expand Up @@ -250,7 +284,11 @@ impl RecordingStreamBuilder {
RecordingStream::new(
store_info,
batcher_config,
Box::new(crate::log_sink::TcpSink::new(addr, flush_timeout)),
if let Some(sink) = forced_sink()? {
sink
} else {
Box::new(crate::log_sink::TcpSink::new(addr, flush_timeout))
},
)
} else {
re_log::debug!("Rerun disabled - call to connect() ignored");
Expand Down Expand Up @@ -278,7 +316,11 @@ impl RecordingStreamBuilder {
RecordingStream::new(
store_info,
batcher_config,
Box::new(crate::sink::FileSink::new(path)?),
if let Some(sink) = forced_sink()? {
sink
} else {
Box::new(crate::sink::FileSink::new(path)?)
},
)
} else {
re_log::debug!("Rerun disabled - call to save() ignored");
Expand Down Expand Up @@ -324,8 +366,12 @@ impl RecordingStreamBuilder {
) -> RecordingStreamResult<RecordingStream> {
let (enabled, store_info, batcher_config) = self.into_args();
if enabled {
let sink = crate::web_viewer::new_sink(open_browser, bind_ip, web_port, ws_port)
.map_err(RecordingStreamError::WebSink)?;
let sink = if let Some(sink) = forced_sink()? {
sink
} else {
crate::web_viewer::new_sink(open_browser, bind_ip, web_port, ws_port)
.map_err(RecordingStreamError::WebSink)?
};
RecordingStream::new(store_info, batcher_config, sink)
} else {
re_log::debug!("Rerun disabled - call to serve() ignored");
Expand Down Expand Up @@ -536,6 +582,7 @@ impl RecordingStream {
batcher_config: DataTableBatcherConfig,
sink: Box<dyn LogSink>,
) -> RecordingStreamResult<Self> {
let sink = forced_sink()?.unwrap_or(sink);
RecordingStreamInner::new(info, batcher_config, sink).map(|inner| Self {
inner: Arc::new(Some(inner)),
})
Expand Down Expand Up @@ -1097,7 +1144,12 @@ impl RecordingStream {
/// terms of data durability and ordering.
/// See [`Self::set_sink`] for more information.
pub fn connect(&self, addr: std::net::SocketAddr, flush_timeout: Option<std::time::Duration>) {
self.set_sink(Box::new(crate::log_sink::TcpSink::new(addr, flush_timeout)));
// NOTE: `forced_sink` is only used for tests, it's ok to unwrap.
self.set_sink(if let Some(sink) = forced_sink().unwrap() {
sink
} else {
Box::new(crate::log_sink::TcpSink::new(addr, flush_timeout))
});
}

/// Swaps the underlying sink for a [`crate::sink::MemorySink`] sink and returns the associated
Expand All @@ -1109,7 +1161,14 @@ impl RecordingStream {
pub fn memory(&self) -> MemorySinkStorage {
let sink = crate::sink::MemorySink::default();
let buffer = sink.buffer();
self.set_sink(Box::new(sink));

// NOTE: `forced_sink` is only used for tests, it's ok to unwrap.
self.set_sink(if let Some(sink) = forced_sink().unwrap() {
sink
} else {
Box::new(sink)
});

buffer
}

Expand All @@ -1123,7 +1182,14 @@ impl RecordingStream {
path: impl Into<std::path::PathBuf>,
) -> Result<(), crate::sink::FileSinkError> {
let sink = crate::sink::FileSink::new(path)?;
self.set_sink(Box::new(sink));

// NOTE: `forced_sink` is only used for tests, it's ok to unwrap.
self.set_sink(if let Some(sink) = forced_sink().unwrap() {
sink
} else {
Box::new(sink)
});

Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion crates/re_types/definitions/rerun/archetypes/image.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace rerun.archetypes;
/// \py -------
/// \py
/// \py ```python
/// \py \include:../../../../../docs/code-examples/image_simple_v2.py
/// \py \include:../../../../../docs/code-examples/image_simple.py
/// \py ```
///
/// \rs ## Example
Expand Down
2 changes: 1 addition & 1 deletion crates/re_types/source_hash.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/re_types/src/archetypes/annotation_context.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions crates/re_types/src/archetypes/arrows3d.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/re_types/src/archetypes/image.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/re_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ pub use self::arrow_string::ArrowString;

pub mod external {
pub use arrow2;
pub use uuid;
}

// TODO(jleibs): Should all of this go into `tensor_data_ext`? Don't have a good way to export
Expand Down
24 changes: 24 additions & 0 deletions docs/code-examples/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
cmake_minimum_required(VERSION 3.16)

# Setup builds for examples
file(GLOB sources_list true ${CMAKE_CURRENT_SOURCE_DIR}/*.cpp)

add_custom_target(doc_examples)

foreach(SOURCE_PATH ${sources_list})
get_filename_component(SOURCE_NAME ${SOURCE_PATH} NAME_WLE)

if(${SOURCE_NAME} STREQUAL "CMakeFiles")
CONTINUE()
endif()

set(EXAMPLE_TARGET ${SOURCE_NAME})

add_executable(${EXAMPLE_TARGET} ${SOURCE_PATH})

set_default_warning_settings(rerun_example)
target_link_libraries(${EXAMPLE_TARGET} PRIVATE rerun_sdk)

add_dependencies(doc_examples ${EXAMPLE_TARGET})
endforeach()

37 changes: 37 additions & 0 deletions docs/code-examples/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# API Examples

These examples showcase common usage of each individual Rerun `Archetype`s.

Most of these examples are automatically used as docstrings for the `Archetype` APIs in their respective SDKs, as well as the [Archetypes](https://www.rerun.io/docs/reference/data_types) section of the high-level documentation.

## Usage

You can run each example individually using the following:

- **Python**: `python <example_name>.py`, e.g. `python point3d_random.py`.
- **Rust**: `cargo r -p code_examples --bin <example_name`, e.g. `cargo r -p code_examples --bin point3d_random`.
- **C++**:
- `./docs/code-examples/build_all.sh` to compile all examples
- start a Rerun Viewer listening on the default port: `rerun`
- `./build/docs/code-examples/<example_name>` to run, e.g. `./build/docs/code-examples/point3d_random`

## Roundtrips

All API examples support cross-language roundtrip tests: i.e. we execute the same logging commands from all 3 SDKs, save the results to distinct rrd files, and finally compare these rrd files.
These tests are then automatically run by the CI, which will loudly complain if the resulting rrd files don't match.

These tests check that A) all of our SDKs yield the exact same data when used the same way and B) act as regression tests, relying on the fact that it is extremely unlikely that all supported languages break in the exact same way at the exact same time.

### Usage

To run the roundtrip tests, check out `./docs/code-examples/roundtrips.py --help`.
`./docs/code-examples/roundtrips.py` is a valid invocation that will build all 3 SDKs and run all tests for all of them.

### Implementing new tests

Just pick a name for your test, and look at existing examples to get started. The `app_id` must be `rerun_example_<test_name>`.

The roundtrip process is driven by file names, so make sure all 3 tests use the same name: `<test_name>.rs`, `<test_name>.cpp`, `<test_name>.py`.

For Rust, also make sure to declare the new binary in `docs/code-examples/Cargo.toml`.

8 changes: 4 additions & 4 deletions docs/code-examples/annotation_context_arrows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
namespace rr = rerun;

int main() {
auto rr_stream = rr::RecordingStream("rerun_example_annotation_context_rects");
rr_stream.connect("127.0.0.1:9876").throw_on_failure();
auto rec = rr::RecordingStream("rerun_example_annotation_context_rects");
rec.connect("127.0.0.1:9876").throw_on_failure();

// Log an annotation context to assign a label and color to each class
rr_stream.log(
rec.log(
"/",
rr::AnnotationContext({
rr::datatypes::AnnotationInfo(1, "red", rr::datatypes::Color(255, 0, 0)),
Expand All @@ -18,7 +18,7 @@ int main() {
);

// Log a batch of 2 arrows with different `class_ids`
rr_stream.log(
rec.log(
"arrows",
rr::Arrows3D({{1.0f, 0.0f, 0.0f}, {0.0f, 1.0f, 0.0f}}).with_class_ids({1, 2})
);
Expand Down
45 changes: 27 additions & 18 deletions docs/code-examples/annotation_context_connections.py
Original file line number Diff line number Diff line change
@@ -1,28 +1,37 @@
import rerun as rr
import rerun.experimental as rr2
from rerun.experimental import dt as rrd

rr.init("rerun_example_annotation_context_connections", spawn=True)

rr.log_annotation_context(
rr2.log(
"/",
rr.ClassDescription(
info=0,
keypoint_annotations=[
rr.AnnotationInfo(0, "zero", (255, 0, 0)),
rr.AnnotationInfo(1, "one", (0, 255, 0)),
rr.AnnotationInfo(2, "two", (0, 0, 255)),
rr.AnnotationInfo(3, "three", (255, 255, 0)),
],
keypoint_connections=[(0, 2), (1, 2), (2, 3)],
rr2.AnnotationContext(
[
rrd.ClassDescription(
info=0,
keypoint_annotations=[
(0, "zero", (255, 0, 0)),
(1, "one", (0, 255, 0)),
(2, "two", (0, 0, 255)),
(3, "three", (255, 255, 0)),
],
keypoint_connections=[(0, 2), (1, 2), (2, 3)],
)
]
),
)

rr.log_points(
rr2.log(
"points",
[
(0, 0, 0),
(50, 0, 20),
(100, 100, 30),
(0, 50, 40),
],
keypoint_ids=[0, 1, 2, 3],
rr2.Points3D(
[
(0, 0, 0),
(50, 0, 20),
(100, 100, 30),
(0, 50, 40),
],
class_ids=[0],
keypoint_ids=[0, 1, 2, 3],
),
)
2 changes: 1 addition & 1 deletion docs/code-examples/annotation_context_rects.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
rr2.log("/", rr2.AnnotationContext([(1, "red", (255, 0, 0)), (2, "green", (0, 255, 0))]))

# Log a batch of 2 rectangles with different `class_ids`
rr.log_rects("/", [[-2, -2, 3, 3], [0, 0, 2, 2]], class_ids=[1, 2], rect_format=rr.RectFormat.XYWH)
rr.log_rects("detections", [[-2, -2, 3, 3], [0, 0, 2, 2]], class_ids=[1, 2], rect_format=rr.RectFormat.XYWH)

# Log an extra rect to set the view bounds
rr.log_rect("bounds", [0, 0, 5, 5], rect_format=rr.RectFormat.XCYCWH)
Loading

0 comments on commit 8b428ce

Please sign in to comment.