Skip to content

fix(tests): replace hardcoded /tmp paths with tempdir + add 300 unit tests - #659

Merged
ilblackdragon merged 2 commits into
mainfrom
fix/tempdir-test-isolation
Mar 7, 2026
Merged

ilblackdragon merged 2 commits into
mainfrom
fix/tempdir-test-isolation

Conversation

@ilblackdragon

Copy link
Copy Markdown
Member

Summary

  • Fix failing test e2e_metrics_test::test_metrics_collected_from_tool_trace caused by a path mismatch: setup_test_dir() created /tmp/ironclaw_metrics_test but the fixture wrote to /tmp/ironclaw_e2e_test/hello.txt
  • Add LlmTrace::replace_paths() to substitute hardcoded fixture paths with dynamic tempfile::tempdir() paths at runtime
  • Convert all 12 test files from hardcoded /tmp/ironclaw_* paths to tempfile::tempdir(), making tests isolated, parallel-safe, and leaving no debris
  • Add 300+ unit tests across 20 modules (config, context, evaluation, extensions, LLM, secrets, tools/builder, tools/mcp) for coverage push

Test plan

  • cargo fmt clean
  • cargo clippy --all --benches --tests --examples --all-features zero warnings
  • test_metrics_collected_from_tool_trace passes (was failing)
  • All 67 modified e2e/support tests pass
  • All 2385 library unit tests pass

🤖 Generated with Claude Code

ilblackdragon and others added 2 commits March 6, 2026 23:54
Add 300+ unit tests covering config, context, evaluation, extensions,
LLM, secrets, tools/builder, and tools/mcp modules. All tests are
pure unit tests (no mocks) exercising serde roundtrips, edge cases,
error paths, and business logic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The e2e_metrics_test::test_metrics_collected_from_tool_trace test was
failing because setup_test_dir() created /tmp/ironclaw_metrics_test but
the fixture referenced /tmp/ironclaw_e2e_test/hello.txt (path mismatch).

Added LlmTrace::replace_paths() to substitute fixture paths at runtime,
then converted all 12 test files from hardcoded /tmp/ironclaw_* paths to
tempfile::tempdir(). Tests are now isolated, parallel-safe, and leave no
debris on disk.

Regression test: test_metrics_collected_from_tool_trace now passes
consistently regardless of prior /tmp state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 7, 2026 07:55
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions github-actions Bot added scope: tool/builtin Built-in tools scope: tool/mcp MCP client scope: tool/builder Dynamic tool builder scope: llm LLM integration scope: secrets Secrets management scope: extensions Extension management scope: setup Onboarding / setup scope: evaluation Success evaluation scope: docs Documentation size: XL 500+ changed lines risk: high Safety, secrets, auth, or critical infrastructure contributor: core 20+ merged PRs labels Mar 7, 2026

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

Pull request overview

This PR fixes a failing e2e test caused by a path mismatch between setup_test_dir() and a fixture, introduces LlmTrace::replace_paths() to dynamically substitute hardcoded fixture paths with tempfile::tempdir() paths at runtime, converts all 12 test files from hardcoded /tmp/ironclaw_* paths to isolated temp directories, and adds 300+ unit tests across 20 modules for improved coverage.

Changes:

  • Added LlmTrace::replace_paths() and replace_in_json_value() helper for recursive JSON path substitution, enabling test fixtures to work with dynamic temp directories
  • Converted all e2e test files from hardcoded /tmp/ironclaw_* paths to tempfile::tempdir(), removing CleanupGuard usage and making tests parallel-safe
  • Added 300+ unit tests across config, context, evaluation, extensions, LLM, secrets, tools/builder, and tools/mcp modules, plus a detailed COVERAGE_PLAN.md roadmap

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/support/trace_llm.rs Added replace_paths() method and replace_in_json_value() recursive helper
tests/e2e_trace_file_tools.rs Converted to tempdir, removed CleanupGuard
tests/e2e_metrics_test.rs Fixed the originally failing test by using tempdir + replace_paths
tests/e2e_spot_checks.rs Converted two tests to tempdir
tests/e2e_advanced_traces.rs Converted four tests to tempdir
tests/e2e_tool_coverage.rs Converted two tests to tempdir
tests/e2e_worker_coverage.rs Simplified fixture patching to use replace_paths
tests/support_unit_tests.rs CleanupGuard tests now use tempdir internally
src/config/mod.rs Replaced hardcoded /tmp with std::env::temp_dir() in test defaults
src/config/llm.rs Replaced hardcoded /tmp with std::env::temp_dir() in test defaults
src/config/channels.rs Added 162 lines of unit tests for channel config types
src/config/tunnel.rs Added 215 lines of unit tests for tunnel config
src/config/sandbox.rs Added 204 lines of unit tests for sandbox config
src/tools/builtin/extension_tools.rs Replaced hardcoded /tmp with std::env::temp_dir()
src/tools/builder/validation.rs Added 163 lines of WASM validation tests
src/tools/builder/templates.rs Added 163 lines of template engine tests
src/tools/builder/core.rs Added 384 lines of builder core type tests
src/tools/mcp/session.rs Added 108 lines of MCP session manager tests
src/tools/mcp/protocol.rs Added 279 lines of MCP protocol tests
src/tools/mcp/client.rs Added 161 lines of MCP client tests
src/tools/mcp/auth.rs Added 310 lines of MCP auth tests
src/secrets/types.rs Added 226 lines of secrets type tests
src/secrets/crypto.rs Added 108 lines of crypto tests
src/llm/session.rs Added 154 lines of session manager tests
src/llm/nearai_chat.rs Added 599 lines of NEAR AI chat tests
src/extensions/mod.rs Added 418 lines of extension type tests
src/extensions/discovery.rs Added 180 lines of discovery tests
src/evaluation/success.rs Added 257 lines of evaluation tests
src/evaluation/metrics.rs Added 220 lines of metrics tests
src/context/memory.rs Added 276 lines of memory tests
src/context/manager.rs Added 391 lines of context manager tests
src/setup/wizard.rs Updated test to use dynamic path
COVERAGE_PLAN.md Added comprehensive coverage plan document

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ilblackdragon
ilblackdragon merged commit cf96a32 into main Mar 7, 2026
26 checks passed
@ilblackdragon
ilblackdragon deleted the fix/tempdir-test-isolation branch March 7, 2026 08:24
This was referenced Mar 7, 2026
bkutasi pushed a commit to bkutasi/ironclaw that referenced this pull request Mar 28, 2026
…tests (nearai#659)

* test: add unit tests across 20 modules for coverage push

Add 300+ unit tests covering config, context, evaluation, extensions,
LLM, secrets, tools/builder, and tools/mcp modules. All tests are
pure unit tests (no mocks) exercising serde roundtrips, edge cases,
error paths, and business logic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(tests): replace hardcoded /tmp paths with tempfile::tempdir

The e2e_metrics_test::test_metrics_collected_from_tool_trace test was
failing because setup_test_dir() created /tmp/ironclaw_metrics_test but
the fixture referenced /tmp/ironclaw_e2e_test/hello.txt (path mismatch).

Added LlmTrace::replace_paths() to substitute fixture paths at runtime,
then converted all 12 test files from hardcoded /tmp/ironclaw_* paths to
tempfile::tempdir(). Tests are now isolated, parallel-safe, and leave no
debris on disk.

Regression test: test_metrics_collected_from_tool_trace now passes
consistently regardless of prior /tmp state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
…tests (nearai#659)

* test: add unit tests across 20 modules for coverage push

Add 300+ unit tests covering config, context, evaluation, extensions,
LLM, secrets, tools/builder, and tools/mcp modules. All tests are
pure unit tests (no mocks) exercising serde roundtrips, edge cases,
error paths, and business logic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(tests): replace hardcoded /tmp paths with tempfile::tempdir

The e2e_metrics_test::test_metrics_collected_from_tool_trace test was
failing because setup_test_dir() created /tmp/ironclaw_metrics_test but
the fixture referenced /tmp/ironclaw_e2e_test/hello.txt (path mismatch).

Added LlmTrace::replace_paths() to substitute fixture paths at runtime,
then converted all 12 test files from hardcoded /tmp/ironclaw_* paths to
tempfile::tempdir(). Tests are now isolated, parallel-safe, and leave no
debris on disk.

Regression test: test_metrics_collected_from_tool_trace now passes
consistently regardless of prior /tmp state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: high Safety, secrets, auth, or critical infrastructure scope: docs Documentation scope: evaluation Success evaluation scope: extensions Extension management scope: llm LLM integration scope: secrets Secrets management scope: setup Onboarding / setup scope: tool/builder Dynamic tool builder scope: tool/builtin Built-in tools scope: tool/mcp MCP client size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants