Skip to content

fix tests - #3586

Open
paul-nechifor wants to merge 1 commit into
mainfrom
paul/fix/tests-again
Open

fix tests#3586
paul-nechifor wants to merge 1 commit into
mainfrom
paul/fix/tests-again

Conversation

@paul-nechifor

Copy link
Copy Markdown
Contributor

Problem

Three broken tests:

ERROR dimos/e2e_tests/test_cockpit_browser.py::test_cockpit_live_data_and_reconnect[chromium] - playwright._impl._errors.Error: BrowserType.launch: Executable doesn't exist at /home/p/.cache/ms-playwright/chromium_headless_shell-1228/chrome-headless-shell-linux64/chrome-headless-shell
ERROR dimos/e2e_tests/test_cockpit_browser.py::test_cockpit_live_data_and_reconnect[firefox] - playwright._impl._errors.Error: BrowserType.launch: Executable doesn't exist at /home/p/.cache/ms-playwright/firefox-1532/firefox/firefox
ERROR dimos/models/vl/test_captioner.py::test_captioner[MoondreamVlModel] - Failed: Non-closed threads created during this test. Thread names: ['Thread-3347 (_read_thread)']. Please look at the first test that fails and fix that.

Solution

Fixed.

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 30.00000% with 7 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/e2e_tests/test_cockpit_browser.py 33.33% 4 Missing ⚠️
dimos/models/base.py 25.00% 2 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main    #3586      +/-   ##
==========================================
- Coverage   76.99%   76.99%   -0.01%     
==========================================
  Files        1260     1260              
  Lines      119791   119800       +9     
  Branches    10603    10604       +1     
==========================================
- Hits        92237    92236       -1     
- Misses      24500    24507       +7     
- Partials     3054     3057       +3     
Flag Coverage Δ
OS-ubuntu-24.04-arm 71.94% <30.00%> (-0.01%) ⬇️
OS-ubuntu-latest 73.68% <30.00%> (-0.01%) ⬇️
Py-3.10 73.67% <30.00%> (-0.01%) ⬇️
Py-3.11 73.67% <30.00%> (-0.01%) ⬇️
Py-3.12 73.67% <30.00%> (-0.01%) ⬇️
Py-3.13 73.67% <30.00%> (-0.01%) ⬇️
Py-3.14 73.67% <30.00%> (-0.02%) ⬇️
Py-3.14t 73.67% <30.00%> (-0.01%) ⬇️
SelfHosted-Large 30.11% <30.00%> (+0.02%) ⬆️
SelfHosted-Linux 35.48% <30.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
dimos/models/base.py 71.76% <25.00%> (-2.31%) ⬇️
dimos/e2e_tests/test_cockpit_browser.py 9.41% <33.33%> (+1.91%) ⬆️

... and 4 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.

@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change automatically installs the pinned Chromium and Firefox browsers for cockpit tests, documents that behavior, and releases Torch Inductor compilation resources when a local model stops.

A focused concurrent reproduction exercised the local-model cleanup path while a separate model performed lazy Inductor compilation. Cleanup terminated the shared compile-worker processes, but the concurrent model completed successfully with a result and without a closed-pool error or blocking. This disproves the reported failure consequence, so no code changes are requested.

Merge safety: safe to merge.

Confidence Score: 5/5

The tested local-model cleanup behavior allows an overlapping Inductor compilation to complete after compile workers are shut down.

No actionable findings remain. The reported concurrent compilation failure was directly exercised and contradicted by the observed successful completion.

Files Needing Attention: None.

T-Rex T-Rex Logs

What T-Rex did

  • Ran an authored reproduction that first created the Inductor worker pool, then started a second lazy torch.compile(..., backend="inductor") while invoking the same loaded-module worker-shutdown path used by LocalModel.stop().
  • The baseline run recorded five torch._inductor.compile_worker processes.
  • In the concurrent run, the workers exited after shutdown, the second model thread completed with no error, and the RESULT was race-completed.
  • The installed PyTorch async compile shutdown implementation was inspected, showing the shared worker-pool is terminated and the concurrent compilation completed.
  • The analysis identifies the product call site dimos/models/base.py:122-127 and, given the runtime results, describes this concurrent-compilation scenario as non-problematic for the tested conditions.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "fix tests" | Re-trigger Greptile

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
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.

1 participant