Skip to content

test: keep the developer's robot out of the suite - #4103

Closed
KrishnaH96 wants to merge 1 commit into
mainfrom
krishna/fix/rel-env-leak
Closed

test: keep the developer's robot out of the suite#4103
KrishnaH96 wants to merge 1 commit into
mainfrom
krishna/fix/rel-env-leak

Conversation

@KrishnaH96

Copy link
Copy Markdown
Contributor

Problem

GlobalConfig reads .env itself, so a ROBOT_IP in the checkout makes every zenoh session dial the robot and stall 1 s; 5 async-module tests fail and the suite runs 3x slower.

Solution

conftest blanks ROBOT_IP/ROBOT_IPS/ZENOH_CONNECT before dimos imports; the dotenv precedence test drops them too. Popping is not enough because pydantic-settings reads the file directly.

How to test

echo ROBOT_IP=10.0.0.1 >> .env
uv run pytest dimos/core/test_async_module_rpc.py dimos/core/coordination/blueprint_config/test_sources.py

19 passed across the six affected files with ROBOT_IP set; 5 failed before.

AI assistance

Used Fable 5.1 extensively for root cause analysis, fix and testing.

Checklist

  • I have read and approved the CLA.

@codecov

codecov Bot commented Sep 13, 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             @@
##             main    #4103      +/-   ##
==========================================
- Coverage   78.59%   78.59%   -0.01%     
==========================================
  Files        1460     1460              
  Lines      138230   138234       +4     
  Branches    11881    11882       +1     
==========================================
- Hits       108642   108640       -2     
- Misses      26244    26247       +3     
- Partials     3344     3347       +3     
Components Coverage Δ
Tests 94.59% <100.00%> (-0.01%) ⬇️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.77% <100.00%> (-0.01%) ⬇️
OS-ubuntu-latest 75.48% <100.00%> (+<0.01%) ⬆️
Py-3.10 75.25% <100.00%> (+<0.01%) ⬆️
Py-3.11 75.46% <100.00%> (+<0.01%) ⬆️
Py-3.12 75.46% <100.00%> (-0.01%) ⬇️
SelfHosted-Large 30.77% <50.00%> (-0.01%) ⬇️
SelfHosted-Linux 35.68% <50.00%> (-0.01%) ⬇️
SelfHosted-macOS 35.02% <50.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
dimos/conftest.py 81.81% <100.00%> (+0.20%) ⬆️
...core/coordination/blueprint_config/test_sources.py 100.00% <100.00%> (ø)

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

Comment thread dimos/conftest.py
os.environ.setdefault("ZENOH_SCOUT_ADDR", f"224.0.0.224:{17700 + _BUCKET}")

# GlobalConfig reads .env itself, so a developer's robot must be blanked, not just unset.
for _k in ("ROBOT_IP", "ROBOT_IPS", "ZENOH_CONNECT"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should ignore everything in .env. I'm checking how it can be done in pytest.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added alternative here: #4111

@spomichter
spomichter marked this pull request as ready for review September 13, 2026 00:58
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 13, 2026
@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

Safe to merge: the changed test bootstrap and dotenv precedence behavior work with inherited local configuration values present.

What we checked:

  • Ran a focused pytest probe to validate that inherited environment variables are not used by the connection configuration and that dotenv precedence remains authoritative, as described in the contract for the dotenv-related tests. T-Rex
  • Validated that the generated probe imports the global_config and confirms emptiness for the three connection variables in both process environment and captured configuration, with the existing test passing despite inherited values and the change protection verified in the specified files. T-Rex

Summary

  • Test startup clears inherited robot and Zenoh connection settings before DIMOS configuration is imported.
  • The dotenv-precedence test clears inherited values so its temporary .env file remains authoritative.
  • Focused before-and-after checks confirmed the changed paths are isolated from local configuration.

Reviews (1) · Last reviewed commit: "test: keep the developer's robot out of ..."

@paul-nechifor

Copy link
Copy Markdown
Contributor

Added an alternative here: #4111

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport release/0.0.14 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