Skip to content

test(install): stop three Windows failures that assume the POSIX layout - #2802

Open
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:test/windows-install-destination-portability
Open

test(install): stop three Windows failures that assume the POSIX layout#2802
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:test/windows-install-destination-portability

Conversation

@abhay-codes07

Copy link
Copy Markdown
Contributor

Three install tests fail on Windows for reasons that have nothing to do with what they are testing. Same class as #2647 and #2651: the assertion, not the product, is what assumes POSIX.

No product code changes.

1. test_codex_skill_uses_graphify_with_existing_graph

skill = (Path(graphify.__file__).parent / "skill-codex.md").read_text()
assert "Fast path — existing graph" in skill

read_text() with no encoding= uses the locale codepage, so on Windows the skill's UTF-8 em dash is decoded as cp1252 and Fast path — existing graph never matches. The file is fine; the read is not.

Note this is the same defect #2209 fixes in test_merge_chunks_validation.py and test_pipeline.py. That PR does not touch test_install.py, so there is no overlap, but it is the same fix and worth landing together.

2. test_hermes_skill_destination_posix_uses_home

assert str(dst).endswith(".hermes/skills/graphify/SKILL.md"), dst

The test patches platform.system() to "Linux" to exercise the non-Windows branch, but patching that does not change pathlib's flavour — Path on Windows is still WindowsPath, so str(dst) comes back with backslashes and the forward-slash suffix never matches. The branch under test is being reached; only the comparison is wrong. as_posix() compares the path rather than the host's separator.

3. test_skill_roundtrip_at_real_destination[user-hermes]

The test patches Path.home to its own home and then asserts the destination lands under it. Hermes on Windows deliberately resolves through %LOCALAPPDATA% instead of ~ (#1403), and LOCALAPPDATA still points at the session-wide sandbox home from conftest._sandbox_home, so the destination lands under that home and the assertion fails:

'...\sandbox-home2166\AppData\Local\hermes\skills\graphify\SKILL.md'
  .startswith('...\test_skill_roundtrip_at_real_d12\home')  ->  False

Redirecting LOCALAPPDATA alongside the Path.home patch is what the test already means by "home" — it mirrors exactly what conftest does, just scoped to this test's directory. The assertion then checks the real thing again instead of passing by accident on POSIX.

Deliberately not included

The three gemini failures in test_uninstall_scope.py and test_install_references.py look like the same class but are not: they hardcode ~/.gemini/skills/graphify while the install lands in ~/.agents/skills/graphify on Windows, which is a shared destination with the agents platform. Whether the test or the product is wrong depends on a call I cannot make from outside — I opened #2800 with both options rather than guess, because the two answers require opposite edits to these same tests. Happy to follow up once it is decided.

Validation

Windows 11, Python 3.12, branched off 4fca621 (0.9.44).

Full suite: 20 failed, 4469 passed -> 17 failed, 4472 passed. Exactly the three above move from fail to pass, and nothing else changes state:

FIXED:
  tests/test_install.py::test_codex_skill_uses_graphify_with_existing_graph
  tests/test_install.py::test_hermes_skill_destination_posix_uses_home
  tests/test_install_roundtrip.py::test_skill_roundtrip_at_real_destination[user-hermes]
NEW:
  (none)

The remaining 17 are pre-existing Windows failures — symlink privileges, FIFO/socket fixtures, the three gemini ones above, and similar — untouched by this.

On Linux all three passed before and still pass: these assertions were true there by accident of platform, so this is a no-op on your CI. The point is only that the Windows run stops lying about them.

None of these three test the thing that breaks them; the assertion, not the
product, is what assumes POSIX. No product code changes.

test_codex_skill_uses_graphify_with_existing_graph read skill-codex.md through
read_text() with no encoding=, so on Windows the locale codepage decoded the
file's UTF-8 em dash as cp1252 and "Fast path - existing graph" never matched.
Both reads of that file in this module now pass encoding="utf-8", so the sibling
test cannot start failing the day one of its assertions gains a non-ASCII
character. Same defect Graphify-Labs#2209 fixes in test_merge_chunks_validation.py and
test_pipeline.py; that PR does not touch test_install.py, so there is no overlap.

test_hermes_skill_destination_posix_uses_home patched platform.system() to
"Linux" to reach the non-Windows branch, but that does not change pathlib's
flavour: Path stays WindowsPath, str(dst) comes back with backslashes, and the
forward-slash suffix could never match. The branch under test was being reached
correctly; only the comparison was wrong, so it now compares as_posix().

test_skill_roundtrip_at_real_destination[user-hermes] patched Path.home and then
asserted the destination lands under it, but hermes on Windows resolves through
%LOCALAPPDATA% rather than ~ (Graphify-Labs#1403), and LOCALAPPDATA still pointed at the
session-wide sandbox home from conftest._sandbox_home. Redirecting it alongside
the Path.home patch is what the test already means by "home" -- it mirrors what
conftest does, scoped to this test's directory -- so the assertion checks the
real destination again instead of passing by accident of platform.

The three gemini failures in test_uninstall_scope.py and test_install_references.py
look like the same class but are not, and are deliberately left alone: they
hardcode ~/.gemini/skills/graphify while the install lands in
~/.agents/skills/graphify on Windows, a destination shared with the agents
platform. Whether the test or the product is wrong is Graphify-Labs#2800, and the two answers
need opposite edits to these same tests.
Copilot AI lite review requested due to automatic review settings August 16, 2026 18:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).


Graphify review — findings

Fixes Windows test failures in the install suite by reading skill-codex.md with encoding="utf-8" so em/en-dash assertions match, and comparing skill destination paths via as_posix() instead of the host separator. Redirects LOCALAPPDATA in test_skill_roundtrip_at_real_destination so hermes's Windows destination resolves under the test's home rather than the session sandbox.

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 164 functions depend on the 164 functions this change touches.

Health — grade A; no new coupling hotspots.

Verification — 164 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 164 function(s) in the blast radius were not formally verified this run

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