FileSink does not cause python unit tests to fail on errors
Describe the bug
If you have an error that occurs during spawn_and_stream it gets logged as an error on the terminal but it the process returns successfully. This can cause unit test to succeed when they otherwise should fail.
To Reproduce Steps to reproduce the behavior:
- Change the method
write_arrow_to_bytesincrates/store/re_log_encoding/src/codec/arrow.rsto immediately return an error. - Build normally.
- Run unit test
test_roundtrip_sendinrerun_py/tests/unit/test_dataframe.py - Unit test succeeds because we do not raise any assertions from the spawned task errors only being reported to the terminal
Expected behavior
An error message in the terminal should cause unit tests to fail.
Additional context
A proof of concept solution to this is below.I have changed two lines, using capfd fixture and the final assert to make sure "ERROR" doesn't occur in stderr. There is an open question as to whether warnings should also cause unit test failures.
If implemented, we would want to this across most or all of our py unit tests.
def test_roundtrip_send(self, capfd) -> None:
df = self.recording.view(index="my_index", contents="/**").select().read_all()
with tempfile.TemporaryDirectory() as tmpdir:
rrd = tmpdir + "/tmp.rrd"
with rr.RecordingStream("rerun_example_test_recording") as rec:
rec.save(rrd)
rr.dataframe.send_dataframe(df, rec=rec)
round_trip_recording = rr.dataframe.load_recording(rrd)
df_round_trip = round_trip_recording.view(index="my_index", contents="/**").select().read_all()
print("df:")
print(df)
print()
print("df_round_trip:")
print(df_round_trip)
print()
assert df == df_round_trip
assert "ERROR" not in capfd.readouterr().err
If the error originates in Python, then PYTHONWARNINGS="error" should catch it:
- https://github.com/rerun-io/rerun/pull/9384
If the error originates in Rust, then setting RERUN_PANIC_ON_WARN=1 should catch it.
Do you think we should add these variables to py-test in pixi.toml then? Seems easy enough
Yes, I think we should set these three everywhere in our CI:
PYTHONWARNINGS="error"
RERUN_PANIC_ON_WARN=1
RERUN_STRICT=1