Skip to content

test: set USERPROFILE alongside HOME in tilde-expansion tests - #71425

Open
cryptoyasenka wants to merge 1 commit into
NousResearch:mainfrom
cryptoyasenka:fix/windows-tilde-home-tests
Open

test: set USERPROFILE alongside HOME in tilde-expansion tests#71425
cryptoyasenka wants to merge 1 commit into
NousResearch:mainfrom
cryptoyasenka:fix/windows-tilde-home-tests

Conversation

@cryptoyasenka

@cryptoyasenka cryptoyasenka commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Two tests set HOME to a temporary directory and then assert that ~ expands into it. On Windows ntpath.expanduser() reads USERPROFILE and ignores HOME, so ~ kept resolving to the real user profile and both failed.

Setting both variables leaves POSIX behaviour untouched and lets the tests actually exercise the expansion on Windows instead of asserting against whatever home directory the developer happens to have.

Updated: rebased onto current main and narrowed from four tests to the two that are still affected:

  • tests/agent/lsp/test_workspace.py::test_normalize_path_expands_tilde
  • tests/cron/test_cron_workdir.py::TestNormalizeWorkdir::test_tilde_expands

The two CLI tests this PR originally touched (tests/cli/test_cli_file_drop.py, tests/cli/test_cli_image_command.py) already received the same fix on main in 53f7d13, so they are dropped here. That is also what the conflict was about, and it is now gone.

Verified on Windows 11, Python 3.13, cherry-picked onto current main:

  • before: 2 failed, 16 passed
  • after: 18 passed

Linux and macOS are unaffected, since posixpath.expanduser() prefers HOME either way.

Green ubuntu run for this head: https://github.com/cryptoyasenka/hermes-agent/actions/runs/31056700675

Test only, no source changes. ruff check clean.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/cron Cron scheduler and job management comp/lsp Language Server Protocol integration (P2 policy) platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #49118 and the broader #63562 address the same Windows HOME-variable semantics. This PR additionally covers tests/cron/test_cron_workdir.py, so retain that fixture if the work is consolidated.

@monerostar

Copy link
Copy Markdown
Contributor

Native Win11 verification (monerostar)

Host: Windows 11 build 10.0.26200, Hermes venv Python 3.11.15, pytest 9.1.1 (-o addopts=).

Ran the four named tilde-expansion tests on current main vs this PR head.

Results

Tree Result
main (d71033a40) 4 failed
this PR (ab2eb1ba4) 4 passed

Failure shape on main (expected)

ntpath.expanduser honors USERPROFILE, not HOME, so tests that only set HOME still expand ~ to the real profile:

  • test_normalize_path_expands_tilde → got C:\Users\Admin\x.py instead of the patched home
  • test_tilde_expands → got C:\Users\Admin instead of the tmp_path home
  • test_tilde_prefixed_pathassert None is not None (path never resolved into tmp)
  • test_collect_query_images_supports_tilde_pathsValueError: Image file not found: ~/storage/shared/Pictures/cat.png

Fix check

Setting USERPROFILE alongside HOME makes all four pass on this machine with no source changes. POSIX path is unaffected in principle (still sets HOME); this is the right Windows test isolation pattern.

Verdict: solid test-only Windows fix; confirmed on live Win11. Evidence only — not a maintainer approve.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused native-Windows test isolation fix. The premise remains valid on current main: tests/agent/lsp/test_workspace.py:73 and tests/cron/test_cron_workdir.py:50 set only HOME before exercising expansion paths implemented by agent/lsp/workspace.py:41 and cron/jobs.py:1124.

Problems

  • The CLI portions are already implemented on main: tests/cli/test_cli_file_drop.py:160-162 and tests/cli/test_cli_image_command.py:61-63 set USERPROFILE alongside HOME. They landed in 53f7d137ed8fc39257a10db247e7acd4cc3445fe, so those duplicate hunks now cause the PR conflict.

Suggested changes

  • Retain only the LSP and cron fixture updates when salvaging this change; they remain missing and directly address the verified Windows failure mode.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 30, 2026
@cryptoyasenka
cryptoyasenka force-pushed the fix/windows-tilde-home-tests branch from ab2eb1b to a5032c1 Compare August 5, 2026 10:18
@cryptoyasenka

Copy link
Copy Markdown
Contributor Author

@monerostar thank you for running this on a real Win11 box and posting the exact failure shapes. That is far more useful than a green run on my own machine, and it saved me guessing about the CLI cases.

Reworked per the review. Rebased onto current main and dropped the two CLI hunks, since tests/cli/test_cli_file_drop.py and tests/cli/test_cli_image_command.py already received the same fix in 53f7d137ed8. Those duplicate hunks were the conflict, and it is gone now.

What remains is exactly the two fixtures the review asked to retain:

  • tests/agent/lsp/test_workspace.py::test_normalize_path_expands_tilde
  • tests/cron/test_cron_workdir.py::TestNormalizeWorkdir::test_tilde_expands

Windows 11 with current main checked out: 2 failed before, 2 passed after. The other two tests monerostar verified are green on main on their own now, so the reduced scope does not lose anything. PR body updated to match the new diff.

@alt-glitch alt-glitch removed comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard labels Aug 5, 2026
@cryptoyasenka
cryptoyasenka force-pushed the fix/windows-tilde-home-tests branch 2 times, most recently from 5f216f6 to de5d89f Compare August 5, 2026 20:51
ntpath.expanduser() reads USERPROFILE and ignores HOME, so on Windows these
two tests expanded "~" to the real user profile instead of the temporary home
they had just set up. Setting both env vars keeps the tests unchanged on POSIX
and makes them actually exercise the expansion on Windows.

Failures fixed on Windows:
  tests/agent/lsp/test_workspace.py::test_normalize_path_expands_tilde
  tests/cron/test_cron_workdir.py::TestNormalizeWorkdir::test_tilde_expands

The CLI fixtures this change also touched originally landed on main in
53f7d13, so those hunks are dropped here.
@cryptoyasenka
cryptoyasenka force-pushed the fix/windows-tilde-home-tests branch from de5d89f to 2570db1 Compare August 5, 2026 23:32
@cryptoyasenka cryptoyasenka changed the title tests: set USERPROFILE alongside HOME in tilde-expansion tests test: set USERPROFILE alongside HOME in tilde-expansion tests Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management comp/lsp Language Server Protocol integration (P2 policy) P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants