test: hermeticize env-sensitive inference and gateway tests#3
Merged
Conversation
📝 WalkthroughWalkthroughThis PR strengthens test isolation and updates provider discovery assertions across five test files. A global fixture now unsets four additional base-URL override environment variables for every test. Two systemd unit tests add monkeypatching for target-user identity and home remapping. Three provider-related tests explicitly clear environment variables before resolution. The provider discovery count expectation increases from exactly 33 to at least 34 profiles with cache clearing. ChangesTest Environment Isolation and Assertion Updates
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/providers/test_plugin_discovery.py`:
- Around line 57-59: Replace the fragile count assertion on
names/expected_min_profiles in tests/providers/test_plugin_discovery.py with
invariant checks: assert that a set of required canonical provider names (e.g.,
the known core providers) are present in names, assert that len(names) ==
len(set(names)) to guarantee uniqueness, and validate structural shape of each
discovered profile (e.g., required keys/attributes and non-empty strings) using
the same variable names (names, profiles/discovered profiles) and the test
helper(s) that parse/produce profiles so the test fails only on real invariants
rather than enumeration counts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ad55d5e-4b68-450e-9dfe-59e94e949bf8
📒 Files selected for processing (5)
tests/conftest.pytests/hermes_cli/test_gateway_service.pytests/hermes_cli/test_ollama_cloud_auth.pytests/hermes_cli/test_runtime_provider_resolution.pytests/providers/test_plugin_discovery.py
Comment on lines
+57
to
+59
| assert len(names) >= expected_min_profiles, ( | ||
| f"Expected at least {expected_min_profiles} profiles, got {len(names)}: {names}" | ||
| ) |
There was a problem hiding this comment.
Replace the profile-count assertion with invariants.
Line 57 still enforces an enumeration count (>= 34), which is a change-detector and will create churn as bundled profiles evolve. Prefer invariants (e.g., required canonical providers exist, names are unique, and discovered profiles are structurally valid).
Suggested change
- expected_min_profiles = 34
- assert len(names) >= expected_min_profiles, (
- f"Expected at least {expected_min_profiles} profiles, got {len(names)}: {names}"
- )
+ assert names, "No provider profiles were discovered"
+ assert len(names) == len(set(names)), f"Duplicate profile names detected: {names}"As per coding guidelines, "DO NOT write change-detector tests (fail whenever expected-to-change data updates: ... enumeration counts ...); ... write invariant tests instead."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/providers/test_plugin_discovery.py` around lines 57 - 59, Replace the
fragile count assertion on names/expected_min_profiles in
tests/providers/test_plugin_discovery.py with invariant checks: assert that a
set of required canonical provider names (e.g., the known core providers) are
present in names, assert that len(names) == len(set(names)) to guarantee
uniqueness, and validate structural shape of each discovered profile (e.g.,
required keys/attributes and non-empty strings) using the same variable names
(names, profiles/discovered profiles) and the test helper(s) that parse/produce
profiles so the test fails only on real invariants rather than enumeration
counts.
DIZ-admin
added a commit
that referenced
this pull request
May 16, 2026
Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com>
This was referenced May 17, 2026
DIZ-admin
pushed a commit
that referenced
this pull request
May 26, 2026
…y prefixed Companion to the NousResearchGH-25255 incoming-strip fix from @hayka-pacha. Without this, build_anthropic_kwargs unconditionally added 'mcp_' to every tool name in step 3, so a native MCP server tool registered as 'mcp_composio_X' was sent as 'mcp_mcp_composio_X' on the wire. The incoming strip only removes ONE prefix, which still worked on first call, but on subsequent calls the model pattern-matched the single-prefixed form from message history and produced names that stripped to 'composio_X' — registry miss, dispatch fail. The history-rewrite block (#4) already has this guard. Apply the same guard to the schema-rewrite block (#3) so round-trip is symmetric. Added 4 outgoing-side tests. Existing 7 incoming-side tests still pass. Author map: hayka-pacha added for PR NousResearch#25270 salvage attribution. Refs NousResearchGH-25255.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Notes
Summary by CodeRabbit
Tests