Skip to content

Fix more tests#1071

Merged
paul-nechifor merged 4 commits intodevfrom
pauln-fix-more-tests
Jan 21, 2026
Merged

Fix more tests#1071
paul-nechifor merged 4 commits intodevfrom
pauln-fix-more-tests

Conversation

@paul-nechifor
Copy link
Contributor

@paul-nechifor paul-nechifor commented Jan 20, 2026

Run fast tests (should take about 42 seconds):

pytest dimos

Run slow tests (should take about 5–6 minutes):

./bin/pytest-slow

Run MuJoCo tests (should take about 5 minutes):

./bin/pytest-mujoco

@paul-nechifor paul-nechifor requested a review from a team January 20, 2026 09:51
@paul-nechifor paul-nechifor marked this pull request as draft January 20, 2026 09:51
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 20, 2026

Greptile Summary

This PR improves the test infrastructure and fixes various test-related issues across the DimOS codebase.

Key Changes:

  • Test categorization improvements: Added new pytest markers (neverending, mujoco) and created helper scripts (bin/pytest-fast, bin/pytest-mujoco) to separate test execution by category
  • Unused import cleanup: Removed ~30 unused imports across multiple files to reduce code clutter
  • Test fixes: Updated tests for EmbeddingIDSystem to match API changes (embeddings now stored as numpy arrays in lists), fixed trajectory status access from dict-style to attribute access
  • CUDA compatibility: Added C++17 enforcement for NVRTC compilation to support recent CUDA/libcu++ versions
  • MuJoCo cleanup: Added atexit handler to ensure subprocess cleanup, re-enabled spatial_memory() in blueprints
  • Removed deprecated tests: Deleted visualization tests that relied on unavailable test data or manual visual inspection

Confidence Score: 4/5

  • This PR is safe to merge - primarily cleanup and test infrastructure improvements with minimal production code changes.
  • Score reflects that this is mostly test infrastructure changes and import cleanup. The one area requiring attention is the inference.py checkpoint loading change which removes error handling, but this was already flagged in a previous review thread.
  • Pay attention to dimos/models/manipulation/contact_graspnet_pytorch/inference.py (checkpoint loading error handling removed) and dimos/msgs/sensor_msgs/image_impls/CudaImage.py (sharpness method now raises instead of returning 0.0 on error).

Important Files Changed

Filename Overview
pyproject.toml Cleaned up pytest markers - removed deprecated markers, added neverending and mujoco, updated default filter options.
dimos/models/manipulation/contact_graspnet_pytorch/inference.py Replaced CheckpointIO with direct torch.load - removes graceful error handling for missing checkpoint files.
dimos/msgs/sensor_msgs/image_impls/AbstractImage.py Added workaround to force C++17 in NVRTC compilation for compatibility with recent CUDA/libcu++.
dimos/msgs/sensor_msgs/image_impls/CudaImage.py Updated C++ std to c++17, added safer kernel initialization, changed sharpness to re-raise exceptions instead of returning 0.0.
dimos/perception/detection/reid/test_embedding_id_system.py Updated tests to match new API - embeddings now stored as numpy arrays in lists instead of running average tensors.
dimos/robot/unitree_webrtc/mujoco_connection.py Added atexit handler with weakref to ensure MuJoCo subprocess is cleaned up on process exit.
dimos/robot/unitree_webrtc/unitree_go2_blueprints.py Re-enabled spatial_memory() in the spatial blueprint, removed TODO comment block, and removed unused OccupancyGrid import.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Pytest as Pytest Runner
    participant Scripts as Test Scripts
    participant Tests as Test Files
    participant CUDA as CUDA/CuPy
    participant MuJoCo as MuJoCo Subprocess

    Dev->>Scripts: ./bin/pytest-fast
    Scripts->>Pytest: exec pytest dimos
    Pytest->>Tests: Run non-excluded tests
    
    Dev->>Scripts: ./bin/pytest-slow
    Scripts->>Pytest: exec pytest -m 'not (tool or module or neverending or mujoco)'
    Pytest->>Tests: Run slow tests (excluding special categories)
    
    Dev->>Scripts: ./bin/pytest-mujoco
    Scripts->>Pytest: exec pytest -m mujoco
    Pytest->>Tests: Run MuJoCo-marked tests
    Tests->>MuJoCo: Start subprocess
    MuJoCo-->>Tests: Ready signal via shared memory
    Tests->>Tests: Execute test logic
    Tests->>MuJoCo: Stop (or atexit cleanup)
    
    Note over CUDA: C++17 now enforced for NVRTC
    Tests->>CUDA: Compile CUDA kernels
    CUDA-->>Tests: Compiled with --std=c++17
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@paul-nechifor paul-nechifor changed the title WIP: Fix more tests Fix more tests Jan 21, 2026
@paul-nechifor paul-nechifor marked this pull request as ready for review January 21, 2026 04:49
@leshy
Copy link
Contributor

leshy commented Jan 21, 2026

xj3w8-135656482

leshy
leshy previously approved these changes Jan 21, 2026
leshy
leshy previously approved these changes Jan 21, 2026
@paul-nechifor paul-nechifor merged commit 63c5a23 into dev Jan 21, 2026
14 checks passed
mustafab0 pushed a commit that referenced this pull request Jan 22, 2026
* fix more tests

* fix imports

* remove cuda sharpness test

* PYTHONWARNINGS
spomichter added a commit that referenced this pull request Jan 23, 2026
… Unitree Go2 Navigation & Exploration Beta

Pre-Release v0.0.8: Unitree Go2 Navigation & Exploration Beta, Transport Updates, Documentation updates, Rerun fixes, Person follow, Readme updates

## What's Changed
* Small docs clarification about stream getters by @leshy in #1043
* Fix split view on wide monitors by @jeff-hykin in #1048
* Docs: Install & Develop  by @jeff-hykin in #1022
* Add uv to nix and fix resulting problems by @jeff-hykin in #1021
* v0.0.8 by @paul-nechifor in #1050
* Style changes in docs by @paul-nechifor in #1051
* Revert "Add uv to nix and fix resulting problems" by @leshy in #1053
* Transport benchmarks + Raw ros transport by @leshy in #1038
* feat: default to rerun-web and auto-open browser on startup (browser … by @Nabla7 in #1019
* bbox detections visual check by @leshy in #1017
* fix: only auto-open browser for rerun-web viewer backend by @Nabla7 in #1066
* move slow tests to integration by @paul-nechifor in #1063
* Streamline transport start/stop methods by @Kaweees in #1062
* Person follow skill with EdgeTAM by @paul-nechifor in #1042
* fix: increase costmap floor z_offset to avoid z-fighting by @Nabla7 in #1073
* Fixed issue #1074 by @alexlin2 in #1075
* ROS transports initial by @leshy in #1057
* Fix System Config Values for LCM on MacOS and Refactor by @jeff-hykin in #1065
* SHM Transport basic fixes by @leshy in #1041
* commented out Mem Transport test case by @leshy in #1077
* Docs/advanced streams update 2 by @leshy in #1078
* Fix more tests by @paul-nechifor in #1071
* feat: navigation docker updates from bona_local_dev by @baishibona in #1081
* Fix missing dependencies by @Kaweees in #1085
* Release readme fixes by @spomichter in #1076

## New Contributors
* @baishibona made their first contribution in #1081

**Full Changelog**: v0.0.7...v0.0.8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants