Skip to content

fix(tests): repair repo-wide cargo test --lib build (stale test re-exports)#4767

Closed
sanil-23 wants to merge 1 commit into
tinyhumansai:mainfrom
sanil-23:fix/lib-test-build
Closed

fix(tests): repair repo-wide cargo test --lib build (stale test re-exports)#4767
sanil-23 wants to merge 1 commit into
tinyhumansai:mainfrom
sanil-23:fix/lib-test-build

Conversation

@sanil-23

@sanil-23 sanil-23 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Repair the repo-wide cargo test --lib build. It has been broken on main — the lib cfg(test) build fails to compile with 38 errors across ~10 modules, which red-lines the "Rust Core Coverage" gate on every PR that touches Rust (e.g. #4753; and main's own tip e5c6507c is red on it).

Problem

A prior refactor split several modules into submodules and moved constants / types / helpers there (config/schema/loaddirs.rs/env.rs, channels/providers/webtypes.rs/session.rs/schemas.rs/web_errors.rs, etc.), but the parent modules' #[cfg(test)] re-export blocks — and a few inline test imports — were left referencing the old flat locations. The symbols exist; they were just no longer in the test modules' use super::* scope, so the whole lib test build fails to compile.

Solution (scope: core, test visibility only)

Restore the missing #[cfg(test)] pub(crate) use … re-exports on the parent modules, and add the missing test-local imports to the inline test modules. No product code changes.

Modules fixed:

  • config/schema/loadACTION_DIR_ENV_VAR, MEMORY_SYNC_INTERVAL_SECS_ENV_VAR, EnvLookup
  • config/schemasschemas
  • memory/schemaNAMESPACE
  • memory/chatDEFAULT_CLOUD_LLM_MODEL
  • memory/tree_source/fileTreeKind, TreeStatus
  • sandbox/dockerDockerOverrides
  • mcp_server/toolslist_tools_result_for_config
  • inference/provider/reliableStreamError
  • inference/local/service/ollama_admininterrupted_pull_settle_window_secs
  • channels/providers/web — 13 helpers (WebChatParams, compose_system_prompt_suffix, schema field helpers, budget-error helpers, …)

Acceptance criteria

  • cargo test --lib --no-run compiles clean — verified locally (0 errors; only pre-existing unused-import warnings).
  • No product-code changes — only #[cfg(test)] re-exports / test-local uses.
  • "Rust Core Coverage" gate can compile-and-run again for Rust PRs.

Impact

  • Unblocks the Rust Core Coverage / PR CI Gate checks for every Rust PR (they've been failing at compile since the refactor).
  • Test-only visibility change; no runtime behavior change.

Related

Summary by CodeRabbit

  • Tests
    • Expanded test support across web, configuration, memory, inference, MCP tools, and sandbox areas.
    • Improved access to shared helpers and constants used in validation, model selection, environment handling, and error scenarios.
    • These updates help keep existing behaviors well covered and make future regressions easier to catch.

…exports)

The lib `cfg(test)` build has been broken on `main` — `cargo test --lib`
fails to compile with 38 errors across ~10 modules, which red-lines the
"Rust Core Coverage" gate for every PR that touches Rust.

A prior refactor split several modules into submodules and moved
constants/types/helpers there, but the parent modules' `#[cfg(test)]`
re-export blocks (and a few inline test imports) still referenced the old
flat locations. The symbols exist — they were just no longer in the test
modules' `use super::*` scope. Restore the missing re-exports / test-local
imports. **No product code changes — test visibility only.**

Modules fixed: config/schema/load, config/schemas, memory/schema,
memory/chat, memory/tree_source/file, sandbox/docker, mcp_server/tools,
inference/provider/reliable, inference/local/service/ollama_admin,
channels/providers/web.

Verified: `cargo test --lib --no-run` compiles clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sanil-23 sanil-23 requested a review from a team July 9, 2026 18:38
@sanil-23 sanil-23 added the bug label Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds cfg(test)-gated re-exports and test-module imports across ten files in the openhuman crate, exposing additional helper types, functions, and constants (e.g., WebChatParams, EnvLookup, NAMESPACE, DockerOverrides, StreamError) so existing test code compiles. No production logic or public APIs change.

Changes

Test-only visibility fixes

Layer / File(s) Summary
Channels and config module test re-exports
src/openhuman/channels/providers/web/mod.rs, src/openhuman/config/schema/load/mod.rs, src/openhuman/config/schemas/mod.rs
Web module broadens cfg(test) re-exports to include schema helpers, session functions, WebChatParams, and web error helpers; config load module re-exports EnvLookup and dir env constants; config schemas module re-exports schemas for test access.
Inference and MCP server test re-exports
src/openhuman/inference/local/service/ollama_admin/mod.rs, src/openhuman/inference/provider/reliable.rs, src/openhuman/mcp_server/tools/mod.rs
Ollama admin module re-exports interrupted_pull_settle_window_secs; reliable provider re-exports StreamError; MCP tools module re-exports list_tools_result_for_config, all under #[cfg(test)].
Memory and sandbox test module imports
src/openhuman/memory/chat.rs, src/openhuman/memory/schema/mod.rs, src/openhuman/memory/tree_source/file.rs, src/openhuman/sandbox/docker.rs
Test modules import DEFAULT_CLOUD_LLM_MODEL, re-export NAMESPACE, import TreeKind/TreeStatus, and import DockerOverrides respectively to support existing tests.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested labels: rust-core

Suggested reviewers: M3gA-Mind

Poem

A rabbit hopped through crates so deep,
Found tests that couldn't wake from sleep,
Added use with cfg(test) care,
Now every helper's in the air! 🐇✨
No logic bent, no bug set free,
Just tidy paths for tests to see.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main purpose: fixing the repo-wide test build by restoring stale test re-exports.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added the rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. label Jul 9, 2026

@coderabbitai coderabbitai Bot left a comment

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.

🧹 Nitpick comments (1)
src/openhuman/config/schemas/mod.rs (1)

32-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicate #[cfg(test)] attribute.

Lines 32-33 apply #[cfg(test)] twice to the same use statement. Harmless at compile time but clearly an accidental duplication.

🧹 Proposed fix
 #[cfg(test)]
-#[cfg(test)]
 pub(crate) use schema_defs::schemas;
🤖 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 `@src/openhuman/config/schemas/mod.rs` around lines 32 - 34, The `schemas`
re-export in `mod.rs` has a duplicated `#[cfg(test)]` attribute on the same
`use` statement. Remove the extra `#[cfg(test)]` so the conditional import is
declared only once, keeping the `schema_defs::schemas` re-export gated for tests
via the existing attribute on that symbol.
🤖 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.

Nitpick comments:
In `@src/openhuman/config/schemas/mod.rs`:
- Around line 32-34: The `schemas` re-export in `mod.rs` has a duplicated
`#[cfg(test)]` attribute on the same `use` statement. Remove the extra
`#[cfg(test)]` so the conditional import is declared only once, keeping the
`schema_defs::schemas` re-export gated for tests via the existing attribute on
that symbol.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b16d6044-d7f1-4ddf-81c2-d51ea0955146

📥 Commits

Reviewing files that changed from the base of the PR and between e5c6507 and e2eb449.

📒 Files selected for processing (10)
  • src/openhuman/channels/providers/web/mod.rs
  • src/openhuman/config/schema/load/mod.rs
  • src/openhuman/config/schemas/mod.rs
  • src/openhuman/inference/local/service/ollama_admin/mod.rs
  • src/openhuman/inference/provider/reliable.rs
  • src/openhuman/mcp_server/tools/mod.rs
  • src/openhuman/memory/chat.rs
  • src/openhuman/memory/schema/mod.rs
  • src/openhuman/memory/tree_source/file.rs
  • src/openhuman/sandbox/docker.rs

@sanil-23

sanil-23 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #4759, which landed the same test-module import fix across the same 10 modules. Closing as redundant; #4753 is now rebased directly onto main (which includes #4759).

@sanil-23 sanil-23 closed this Jul 9, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in Team Openhuman Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant