Skip to content

test: remove flaky skipped test_basic_deployment#2630

Merged
spomichter merged 1 commit into
release/0.0.13from
chore/remove-flaky-test-basic-deployment
Jun 26, 2026
Merged

test: remove flaky skipped test_basic_deployment#2630
spomichter merged 1 commit into
release/0.0.13from
chore/remove-flaky-test-basic-deployment

Conversation

@spomichter

Copy link
Copy Markdown
Contributor

What

Deletes test_basic_deployment from dimos/core/test_core.py (and its now-unused MockRobotClient / LCMTransport / pLCMTransport / pytest imports).

Why

  • The test was marked @pytest.mark.skipif_in_ci, so it never ran in CI — it gated nothing.
  • It deploys two modules in separate worker processes and streams over LCM, then asserts a sustained ~10 Hz message rate (>= 8 messages in a fixed sleep window). That wall-clock throughput assertion races worker-process startup and is inherently flaky — locally it returns anywhere from 1 to passing across consecutive runs.
  • The behavior it exercised (cross-process deploy() + LCMTransport/pLCMTransport publish/subscribe + RPC counters) is already covered by CI-active tests: test_stream.py reuses the same MockRobotClient; test_rpcstress.py, test_module_coordinator.py, and test_async_module_dispatch_serialization.py cover the cross-process LCM + RPC paths.

The only unique thing it added was the throughput/rate guarantee, which CI never enforced anyway.

Test

dimos/core/test_core.py::test_classmethods still passes; ruff check clean (no unused imports left).

test_basic_deployment was marked skipif_in_ci so it never ran in CI, and
failed locally for timing reasons: it deploys two modules in separate
worker processes over LCM and asserts a sustained ~10Hz message rate
(>=8 messages in a fixed window). That wall-clock throughput assertion
races worker startup and is inherently flaky.

The cross-process deploy + LCMTransport/pLCMTransport publish/subscribe +
RPC behavior it exercised is already covered by CI-active tests
(test_stream.py reuses the same MockRobotClient; test_rpcstress.py,
test_module_coordinator.py, test_async_module_dispatch_serialization.py).
Drop the now-unused MockRobotClient/LCMTransport/pLCMTransport/pytest
imports along with it.
@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Removes the test_basic_deployment test (and its three now-unused imports) from dimos/core/test_core.py. The test was permanently skipped in CI via @pytest.mark.skipif_in_ci and contained a wall-clock throughput assertion that raced worker-process startup, making it inherently flaky locally.

  • The deleted test asserted >= 8 messages received in a fixed 1-second sleep window — a rate check that depended on process scheduling and was never enforced by CI.
  • The behaviors it exercised (cross-process deploy(), LCMTransport/pLCMTransport publish/subscribe, RPC counters) remain covered by test_stream.py, test_rpcstress.py, test_module_coordinator.py, and test_async_module_dispatch_serialization.py.
  • The surviving test_classmethods test and the Navigation module definition are unchanged and unaffected.

Confidence Score: 5/5

Safe to merge — pure deletion of a permanently-skipped flaky test and its unused imports, with no changes to production code or active CI coverage.

The change deletes a test that was always skipped in CI and contained a wall-clock throughput assertion that was unreliable even locally. No production code is touched, the remaining test passes, and ruff reports no stale imports. The behaviors removed from coverage are confirmed to be exercised by other active test files.

No files require special attention.

Important Files Changed

Filename Overview
dimos/core/test_core.py Removes the test_basic_deployment test and its now-unused imports (pytest, MockRobotClient, LCMTransport, pLCMTransport); the remaining test_classmethods and Navigation module definition are untouched and clean.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["test_basic_deployment\n@pytest.mark.skipif_in_ci"] -->|REMOVED| B["Never ran in CI"]
    B --> C["Wall-clock rate assertion\nasserts >= 8 msgs in 1s sleep\nInherently flaky"]
    D["Coverage retained by CI-active tests"] --> E["test_stream.py\nMockRobotClient"]
    D --> F["test_rpcstress.py\nCross-process LCM + RPC"]
    D --> G["test_module_coordinator.py\nLCM paths"]
    D --> H["test_async_module_dispatch_serialization.py\nAsync dispatch"]
    I["test_classmethods"] -->|KEPT| J["Validates Navigation.rpcs\ndict shape and attributes"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["test_basic_deployment\n@pytest.mark.skipif_in_ci"] -->|REMOVED| B["Never ran in CI"]
    B --> C["Wall-clock rate assertion\nasserts >= 8 msgs in 1s sleep\nInherently flaky"]
    D["Coverage retained by CI-active tests"] --> E["test_stream.py\nMockRobotClient"]
    D --> F["test_rpcstress.py\nCross-process LCM + RPC"]
    D --> G["test_module_coordinator.py\nLCM paths"]
    D --> H["test_async_module_dispatch_serialization.py\nAsync dispatch"]
    I["test_classmethods"] -->|KEPT| J["Validates Navigation.rpcs\ndict shape and attributes"]
Loading

Reviews (1): Last reviewed commit: "test: remove flaky skipped test_basic_de..." | Re-trigger Greptile

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@@               Coverage Diff               @@
##           release/0.0.13    #2630   +/-   ##
===============================================
  Coverage           71.00%   71.00%           
===============================================
  Files                 892      892           
  Lines               79256    79234   -22     
  Branches             7081     7081           
===============================================
- Hits                56274    56264   -10     
+ Misses              21144    21130   -14     
- Partials             1838     1840    +2     
Flag Coverage Δ
OS-ubuntu-24.04-arm 63.31% <ø> (+0.01%) ⬆️
OS-ubuntu-latest 66.07% <ø> (+0.01%) ⬆️
Py-3.10 66.07% <ø> (+0.01%) ⬆️
Py-3.11 66.06% <ø> (+<0.01%) ⬆️
Py-3.12 66.06% <ø> (+0.01%) ⬆️
Py-3.13 66.06% <ø> (+<0.01%) ⬆️
Py-3.14 66.07% <ø> (+<0.01%) ⬆️
Py-3.14t 66.06% <ø> (+0.01%) ⬆️
SelfHosted-Large 30.02% <ø> (-0.03%) ⬇️
SelfHosted-Linux 37.59% <ø> (+<0.01%) ⬆️
SelfHosted-macOS 36.44% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/core/test_core.py 72.34% <ø> (+15.81%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 26, 2026
@spomichter spomichter merged commit 971bfef into release/0.0.13 Jun 26, 2026
28 checks passed
@spomichter spomichter deleted the chore/remove-flaky-test-basic-deployment branch June 26, 2026 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants