test(agent): honor Windows expanduser home in HOME-dependent tests - #49118
test(agent): honor Windows expanduser home in HOME-dependent tests#49118hakanpak wants to merge 1 commit into
Conversation
os.path.expanduser resolves ~ from USERPROFILE (then HOMEDRIVE+HOMEPATH) on Windows and from HOME on POSIX. Two tests set only HOME, so on native Windows ~ expanded to the real user profile instead of the test's tmp dir: - test_normalize_path_expands_tilde asserted against the wrong home. - test_blocks_sensitive_home_and_hermes_paths silently skipped the ~/.ssh/id_rsa block (the guard compared against the real profile), so a security check went unexercised on Windows. Pin USERPROFILE alongside HOME and clear HOMEDRIVE/HOMEPATH so ~ resolves to the same home on every platform. No production change; POSIX behavior is unaffected. Part of NousResearch#48986.
|
Thanks for the focused Windows test-isolation fix. The premise remains present on current main: Automated hermes-sweeper review. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three open PRs address Windows tilde-expansion tests that set HOME without pinning the USERPROFILE source used by os.path.expanduser on Windows. #49118 fixes two tests, #63562 applies the more robust HOME/USERPROFILE-plus-fallback isolation to seven tests, and #71425 overlaps three of those while uniquely covering the cron workdir test.
Related pull requests
- #49118
related— (+13/-0) — superseded: Fixes the workspace and sensitive-path tests by pinning USERPROFILE and clearing HOMEDRIVE/HOMEPATH, but both changes are contained in the broader #63562. Despite the keep_open review on #49118, the complete diffs show that #63562 covers the same two test defects and five additional sibling paths. - #63562
duplicate— (+29/-5) — preferred consolidation base: Covers all seven identified tilde-dependent fixtures, including both #49118 targets, and consistently pins HOME/USERPROFILE while removing Windows fallback variables; it also uses semantic Path comparisons where separators can differ. Its keep_open review identified the sibling gaps, and the updated diff addresses every path named by that review. - #71425
related— (+8/-0) — partially duplicate with one unique fixture: Overlaps #63562 in the workspace and two CLI tests and was validated on native Windows, but only sets USERPROFILE rather than also clearing fallback variables. Its cron workdir fixture is not present in #63562 and should be retained during consolidation.
Duplicates
#49118 is fully duplicated by #63562. #71425 substantially overlaps #63562 for three tests, with tests/cron/test_cron_workdir.py as its unique addition.
Suggested consolidation
Merge #63562 after incorporating the cron workdir fixture from #71425, preserving #63562's consistent clearing of HOMEDRIVE/HOMEPATH. Then close #49118 as fully superseded and #71425 as consolidated after its unique cron coverage has been ported.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup49118 ["PRs duplicating each other"]
P49118["PR #49118 (open)"]
P63562["PR #63562 (open)"]
end
class P49118 open
class P63562 open
class P49118 target
click P49118 "https://github.com/NousResearch/hermes-agent/pull/49118"
click P63562 "https://github.com/NousResearch/hermes-agent/pull/63562"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 11 kB of PR diffs, 3 kB of issue/PR text, 5 kB of discussion (8 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
What
Two HOME-dependent tests now resolve
~to the test's tmp dir on native Windows, instead of leaking the real user profile.Why
os.path.expanduserreadsUSERPROFILE(thenHOMEDRIVE+HOMEPATH) on Windows andHOMEon POSIX. The tests set onlyHOME, so on native Windows~expanded to the real profile:test_normalize_path_expands_tildeasserted against the wrong home and failed.test_blocks_sensitive_home_and_hermes_pathscompared the~/.ssh/id_rsaguard against the real profile, so the SSH-key block was never actually exercised on Windows — the security assertion silently passed over.Change
Pin
USERPROFILEalongsideHOMEand clearHOMEDRIVE/HOMEPATHso~resolves to the same home on every platform. Test-only; no production code change, POSIX behavior unaffected.Tests
Both pass on native Windows after the change; unchanged on POSIX.
Part of #48986.