Skip to content

test: make path-handling tests pass on Windows runners#88

Merged
warren618 merged 1 commit into
HKUDS:mainfrom
Teerapat-Vatpitak:fix/windows-test-compat
May 10, 2026
Merged

test: make path-handling tests pass on Windows runners#88
warren618 merged 1 commit into
HKUDS:mainfrom
Teerapat-Vatpitak:fix/windows-test-compat

Conversation

@Teerapat-Vatpitak

Copy link
Copy Markdown
Contributor

Summary

  • Fixes three pre-existing tests that pass on Linux CI but fail on Windows because of test-side platform assumptions, not application bugs.
  • Touches only agent/tests/ — no production code changes.
  • After this PR: full pytest sweep on Windows goes from 3 failed, 863 passed to 866 passed, 0 failed.

Why

A Windows contributor running pytest --ignore=agent/tests/e2e_backtest --tb=short -q against main hits these three failures:

FAILED agent/tests/test_backtest_runner_security.py::test_signal_engine_rejects_top_level_execution
FAILED agent/tests/test_backtest_runner_security.py::test_signal_engine_rejects_class_level_execution
FAILED agent/tests/test_loop_helpers.py::TestNormalizeToolRunDir::test_preserves_absolute_run_dir

Linux CI is unaffected, so the failures didn't surface in #80 / #82 / etc. The behaviour the tests exercise is correct on both platforms; only the test inputs make POSIX-specific assumptions.

What was actually wrong

1. test_backtest_runner_security.py — RCE path interpolation

Both failing tests build a signal_engine.py whose source contains pytest's tmp_path formatted into an os.system('touch ...') call. On Windows tmp_path resolves to e.g. C:\Users\teera\AppData\Local\Temp\pytest-25\.... Interpolated into a single-quoted Python string literal, the leading \U is interpreted as a (truncated) unicode escape:

ValueError: Invalid signal_engine.py syntax:
(unicode error) 'unicodeescape' codec can't decode bytes in position 8-9:
truncated \UXXXXXXXX escape

ast.parse raises SyntaxError before the security scrubber under test ever runs — so the test fails for the wrong reason on Windows, while passing on Linux because the path uses forward slashes there.

Fix: switch the interpolation to Path.as_posix(). Forward-slash paths survive Python source escaping intact while still pointing at the same filesystem location on every OS, so the scrubber gets a chance to see the actual class- / top-level statement and reject it as the test originally intended.

2. test_loop_helpers.py::TestNormalizeToolRunDir::test_preserves_absolute_run_dir

The test passed the literal "/var/tmp/custom_run" and expected it back unchanged from _normalize_tool_run_dir:

args = {"run_dir": "/var/tmp/custom_run"}
out = _normalize_tool_run_dir(args, "/tmp/run_123")
assert out["run_dir"] == "/var/tmp/custom_run"

_normalize_tool_run_dir calls Path.is_absolute(), which on Windows returns False for a bare drive-less path. The function then correctly treats the input as relative and joins it with memory_run_dir, yielding C:\var\tmp\custom_run. The function is doing the right thing — the test asserted POSIX semantics on a cross-platform helper.

Fix: resolve the test input through os.path.abspath before handing it to the function, so the input is the platform's actual absolute form (/var/tmp/custom_run on POSIX, C:\var\tmp\custom_run on Windows). The assertion then holds on both.

Verification

Before (on Windows / Python 3.12):

3 failed, 863 passed, 1 skipped, 2 warnings in 13.54s

After (same machine):

866 passed, 1 skipped, 2 warnings in 12.33s

The three previously-failing tests now pass without changing any production code. Linux CI continues to pass (the patches are no-ops on POSIX where forward slashes are already the path separator and /var/tmp/custom_run is already an absolute path).

Test plan

  • Targeted run — the three previously-failing tests pass: pytest agent/tests/test_backtest_runner_security.py::test_signal_engine_rejects_top_level_execution agent/tests/test_backtest_runner_security.py::test_signal_engine_rejects_class_level_execution agent/tests/test_loop_helpers.py::TestNormalizeToolRunDir::test_preserves_absolute_run_dir -v → 3 passed.
  • Full sweep on Windows — pytest --ignore=agent/tests/e2e_backtest --tb=line -q866 passed, 0 failed.
  • Linux behaviour preserved — Path.as_posix() on POSIX is a no-op, and os.path.abspath('/var/tmp/custom_run') returns /var/tmp/custom_run unchanged.

Checklist

  • No changes to protected areas (agent/src/agent/, agent/src/session/, agent/src/providers/) — only agent/tests/.
  • No production code touched.
  • No hardcoded secrets / paths.
  • Code follows CONTRIBUTING.md (Conventional Commit prefix test:, Google-style docstrings via comments).
  • Documentation updated — N/A.

Three tests fail when the suite is run on Windows even though the
underlying behaviour is correct on both platforms — the failures are
test-side platform assumptions, not application bugs. Linux CI is
unaffected; this is purely a quality-of-life fix for contributors who
run pytest on Windows.

1. test_backtest_runner_security.py — rce path interpolation
   ----------------------------------------------------------
   Both ``test_signal_engine_rejects_top_level_execution`` and
   ``test_signal_engine_rejects_class_level_execution`` write a
   ``signal_engine.py`` whose source contains the pytest tmp_path
   formatted into an ``os.system('touch ...')`` call. On Windows,
   tmp_path looks like ``C:\Users\teera\AppData\Local\Temp\...``;
   interpolated into a single-quoted Python string literal, the leading
   ``\U`` is interpreted as a (truncated) unicode escape and ``ast.parse``
   raises ``SyntaxError`` before the security scrubber under test ever
   runs. Switching to ``Path.as_posix()`` produces forward-slash paths
   that survive Python source escaping intact while pointing at the same
   filesystem location on every OS.

2. test_loop_helpers.py — absolute-path preservation
   --------------------------------------------------
   ``test_preserves_absolute_run_dir`` passed the literal
   ``/var/tmp/custom_run`` and expected it back unchanged.
   ``_normalize_tool_run_dir`` calls ``Path.is_absolute()``, which on
   Windows returns False for a bare drive-less path — so the function
   correctly treats the input as relative and joins it with
   ``memory_run_dir``. The test was wrong, not the function: it asserted
   POSIX semantics on a cross-platform helper. Resolving the input
   through ``os.path.abspath`` before handing it to the function
   produces the platform's actual absolute form (``/var/tmp/custom_run``
   on POSIX, ``C:\var\tmp\custom_run`` on Windows) and the assertion now
   holds on both.

Verified locally: the targeted three now pass; the full pytest sweep
goes from "3 failed, 863 passed" on main to "866 passed, 0 failed".
No production code is touched.
@warren618 warren618 merged commit e4c8858 into HKUDS:main May 10, 2026
1 check passed
@warren618

Copy link
Copy Markdown
Collaborator

Thanks for the focused Windows compatibility fix. I verified the targeted path-handling tests locally in a combined merge gate, and CI was already green, so this is now merged.

@Teerapat-Vatpitak Teerapat-Vatpitak deleted the fix/windows-test-compat branch May 13, 2026 03:58
ykykj pushed a commit to ykykj/Vibe-Trading that referenced this pull request May 23, 2026
Empirical end-to-end verification on a real run revealed the dispatch
in fetch_grounding_data was wrong:

    loader_cls = resolve_loader(code)   # resolve_loader takes a market key
    loader     = loader_cls()           # resolve_loader already returns
                                        # a ready instance, not a class

So a real call with code="NVDA.US" produced
"No available data source for market 'NVDA.US'. Tried: []." — the
fallback chain table is keyed by market type ("us_equity",
"hk_equity", "crypto", ...), not by raw symbol shape.

The unit tests passed only because they monkey-patched resolve_loader
to a `lambda code: lambda: _StubLoader(...)` that happened to satisfy
both the wrong call site and the wrong return-type expectation. A
useful reminder that mock-shaped tests can hide real interface bugs.

Fix
---
- Import the same `_detect_market` runner.py uses for source routing
  and feed its output to `resolve_loader`. The private import is a
  reasonable pragmatic choice — there is no public equivalent today,
  and replicating the regex table in two places would invite drift.
- Drop the `loader_cls()` instantiation step; `resolve_loader` already
  returns a ready loader instance.

Test
----
Updated `test_fetch_returns_normalized_bars` to assert the call is
routed through `"us_equity"` (captured via a stub `resolve_loader`).
This pins the contract: a regression that passes the raw code as the
market key, or rewrites the helper so dispatch goes elsewhere, will
fire here instead of waiting for someone to actually run a swarm.

Verified with the local pipeline:
    NVDA.US latest close = $215.20 on 2026-05-08  (7 bars, yfinance)
i.e. the same number this PR was supposed to make available to the
worker prompt, instead of the empty block the original commit
silently produced.

`pytest agent/tests/test_swarm_grounding.py -v` -> 12/12 pass.
Full sweep at the same baseline (877 passed, 3 pre-existing Windows-
specific failures untouched, separately addressed in HKUDS#88).
longmfe pushed a commit to longmfe/Vibe-Trading-Mars that referenced this pull request Jul 14, 2026
…ompat

test: make path-handling tests pass on Windows runners
longmfe pushed a commit to longmfe/Vibe-Trading-Mars that referenced this pull request Jul 14, 2026
Empirical end-to-end verification on a real run revealed the dispatch
in fetch_grounding_data was wrong:

    loader_cls = resolve_loader(code)   # resolve_loader takes a market key
    loader     = loader_cls()           # resolve_loader already returns
                                        # a ready instance, not a class

So a real call with code="NVDA.US" produced
"No available data source for market 'NVDA.US'. Tried: []." — the
fallback chain table is keyed by market type ("us_equity",
"hk_equity", "crypto", ...), not by raw symbol shape.

The unit tests passed only because they monkey-patched resolve_loader
to a `lambda code: lambda: _StubLoader(...)` that happened to satisfy
both the wrong call site and the wrong return-type expectation. A
useful reminder that mock-shaped tests can hide real interface bugs.

Fix
---
- Import the same `_detect_market` runner.py uses for source routing
  and feed its output to `resolve_loader`. The private import is a
  reasonable pragmatic choice — there is no public equivalent today,
  and replicating the regex table in two places would invite drift.
- Drop the `loader_cls()` instantiation step; `resolve_loader` already
  returns a ready loader instance.

Test
----
Updated `test_fetch_returns_normalized_bars` to assert the call is
routed through `"us_equity"` (captured via a stub `resolve_loader`).
This pins the contract: a regression that passes the raw code as the
market key, or rewrites the helper so dispatch goes elsewhere, will
fire here instead of waiting for someone to actually run a swarm.

Verified with the local pipeline:
    NVDA.US latest close = $215.20 on 2026-05-08  (7 bars, yfinance)
i.e. the same number this PR was supposed to make available to the
worker prompt, instead of the empty block the original commit
silently produced.

`pytest agent/tests/test_swarm_grounding.py -v` -> 12/12 pass.
Full sweep at the same baseline (877 passed, 3 pre-existing Windows-
specific failures untouched, separately addressed in HKUDS#88).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants